[PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability v2

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Eric W. Biederman
Date: Friday, January 1, 2010 - 3:43 pm

When sysfs_readdir stops short we now cache the next
sysfs_dirent to return to user space in filp->private_data.
There is no impact on the rest of sysfs by doing this and
in the common case it allows us to pick up exactly where
we left off with no seeking.

Additionally I drop and regrab the sysfs_mutex around
filldir to avoid a page fault abritrarily increasing the
hold time on the sysfs_mutex.

v2: Returned to using INT_MAX as the EOF condition.
    seekdir is ambiguous unless all directory entries have
    a unique f_pos value.

Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
---

Linus good catch.

 fs/sysfs/dir.c |   82 +++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index f05f230..5d88e30 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -827,11 +827,46 @@ static inline unsigned char dt_type(struct sysfs_dirent *sd)
 	return (sd->s_mode >> 12) & 15;
 }
 
+static int sysfs_dir_release(struct inode *inode, struct file *filp)
+{
+	sysfs_put(filp->private_data);
+	return 0;
+}
+
+static struct sysfs_dirent *sysfs_dir_pos(struct sysfs_dirent *parent_sd,
+	ino_t ino, struct sysfs_dirent *pos)
+{
+	if (pos) {
+		int valid = !(pos->s_flags & SYSFS_FLAG_REMOVED) &&
+			pos->s_parent == parent_sd &&
+			ino == pos->s_ino;
+		sysfs_put(pos);
+		if (valid)
+			return pos;
+	}
+	pos = NULL;
+	if ((ino > 1) && (ino < INT_MAX)) {
+		pos = parent_sd->s_dir.children;
+		while (pos && (ino > pos->s_ino))
+			pos = pos->s_sibling;
+	}
+	return pos;
+}
+
+static struct sysfs_dirent *sysfs_dir_next_pos(struct sysfs_dirent *parent_sd,
+	ino_t ino, struct sysfs_dirent *pos)
+{
+	pos = sysfs_dir_pos(parent_sd, ino, pos);
+	if (pos)
+		pos = pos->s_sibling;
+	return pos;
+}
+
 static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 {
 	struct dentry *dentry = filp->f_path.dentry;
 	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
-	struct sysfs_dirent *pos;
+	struct sysfs_dirent *pos = filp->private_data;
 	ino_t ino;
 
 	if (filp->f_pos == 0) {
@@ -847,29 +882,31 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 		if (filldir(dirent, "..", 2, filp->f_pos, ino, DT_DIR) == 0)
 			filp->f_pos++;
 	}
-	if ((filp->f_pos > 1) && (filp->f_pos < INT_MAX)) {
-		mutex_lock(&sysfs_mutex);
-
-		/* Skip the dentries we have already reported */
-		pos = parent_sd->s_dir.children;
-		while (pos && (filp->f_pos > pos->s_ino))
-			pos = pos->s_sibling;
-
-		for ( ; pos; pos = pos->s_sibling) {
-			const char * name;
-			int len;
-
-			name = pos->s_name;
-			len = strlen(name);
-			filp->f_pos = ino = pos->s_ino;
+	mutex_lock(&sysfs_mutex);
+	for (pos = sysfs_dir_pos(parent_sd, filp->f_pos, pos);
+	     pos;
+	     pos = sysfs_dir_next_pos(parent_sd, filp->f_pos, pos)) {
+		const char * name;
+		unsigned int type;
+		int len, ret;
+
+		name = pos->s_name;
+		len = strlen(name);
+		ino = pos->s_ino;
+		type = dt_type(pos);
+		filp->f_pos = ino;
+		filp->private_data = sysfs_get(pos);
 
-			if (filldir(dirent, name, len, filp->f_pos, ino,
-					 dt_type(pos)) < 0)
-				break;
-		}
-		if (!pos)
-			filp->f_pos = INT_MAX;
 		mutex_unlock(&sysfs_mutex);
+		ret = filldir(dirent, name, len, filp->f_pos, ino, type);
+		mutex_lock(&sysfs_mutex);
+		if (ret < 0)
+			break;
+	}
+	mutex_unlock(&sysfs_mutex);
+	if ((filp->f_pos > 1) && !pos) { /* EOF */
+		filp->f_pos = INT_MAX;
+		filp->private_data = NULL;
 	}
 	return 0;
 }
@@ -878,5 +915,6 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 const struct file_operations sysfs_dir_operations = {
 	.read		= generic_read_dir,
 	.readdir	= sysfs_readdir,
+	.release	= sysfs_dir_release,
 	.llseek		= generic_file_llseek,
 };
-- 
1.6.5.2.143.g8cc62

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Linux 2.6.33-rc2 - Merry Christmas ..., Linus Torvalds, (Thu Dec 24, 3:00 pm)
-tip: origin tree boot crash, Ingo Molnar, (Fri Dec 25, 3:27 am)
Re: Linux 2.6.33-rc2 - Blank screen for Intel KMS, Miguel Calleja, (Fri Dec 25, 6:10 am)
Re: -tip: origin tree boot crash, Dmitry Torokhov, (Fri Dec 25, 12:49 pm)
Re: Linux 2.6.33-rc2 - Merry Christmas ..., Borislav Petkov, (Fri Dec 25, 1:00 pm)
Re: Linux 2.6.33-rc2 - Merry Christmas ..., Borislav Petkov, (Fri Dec 25, 2:50 pm)
Re: Linux 2.6.33-rc2 - Merry Christmas ..., Jesse Barnes, (Fri Dec 25, 11:00 pm)
Re: Linux 2.6.33-rc2 - Merry Christmas ..., Borislav Petkov, (Sat Dec 26, 1:02 am)
Re: -tip: origin tree boot crash, Len Brown, (Sat Dec 26, 1:17 pm)
Re: -tip: origin tree boot crash, Len Brown, (Sat Dec 26, 1:19 pm)
Re: -tip: origin tree boot crash, Len Brown, (Sat Dec 26, 9:20 pm)
Re: -tip: origin tree boot crash, Ingo Molnar, (Mon Dec 28, 2:44 am)
Re: -tip: origin tree boot crash, Ingo Molnar, (Mon Dec 28, 5:01 am)
Re: -tip: origin tree boot crash, Paul Rolland, (Mon Dec 28, 8:02 am)
Re: -tip: origin tree boot crash, Paul Rolland, (Mon Dec 28, 9:15 am)
Re: -tip: origin tree boot crash, Paul Rolland, (Mon Dec 28, 9:53 am)
Re: -tip: origin tree boot crash, Dmitry Torokhov, (Mon Dec 28, 1:17 pm)
Re: Linux 2.6.33-rc2 - Blank screen for Intel KMS, Miguel Calleja, (Tue Dec 29, 2:50 am)
Re: Linux 2.6.33-rc2 - Blank screen for Intel KMS, Rafael J. Wysocki, (Tue Dec 29, 7:01 am)
Re: -tip: origin tree boot crash, Len Brown, (Tue Dec 29, 11:14 pm)
Re: -tip: origin tree boot crash, Paul Rolland, (Wed Dec 30, 12:13 am)
Re: drm_vm.c:drm_mmap: possible circular locking dependenc ..., Eric W. Biederman, (Wed Dec 30, 2:34 pm)
Re: drm_vm.c:drm_mmap: possible circular locking dependenc ..., Eric W. Biederman, (Thu Dec 31, 1:40 am)
Re: drm_vm.c:drm_mmap: possible circular locking dependenc ..., Eric W. Biederman, (Thu Dec 31, 1:40 am)
[PATCH] sysfs: Cache the last sysfs_dirent to improve read ..., Eric W. Biederman, (Fri Jan 1, 3:43 pm)
Re: drm_vm.c:drm_mmap: possible circular locking dependenc ..., Eric W. Biederman, (Sat Jan 2, 10:38 pm)
Re: drm_vm.c:drm_mmap: possible circular locking dependenc ..., Eric W. Biederman, (Mon Jan 4, 12:43 pm)
Re: [PATCH] sysfs: Add lockdep annotations for the sysfs a ..., Eric W. Biederman, (Sun Jan 17, 10:18 am)
Re: [PATCH] sysfs: Add lockdep annotations for the sysfs a ..., Dominik Brodowski, (Sun Jan 17, 11:03 am)