All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: ebiederm@xmission.com, cornelia.huck@de.ibm.com, greg@kroah.com,
	stern@rowland.harvard.edu, kay.sievers@vrfy.org,
	linux-kernel@vger.kernel.org, htejun@gmail.com
Cc: Tejun Heo <htejun@gmail.com>
Subject: [PATCH 15/15] sysfs: move sysfs file poll implementation to sysfs_open_dirent
Date: Thu, 20 Sep 2007 16:05:12 +0900	[thread overview]
Message-ID: <11902719121259-git-send-email-htejun@gmail.com> (raw)
In-Reply-To: <11902719091692-git-send-email-htejun@gmail.com>

Sysfs file poll implementation is scattered over sysfs and kobject.
Event numbering is done in sysfs_dirent but wait itself is done on
kobject.  This not only unecessarily bloats both kobject and
sysfs_dirent but is also buggy - if a sysfs_dirent is removed while
there still are pollers, the associaton betwen the kobject and
sysfs_dirent breaks and kobject may be freed with the pollers still
sleeping on it.

This patch moves whole poll implementation into sysfs_open_dirent.
Each time a sysfs_open_dirent is created, event number restarts from 1
and pollers sleep on sysfs_open_dirent.  As event sequence number is
meaningless without any open file and pollers should have open file
and thus sysfs_open_dirent, this ephemeral event counting works and is
a saner implementation.

This patch fixes the dnagling sleepers bug and reduces the sizes of
kobject and sysfs_dirent by one pointer.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/dir.c          |    1 -
 fs/sysfs/file.c         |   25 +++++++++++++++++++------
 fs/sysfs/sysfs.h        |    1 -
 include/linux/kobject.h |    1 -
 lib/kobject.c           |    1 -
 5 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b81744b..ad3394a 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -318,7 +318,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)
 
 	atomic_set(&sd->s_count, 1);
 	atomic_set(&sd->s_active, 0);
-	atomic_set(&sd->s_event, 1);
 
 	sd->s_name = name;
 	sd->s_mode = mode;
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index b13ba94..c05f961 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -62,6 +62,8 @@ static spinlock_t sysfs_open_dirent_lock = SPIN_LOCK_UNLOCKED;
 
 struct sysfs_open_dirent {
 	atomic_t		refcnt;
+	atomic_t		event;
+	wait_queue_head_t	poll;
 	struct list_head	buffers; /* goes through sysfs_buffer.list */
 };
 
@@ -104,7 +106,7 @@ static int fill_read_buffer(struct dentry * dentry, struct sysfs_buffer * buffer
 	if (!sysfs_get_active_two(attr_sd))
 		return -ENODEV;
 
-	buffer->event = atomic_read(&attr_sd->s_event);
+	buffer->event = atomic_read(&attr_sd->s_attr.open->event);
 	count = ops->show(kobj, attr_sd->s_attr.attr, buffer->page);
 
 	sysfs_put_active_two(attr_sd);
@@ -301,6 +303,8 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
 		return -ENOMEM;
 
 	atomic_set(&new_od->refcnt, 0);
+	atomic_set(&new_od->event, 1);
+	init_waitqueue_head(&new_od->poll);
 	INIT_LIST_HEAD(&new_od->buffers);
 	goto retry;
 }
