linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Borislav Petkov <petkovbb@googlemail.com>,
	David Airlie <airlied@linux.ie>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg KH <greg@kroah.com>, Al Viro <viro@ZenIV.linux.org.uk>
Subject: [PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability v2
Date: Fri, 01 Jan 2010 14:43:53 -0800	[thread overview]
Message-ID: <m1y6kh30pi.fsf_-_@fess.ebiederm.org> (raw)
In-Reply-To: <alpine.LFD.2.00.1001011047580.3630@localhost.localdomain> (Linus Torvalds's message of "Fri\, 1 Jan 2010 10\:56\:15 -0800 \(PST\)")


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


  reply	other threads:[~2010-01-01 22:44 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-24 22:00 Linux 2.6.33-rc2 - Merry Christmas Linus Torvalds
2009-12-25 10:27 ` -tip: origin tree boot crash Ingo Molnar
2009-12-25 19:49   ` Dmitry Torokhov
2009-12-26 20:19     ` Len Brown
2009-12-26 20:17   ` Len Brown
2009-12-27  4:20     ` Len Brown
2009-12-28  9:44       ` Ingo Molnar
2009-12-28 12:01         ` Ingo Molnar
2009-12-28 15:02           ` Paul Rolland
2009-12-28 16:15             ` Paul Rolland
2009-12-28 16:53             ` Paul Rolland
2009-12-28 20:17               ` Dmitry Torokhov
2009-12-30  6:14               ` Len Brown
2009-12-30  7:13                 ` Paul Rolland
2009-12-30  6:19               ` [PATCH] wmi: check find_guid() return value to prevent oops Len Brown
2009-12-30  6:21               ` [PATCH] dell-wmi: sys_init_module: 'dell_wmi'->init suspiciously returned 21, it should follow 0/-E convention Len Brown
2009-12-25 13:10 ` Linux 2.6.33-rc2 - Blank screen for Intel KMS Miguel Calleja
2009-12-29  9:50   ` Miguel Calleja
2009-12-29 14:01     ` Rafael J. Wysocki
2009-12-25 20:00 ` Linux 2.6.33-rc2 - Merry Christmas Borislav Petkov
2009-12-25 21:50   ` Borislav Petkov
2009-12-26  6:00     ` Jesse Barnes
2009-12-26  8:02       ` Borislav Petkov
2009-12-26  9:36 ` EHCI resume sysfs duplicates (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...) Borislav Petkov
2009-12-26  9:45 ` drm_vm.c:drm_mmap: possible circular locking dependency detected " Borislav Petkov
2009-12-28  0:40   ` KOSAKI Motohiro
2009-12-30 21:10     ` Linus Torvalds
2009-12-30 21:34       ` Eric W. Biederman
2009-12-30 22:03         ` Linus Torvalds
2009-12-31  8:40           ` Eric W. Biederman
2009-12-31 19:04             ` Linus Torvalds
2010-01-01 13:58               ` [PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability Eric W. Biederman
2010-01-01 15:33                 ` Borislav Petkov
2010-01-01 18:56                 ` Linus Torvalds
2010-01-01 22:43                   ` Eric W. Biederman [this message]
2010-01-01 23:10                     ` [PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability v2 Linus Torvalds
2010-01-02  5:59                       ` Greg KH
2010-01-02 15:40                       ` Borislav Petkov
2010-01-01 15:16               ` drm_vm.c:drm_mmap: possible circular locking dependency detected (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...) Eric W. Biederman
2010-01-02  2:59                 ` drm_vm.c:drm_mmap: possible circular locking dependency detected Tejun Heo
2010-01-02 21:37                   ` [PATCH] sysfs: Add lockdep annotations for the sysfs active reference Eric W. Biederman
2010-01-03  0:02                     ` Tejun Heo
2010-01-17 16:26                     ` Ming Lei
2010-01-17 17:18                       ` Eric W. Biederman
2010-01-17 18:03                         ` Dominik Brodowski
2010-01-02 21:49                   ` drm_vm.c:drm_mmap: possible circular locking dependency detected Eric W. Biederman
2010-01-03  0:32                     ` Tejun Heo
2010-01-03  2:06                       ` Eric W. Biederman
2010-01-03  5:01                         ` Tejun Heo
2010-01-03  5:38                           ` Eric W. Biederman
2010-01-03  6:05                             ` Tejun Heo
2010-01-03  7:47                       ` Dmitry Torokhov
2010-01-03 10:57                         ` Eric W. Biederman
2010-01-03 11:14                           ` Eric W. Biederman
2010-01-04 19:16                             ` Dmitry Torokhov
2010-01-04 18:57                           ` Dmitry Torokhov
2010-01-04 19:43                             ` Eric W. Biederman
2010-01-04 21:12                               ` Dmitry Torokhov
2010-01-04 23:09                               ` Tejun Heo
2009-12-31  8:40           ` drm_vm.c:drm_mmap: possible circular locking dependency detected (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...) Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1y6kh30pi.fsf_-_@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=airlied@linux.ie \
    --cc=greg@kroah.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=petkovbb@googlemail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).