@@ -443,17 +447,17 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
 {
 	struct sysfs_buffer * buffer = filp->private_data;
 	struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata;
-	struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
+	struct sysfs_open_dirent *od = attr_sd->s_attr.open;
 
 	/* need parent for the kobj, grab both */
 	if (!sysfs_get_active_two(attr_sd))
 		goto trigger;
 
-	poll_wait(filp, &kobj->poll, wait);
+	poll_wait(filp, &od->poll, wait);
 
 	sysfs_put_active_two(attr_sd);
 
-	if (buffer->event != atomic_read(&attr_sd->s_event))
+	if (buffer->event != atomic_read(&od->event))
 		goto trigger;
 
 	return 0;
@@ -474,8 +478,17 @@ void sysfs_notify(struct kobject *k, char *dir, char *attr)
 	if (sd && attr)
 		sd = sysfs_find_dirent(sd, attr);
 	if (sd) {
-		atomic_inc(&sd->s_event);
-		wake_up_interruptible(&k->poll);
+		struct sysfs_open_dirent *od;
+
+		spin_lock(&sysfs_open_dirent_lock);
+
+		od = sd->s_attr.open;
+		if (od) {
+			atomic_inc(&od->event);
+			wake_up_interruptible(&od->poll);
+		}
+
+		spin_unlock(&sysfs_open_dirent_lock);
 	}
 
 	mutex_unlock(&sysfs_mutex);
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 3adce7d..269c845 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -46,7 +46,6 @@ struct sysfs_dirent {
 	ino_t			s_ino;
 	umode_t			s_mode;
 	struct iattr		*s_iattr;
-	atomic_t		s_event;
 };
 
 #define SD_DEACTIVATED_BIAS		INT_MIN
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index 0777b3f..a8a84fc 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -66,7 +66,6 @@ struct kobject {
 	struct kset		* kset;
 	struct kobj_type	* ktype;
 	struct sysfs_dirent	* sd;
-	wait_queue_head_t	poll;
 };
 
 extern int kobject_set_name(struct kobject *, const char *, ...)
diff --git a/lib/kobject.c b/lib/kobject.c
index 01f9c3b..a280c62 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -131,7 +131,6 @@ void kobject_init(struct kobject * kobj)
 		return;
 	kref_init(&kobj->kref);
 	INIT_LIST_HEAD(&kobj->entry);
-	init_waitqueue_head(&kobj->poll);
 	kobj->kset = kset_get(kobj->kset);
 }
 
-- 
1.5.0.3



  parent reply	other threads:[~2007-09-20  7:10 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-20  7:05 [PATCHSET 1/4] sysfs: misc updates Tejun Heo
2007-09-20  7:05 ` [PATCH 02/15] sysfs: fix comments of sysfs_add/remove_one() Tejun Heo
2007-09-20  7:05 ` [PATCH 01/15] sysfs: kill SYSFS_FLAG_REMOVED Tejun Heo
2007-09-25 21:32   ` Greg KH
2007-09-20  7:05 ` [PATCH 06/15] sysfs: reposition sysfs_dirent->s_mode Tejun Heo
2007-09-20  7:05 ` [PATCH 04/15] sysfs: clean up header files Tejun Heo
2007-09-20  7:05 ` [PATCH 03/15] sysfs: fix sysfs_chmod_file() such that it updates sd->s_mode too Tejun Heo
2007-09-20  7:05 ` [PATCH 07/15] sysfs: kill unnecessary sysfs_get() in open paths Tejun Heo
2007-09-20  7:05 ` [PATCH 08/15] sysfs: kill unnecessary NULL pointer check in sysfs_release() Tejun Heo
2007-09-20  7:05 ` [PATCH 09/15] sysfs: make bin attr open get active reference of parent too Tejun Heo
2007-09-20  7:05 ` [PATCH 05/15] sysfs: kill sysfs_update_file() Tejun Heo
2007-09-20  7:05 ` [PATCH 11/15] sysfs: open code sysfs_attach_dentry() Tejun Heo
2007-09-20  7:05 ` [PATCH 10/15] sysfs: make s_elem an anonymous union Tejun Heo
2007-09-20  7:05 ` [PATCH 12/15] sysfs: make sysfs_root a regular directory dirent Tejun Heo
2007-09-20  7:05 ` [PATCH 13/15] sysfs: move sysfs_dirent->s_children into sysfs_dirent->s_dir Tejun Heo
2007-09-20  7:05 ` Tejun Heo [this message]
2007-09-20  7:05 ` [PATCH 14/15] sysfs: implement sysfs_open_dirent Tejun Heo
2007-09-26 11:04 ` [PATCHSET 1/4] sysfs: misc updates Cornelia Huck
2007-09-26 15:19   ` Greg KH

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=11902719121259-git-send-email-htejun@gmail.com \
    --to=htejun@gmail.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=ebiederm@xmission.com \
    --cc=greg@kroah.com \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.