[PATCHSET 2.6.22-rc2-mm1 REVIEW] sysfs: make directory dentries/inodes reclaimable

[PATCHSET 2.6.22-rc2-mm1 REVIEW] sysfs: make directory dentries/inodes reclaimable

[Tejun Heo]

Hello, again.

THIS PATCHSET NEEDS MORE REVIEW AND TESTING.  PLEASE DO NOT APPLY YET.

This patchset makes directory dentries and inodes reclaimable and is
consisted of the following six patches.

#01: implement-sysfs-flags-and-SYSFS_FLAG_REMOVED
#02: implement-sysfs_find_dirent-and-sysfs_get_dirent
#03: make-kobj-point-to-sysfs_dirent-instead-of-dentry
#04: use-sysfs_lock-to-protect-the-sysfs_dirent-tree
#05: implement-sysfs_get_dentry
#06: make-directory-dentries-and-inodes-reclaimable

Patch #01 and #06 probably need more splitting and #04-06 definitely
need a lot more testing and review but the basic seems to work.  Now
having 10k sysfs files/directories cost slightly under 9 megabytes,
which isn't too bad and makes sysfs useable on wider range of systems.

API changes...

* kobj->dentry replaced with kobj->sd as dentry can go away
* shadowed directory handling functions now take sysfs_dirent instead
  of dentry

As dirent and dentry are confusing as hell, I'd like to rename
sysfs_dirent to sysfs_node or something.  Any better ideas?

Please review, test, scream... :-)

This patchset is on top of

  2.6.22-rc2-mm1
+ [1] sysfs-assorted-fixes patchset
+ [2] sysfs-reduce-memory-footprint-of-sysfs_dirent patchset

 fs/sysfs/bin.c          |    6 
 fs/sysfs/dir.c          |  590 +++++++++++++++++++++++++++++++-----------------
 fs/sysfs/file.c         |  196 +++++++--------
 fs/sysfs/group.c        |   54 ++--
 fs/sysfs/inode.c        |   36 +-
 fs/sysfs/mount.c        |    4 
 fs/sysfs/symlink.c      |   67 ++---
 fs/sysfs/sysfs.h        |   25 +-
 include/linux/kobject.h |    9 
 include/linux/sysfs.h   |   24 +
 lib/kobject.c           |   10 
 11 files changed, 603 insertions(+), 418 deletions(-)

Thanks.

--
tejun

[1] http://thread.gmane.org/gmane.linux.kernel/535372
[2] http://thread.gmane.org/gmane.linux.kernel/535379

[PATCH 1/6] sysfs: implement sysfs flags and SYSFS_FLAG_REMOVED

[Tejun Heo]

Rename sysfs_dirent->s_type to s_flags, pack type into lower eight
bits and use the rest for flags.  sysfs_type() can used to access the
type.  This patch also implements SYSFS_FLAG_REMOVED which is used to
improve sanity check in sysfs_deactivate().  The flag will also be
used to make directory entries reclamiable.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/dir.c        |   37 +++++++++++++++++++++++--------------
 fs/sysfs/inode.c      |    5 +++--
 fs/sysfs/mount.c      |    2 +-
 fs/sysfs/sysfs.h      |    7 ++++++-
 include/linux/sysfs.h |    4 ++++
 5 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b4074ad..4a2bd8b 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -171,7 +171,7 @@ void sysfs_deactivate(struct sysfs_diren
 	DECLARE_COMPLETION_ONSTACK(wait);
 	int v;
 
-	BUG_ON(sd->s_sibling);
+	BUG_ON(sd->s_sibling || !(sd->s_flags & SYSFS_FLAG_REMOVED));
 	sd->s_sibling = (void *)&wait;
 
 	/* atomic_add_return() is a mb(), put_active() will always see
@@ -218,9 +218,9 @@ void release_sysfs_dirent(struct sysfs_d
  repeat:
 	parent_sd = sd->s_parent;
 
-	if (sd->s_type & SYSFS_KOBJ_LINK)
+	if (sysfs_type(sd) & SYSFS_KOBJ_LINK)
 		sysfs_put(sd->s_elem.symlink.target_sd);
-	if (sd->s_type & SYSFS_COPY_NAME)
+	if (sysfs_type(sd) & SYSFS_COPY_NAME)
 		kfree(sd->s_name);
 	kfree(sd->s_iattr);
 	sysfs_free_ino(sd->s_ino);
@@ -282,7 +282,7 @@ struct sysfs_dirent *sysfs_new_dirent(co
 
 	sd->s_name = name;
 	sd->s_mode = mode;
-	sd->s_type = type;
+	sd->s_flags = type;
 
 	return sd;
 
@@ -330,7 +330,7 @@ int sysfs_dirent_exist(struct sysfs_dire
 	struct sysfs_dirent * sd;
 
 	for (sd = parent_sd->s_children; sd; sd = sd->s_sibling) {
-		if (sd->s_type) {
+		if (sysfs_type(sd)) {
 			if (strcmp(sd->s_name, new))
 				continue;
 			else
@@ -446,11 +446,12 @@ static struct dentry * sysfs_lookup(stru
 {
 	struct sysfs_dirent * parent_sd = dentry->d_parent->d_fsdata;
 	struct sysfs_dirent * sd;
+	struct bin_attribute *bin_attr;
 	struct inode *inode;
 	int found = 0;
 
 	for (sd = parent_sd->s_children; sd; sd = sd->s_sibling) {
-		if ((sd->s_type & SYSFS_NOT_PINNED) &&
+		if ((sysfs_type(sd) & SYSFS_NOT_PINNED) &&
 		    !strcmp(sd->s_name, dentry->d_name.name)) {
 			found = 1;
 			break;
@@ -468,16 +469,22 @@ static struct dentry * sysfs_lookup(stru
 
 	if (inode->i_state & I_NEW) {
 		/* initialize inode according to type */
-		if (sd->s_type & SYSFS_KOBJ_ATTR) {
+		switch (sysfs_type(sd)) {
+		case SYSFS_KOBJ_ATTR:
 			inode->i_size = PAGE_SIZE;
 			inode->i_fop = &sysfs_file_operations;
-		} else if (sd->s_type & SYSFS_KOBJ_BIN_ATTR) {
-			struct bin_attribute *bin_attr =
-				sd->s_elem.bin_attr.bin_attr;
+			break;
+		case SYSFS_KOBJ_BIN_ATTR:
+			bin_attr = sd->s_elem.bin_attr.bin_attr;
 			inode->i_size = bin_attr->size;
 			inode->i_fop = &bin_fops;
-		} else if (sd->s_type & SYSFS_KOBJ_LINK)
+			break;
+		case SYSFS_KOBJ_LINK:
 			inode->i_op = &sysfs_symlink_inode_operations;
+			break;
+		default:
+			BUG();
+		}
 	}
 
 	sysfs_instantiate(dentry, inode);
@@ -499,6 +506,7 @@ static void remove_dir(struct dentry * d
 	mutex_lock(&parent->d_inode->i_mutex);
 
 	sysfs_unlink_sibling(sd);
+	sd->s_flags |= SYSFS_FLAG_REMOVED;
 
 	pr_debug(" o %s removing done (%d)\n",d->d_name.name,
 		 atomic_read(&d->d_count));
@@ -532,7 +540,8 @@ static void __sysfs_remove_dir(struct de
 	while (*pos) {
 		struct sysfs_dirent *sd = *pos;
 
-		if (sd->s_type && (sd->s_type & SYSFS_NOT_PINNED)) {
+		if (sysfs_type(sd) && (sysfs_type(sd) & SYSFS_NOT_PINNED)) {
+			sd->s_flags |= SYSFS_FLAG_REMOVED;
 			*pos = sd->s_sibling;
 			sd->s_sibling = removed;
 			removed = sd;
@@ -775,7 +784,7 @@ static int sysfs_readdir(struct file * f
 				const char * name;
 				int len;
 
-				if (!next->s_type)
+				if (!sysfs_type(next))
 					continue;
 
 				name = next->s_name;
@@ -824,7 +833,7 @@ static loff_t sysfs_dir_lseek(struct fil
 			pos = &sd->s_children;
 			while (n && *pos) {
 				struct sysfs_dirent *next = *pos;
-				if (next->s_type)
+				if (sysfs_type(next))
 					n--;
 				pos = &(*pos)->s_sibling;
 			}
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 4d79a61..b67623d 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -264,7 +264,7 @@ void sysfs_drop_dentry(struct sysfs_dire
 	if (dentry) {
 		dentry->d_inode->i_ctime = curtime;
 		drop_nlink(dentry->d_inode);
-		if (sd->s_type & SYSFS_DIR) {
+		if (sysfs_type(sd) & SYSFS_DIR) {
 			drop_nlink(dentry->d_inode);
 			drop_nlink(dir);
 			/* XXX: unpin if directory, this will go away soon */
@@ -298,9 +298,10 @@ int sysfs_hash_and_remove(struct dentry
 	for (pos = &parent_sd->s_children; *pos; pos = &(*pos)->s_sibling) {
 		sd = *pos;
 
-		if (!sd->s_type)
+		if (!sysfs_type(sd))
 			continue;
 		if (!strcmp(sd->s_name, name)) {
+			sd->s_flags |= SYSFS_FLAG_REMOVED;
 			*pos = sd->s_sibling;
 			sd->s_sibling = NULL;
 			found = 1;
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 2e66701..d5ce3aa 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -24,7 +24,7 @@ static const struct super_operations sys
 
 static struct sysfs_dirent sysfs_root = {
 	.s_count	= ATOMIC_INIT(1),
-	.s_type		= SYSFS_ROOT,
+	.s_flags	= SYSFS_ROOT,
 	.s_mode		= S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
 	.s_ino		= 1,
 };
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 6f8aaf3..06b5085 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -34,7 +34,7 @@ struct sysfs_dirent {
 		struct sysfs_elem_bin_attr	bin_attr;
 	}			s_elem;
 
-	int			s_type;
+	unsigned int		s_flags;
 	umode_t			s_mode;
 	ino_t			s_ino;
 	struct dentry		* s_dentry;
@@ -86,6 +86,11 @@ extern const struct file_operations bin_
 extern const struct inode_operations sysfs_dir_inode_operations;
 extern const struct inode_operations sysfs_symlink_inode_operations;
 
+static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
+{
+	return sd->s_flags & SYSFS_TYPE_MASK;
+}
+
 static inline struct sysfs_dirent * sysfs_get(struct sysfs_dirent * sd)
 {
 	if (sd) {
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 161e19a..2a6df64 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -74,6 +74,7 @@ struct sysfs_ops {
 	ssize_t	(*store)(struct kobject *,struct attribute *,const char *, size_t);
 };
 
+#define SYSFS_TYPE_MASK		0x00ff
 #define SYSFS_ROOT		0x0001
 #define SYSFS_DIR		0x0002
 #define SYSFS_KOBJ_ATTR 	0x0004
@@ -82,6 +83,9 @@ struct sysfs_ops {
 #define SYSFS_NOT_PINNED	(SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)
 #define SYSFS_COPY_NAME		(SYSFS_DIR | SYSFS_KOBJ_LINK)
 
+#define SYSFS_FLAG_MASK		~SYSFS_TYPE_MASK
+#define SYSFS_FLAG_REMOVED	0x0100
+
 #ifdef CONFIG_SYSFS
 
 extern int sysfs_schedule_callback(struct kobject *kobj,
-- 
1.4.3.4

Re: [PATCH 1/6] sysfs: implement sysfs flags and SYSFS_FLAG_REMOVED

[Cornelia Huck]

On Tue, 29 May 2007 01:36:26 +0900,
Tejun Heo <htejun@gmail.com> wrote:

> Rename sysfs_dirent->s_type to s_flags, pack type into lower eight
> bits and use the rest for flags.  sysfs_type() can used to access the
> type.  This patch also implements SYSFS_FLAG_REMOVED which is used to
> improve sanity check in sysfs_deactivate().  The flag will also be
> used to make directory entries reclamiable.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  fs/sysfs/dir.c        |   37 +++++++++++++++++++++++--------------
>  fs/sysfs/inode.c      |    5 +++--
>  fs/sysfs/mount.c      |    2 +-
>  fs/sysfs/sysfs.h      |    7 ++++++-
>  include/linux/sysfs.h |    4 ++++
>  5 files changed, 37 insertions(+), 18 deletions(-)

This seems sane, but I'd split this into two patches: One introducing
sysfs_type(), and one adding SYSFS_FLAG_REMOVED.

Re: [PATCH 1/6] sysfs: implement sysfs flags and SYSFS_FLAG_REMOVED

[Tejun Heo]

Cornelia Huck wrote:
> On Tue, 29 May 2007 01:36:26 +0900,
> Tejun Heo <htejun@gmail.com> wrote:
> 
>> Rename sysfs_dirent->s_type to s_flags, pack type into lower eight
>> bits and use the rest for flags.  sysfs_type() can used to access the
>> type.  This patch also implements SYSFS_FLAG_REMOVED which is used to
>> improve sanity check in sysfs_deactivate().  The flag will also be
>> used to make directory entries reclamiable.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>> ---
>>  fs/sysfs/dir.c        |   37 +++++++++++++++++++++++--------------
>>  fs/sysfs/inode.c      |    5 +++--
>>  fs/sysfs/mount.c      |    2 +-
>>  fs/sysfs/sysfs.h      |    7 ++++++-
>>  include/linux/sysfs.h |    4 ++++
>>  5 files changed, 37 insertions(+), 18 deletions(-)
> 
> This seems sane, but I'd split this into two patches: One introducing
> sysfs_type(), and one adding SYSFS_FLAG_REMOVED.

Yeap, I would too.  :-)  I'll split them up on next posting.

-- 
tejun

[PATCH 3/6] sysfs: make kobj point to sysfs_dirent instead of dentry

[Tejun Heo]

As kobj sysfs dentries and inodes are gonna be made reclaimable,
dentry can't be used as naming token for sysfs file/directory, replace
kobj->dentry with kobj->sd.  The only external interface change is
shadow directory handling.  All other changes are contained in kobj
and sysfs.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/bin.c          |    6 +-
 fs/sysfs/dir.c          |  119 +++++++++++++++++++++++------------------------
 fs/sysfs/file.c         |   47 +++++++++---------
 fs/sysfs/group.c        |   54 ++++++++++-----------
 fs/sysfs/inode.c        |    9 +--
 fs/sysfs/symlink.c      |   22 ++++-----
 fs/sysfs/sysfs.h        |   10 ++--
 include/linux/kobject.h |    9 ++--
 include/linux/sysfs.h   |   19 ++++---
 lib/kobject.c           |   10 ++--
 10 files changed, 152 insertions(+), 153 deletions(-)

diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index 3c5574a..55796bd 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -234,9 +234,9 @@ const struct file_operations bin_fops =
 
 int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	BUG_ON(!kobj || !kobj->dentry || !attr);
+	BUG_ON(!kobj || !kobj->sd || !attr);
 
-	return sysfs_add_file(kobj->dentry, &attr->attr, SYSFS_KOBJ_BIN_ATTR);
+	return sysfs_add_file(kobj->sd, &attr->attr, SYSFS_KOBJ_BIN_ATTR);
 }
 
 
@@ -248,7 +248,7 @@ int sysfs_create_bin_file(struct kobject
 
 void sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	if (sysfs_hash_and_remove(kobj->dentry, attr->attr.name) < 0) {
+	if (sysfs_hash_and_remove(kobj->sd, attr->attr.name) < 0) {
 		printk(KERN_ERR "%s: "
 			"bad dentry or inode or no such file: \"%s\"\n",
 			__FUNCTION__, attr->attr.name);
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index c05d04a..58a92aa 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -368,9 +368,10 @@ struct sysfs_dirent *sysfs_get_dirent(st
 	return sd;
 }
 
-static int create_dir(struct kobject *kobj, struct dentry *parent,
-		      const char *name, struct dentry **p_dentry)
+static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
+		      const char *name, struct sysfs_dirent **p_sd)
 {
+	struct dentry *parent = parent_sd->s_dentry;
 	int error;
 	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
 	struct dentry *dentry;
@@ -409,14 +410,14 @@ static int create_dir(struct kobject *ko
 
 	/* link in */
 	error = -EEXIST;
-	if (sysfs_find_dirent(parent->d_fsdata, name))
+	if (sysfs_find_dirent(parent_sd, name))
 		goto out_iput;
 
 	sysfs_instantiate(dentry, inode);
 	inc_nlink(parent->d_inode);
-	sysfs_attach_dirent(sd, parent->d_fsdata, dentry);
+	sysfs_attach_dirent(sd, parent_sd, dentry);
 
-	*p_dentry = dentry;
+	*p_sd = sd;
 	error = 0;
 	goto out_unlock;	/* pin directory dentry in core */
 
@@ -433,38 +434,37 @@ static int create_dir(struct kobject *ko
 	return error;
 }
 
-
-int sysfs_create_subdir(struct kobject * k, const char * n, struct dentry ** d)
+int sysfs_create_subdir(struct kobject *kobj, const char *name,
+			struct sysfs_dirent **p_sd)
 {
-	return create_dir(k,k->dentry,n,d);
+	return create_dir(kobj, kobj->sd, name, p_sd);
 }
 
 /**
  *	sysfs_create_dir - create a directory for an object.
  *	@kobj:		object we're creating directory for. 
- *	@shadow_parent:	parent parent object.
+ *	@shadow_parent:	parent object.
  */
-
-int sysfs_create_dir(struct kobject * kobj, struct dentry *shadow_parent)
+int sysfs_create_dir(struct kobject * kobj,
+		     struct sysfs_dirent *shadow_parent_sd)
 {
-	struct dentry * dentry = NULL;
-	struct dentry * parent;
+	struct sysfs_dirent *parent_sd, *sd;
 	int error = 0;
 
 	BUG_ON(!kobj);
 
-	if (shadow_parent)
-		parent = shadow_parent;
+	if (shadow_parent_sd)
+		parent_sd = shadow_parent_sd;
 	else if (kobj->parent)
-		parent = kobj->parent->dentry;
+		parent_sd = kobj->parent->sd;
 	else if (sysfs_mount && sysfs_mount->mnt_sb)
-		parent = sysfs_mount->mnt_sb->s_root;
+		parent_sd = sysfs_mount->mnt_sb->s_root->d_fsdata;
 	else
 		return -EFAULT;
 
-	error = create_dir(kobj,parent,kobject_name(kobj),&dentry);
+	error = create_dir(kobj, parent_sd, kobject_name(kobj), &sd);
 	if (!error)
-		kobj->dentry = dentry;
+		kobj->sd = sd;
 	return error;
 }
 
@@ -525,18 +525,16 @@ const struct inode_operations sysfs_dir_
 	.setattr	= sysfs_setattr,
 };
 
-static void remove_dir(struct dentry * d)
+static void remove_dir(struct sysfs_dirent *sd)
 {
-	struct dentry *parent = d->d_parent;
-	struct sysfs_dirent *sd = d->d_fsdata;
+	struct dentry *parent = sd->s_parent->s_dentry;
 
 	mutex_lock(&parent->d_inode->i_mutex);
 
 	sysfs_unlink_sibling(sd);
 	sd->s_flags |= SYSFS_FLAG_REMOVED;
 
-	pr_debug(" o %s removing done (%d)\n",d->d_name.name,
-		 atomic_read(&d->d_count));
+	pr_debug(" o %s removing done\n", sd->s_name);
 
 	mutex_unlock(&parent->d_inode->i_mutex);
 
@@ -545,25 +543,26 @@ static void remove_dir(struct dentry * d
 	sysfs_put(sd);
 }
 
-void sysfs_remove_subdir(struct dentry * d)
+void sysfs_remove_subdir(struct sysfs_dirent *sd)
 {
-	remove_dir(d);
+	remove_dir(sd);
 }
 
 
-static void __sysfs_remove_dir(struct dentry *dentry)
+static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
 {
 	struct sysfs_dirent *removed = NULL;
-	struct sysfs_dirent *parent_sd;
 	struct sysfs_dirent **pos;
+	struct dentry *dir;
 
-	if (!dentry)
+	if (!dir_sd)
 		return;
 
-	pr_debug("sysfs %s: removing dir\n",dentry->d_name.name);
-	mutex_lock(&dentry->d_inode->i_mutex);
-	parent_sd = dentry->d_fsdata;
-	pos = &parent_sd->s_children;
+	dir = dir_sd->s_dentry;
+
+	pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
+	mutex_lock(&dir->d_inode->i_mutex);
+	pos = &dir_sd->s_children;
 	while (*pos) {
 		struct sysfs_dirent *sd = *pos;
 
@@ -575,7 +574,7 @@ static void __sysfs_remove_dir(struct de
 		} else
 			pos = &(*pos)->s_sibling;
 	}
-	mutex_unlock(&dentry->d_inode->i_mutex);
+	mutex_unlock(&dir->d_inode->i_mutex);
 
 	while (removed) {
 		struct sysfs_dirent *sd = removed;
@@ -588,7 +587,7 @@ static void __sysfs_remove_dir(struct de
 		sysfs_put(sd);
 	}
 
-	remove_dir(dentry);
+	remove_dir(dir_sd);
 }
 
 /**
@@ -602,25 +601,25 @@ static void __sysfs_remove_dir(struct de
 
 void sysfs_remove_dir(struct kobject * kobj)
 {
-	struct dentry *d = kobj->dentry;
+	struct sysfs_dirent *sd = kobj->sd;
 
 	spin_lock(&kobj_sysfs_assoc_lock);
-	kobj->dentry = NULL;
+	kobj->sd = NULL;
 	spin_unlock(&kobj_sysfs_assoc_lock);
 
-	__sysfs_remove_dir(d);
+	__sysfs_remove_dir(sd);
 }
 
-int sysfs_rename_dir(struct kobject * kobj, struct dentry *new_parent,
+int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 		     const char *new_name)
 {
-	struct sysfs_dirent *sd = kobj->dentry->d_fsdata;
-	struct sysfs_dirent *parent_sd = new_parent->d_fsdata;
+	struct sysfs_dirent *sd = kobj->sd;
+	struct dentry *new_parent = new_parent_sd->s_dentry;
 	struct dentry *new_dentry;
 	char *dup_name;
 	int error;
 
-	if (!new_parent)
+	if (!new_parent_sd)
 		return -EFAULT;
 
 	down_write(&sysfs_rename_sem);
@@ -637,9 +636,9 @@ int sysfs_rename_dir(struct kobject * ko
 	 * shadows of the same directory
 	 */
 	error = -EINVAL;
-	if (kobj->dentry->d_parent->d_inode != new_parent->d_inode ||
+	if (sd->s_parent->s_dentry->d_inode != new_parent->d_inode ||
 	    new_dentry->d_parent->d_inode != new_parent->d_inode ||
-	    new_dentry == kobj->dentry)
+	    new_dentry == sd->s_dentry)
 		goto out_dput;
 
 	error = -EEXIST;
@@ -661,12 +660,12 @@ int sysfs_rename_dir(struct kobject * ko
 
 	/* move under the new parent */
 	d_add(new_dentry, NULL);
-	d_move(kobj->dentry, new_dentry);
+	d_move(sd->s_dentry, new_dentry);
 
 	sysfs_unlink_sibling(sd);
-	sysfs_get(parent_sd);
+	sysfs_get(new_parent_sd);
 	sysfs_put(sd->s_parent);
-	sd->s_parent = parent_sd;
+	sd->s_parent = new_parent_sd;
 	sysfs_link_sibling(sd);
 
 	error = 0;
@@ -691,9 +690,9 @@ int sysfs_move_dir(struct kobject *kobj,
 	int error;
 
 	old_parent_dentry = kobj->parent ?
-		kobj->parent->dentry : sysfs_mount->mnt_sb->s_root;
+		kobj->parent->sd->s_dentry : sysfs_mount->mnt_sb->s_root;
 	new_parent_dentry = new_parent ?
-		new_parent->dentry : sysfs_mount->mnt_sb->s_root;
+		new_parent->sd->s_dentry : sysfs_mount->mnt_sb->s_root;
 
 	if (old_parent_dentry->d_inode == new_parent_dentry->d_inode)
 		return 0;	/* nothing to move */
@@ -705,7 +704,7 @@ again:
 	}
 
 	new_parent_sd = new_parent_dentry->d_fsdata;
-	sd = kobj->dentry->d_fsdata;
+	sd = kobj->sd;
 
 	new_dentry = lookup_one_len(kobj->name, new_parent_dentry,
 				    strlen(kobj->name));
@@ -715,7 +714,7 @@ again:
 	} else
 		error = 0;
 	d_add(new_dentry, NULL);
-	d_move(kobj->dentry, new_dentry);
+	d_move(sd->s_dentry, new_dentry);
 	dput(new_dentry);
 
 	/* Remove from old parent's list and insert into new parent's list. */
@@ -885,7 +884,7 @@ int sysfs_make_shadowed_dir(struct kobje
 	struct inode *inode;
 	struct inode_operations *i_op;
 
-	inode = kobj->dentry->d_inode;
+	inode = kobj->sd->s_dentry->d_inode;
 	if (inode->i_op != &sysfs_dir_inode_operations)
 		return -EINVAL;
 
@@ -912,16 +911,16 @@ int sysfs_make_shadowed_dir(struct kobje
  *	directory.
  */
 
-struct dentry *sysfs_create_shadow_dir(struct kobject *kobj)
+struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj)
 {
-	struct dentry *dir = kobj->dentry;
+	struct dentry *dir = kobj->sd->s_dentry;
 	struct inode *inode = dir->d_inode;
 	struct dentry *parent = dir->d_parent;
 	struct sysfs_dirent *parent_sd = parent->d_fsdata;
 	struct dentry *shadow;
 	struct sysfs_dirent *sd;
 
-	shadow = ERR_PTR(-EINVAL);
+	sd = ERR_PTR(-EINVAL);
 	if (!sysfs_is_shadowed_inode(inode))
 		goto out;
 
@@ -944,25 +943,25 @@ struct dentry *sysfs_create_shadow_dir(s
 	dget(shadow);		/* Extra count - pin the dentry in core */
 
 out:
-	return shadow;
+	return sd;
 nomem:
 	dput(shadow);
-	shadow = ERR_PTR(-ENOMEM);
+	sd = ERR_PTR(-ENOMEM);
 	goto out;
 }
 
 /**
  *	sysfs_remove_shadow_dir - remove an object's directory.
- *	@shadow: dentry of shadow directory
+ *	@shadow_sd: sysfs_dirent of shadow directory
  *
  *	The only thing special about this is that we remove any files in
  *	the directory before we remove the directory, and we've inlined
  *	what used to be sysfs_rmdir() below, instead of calling separately.
  */
 
-void sysfs_remove_shadow_dir(struct dentry *shadow)
+void sysfs_remove_shadow_dir(struct sysfs_dirent *shadow_sd)
 {
-	__sysfs_remove_dir(shadow);
+	__sysfs_remove_dir(shadow_sd);
 }
 
 const struct file_operations sysfs_dir_operations = {
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index e448b88..acc9890 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -385,7 +385,7 @@ static struct dentry *step_down(struct d
 
 void sysfs_notify(struct kobject * k, char *dir, char *attr)
 {
-	struct dentry *de = k->dentry;
+	struct dentry *de = k->sd->s_dentry;
 	if (de)
 		dget(de);
 	if (de && dir)
@@ -412,16 +412,17 @@ const struct file_operations sysfs_file_
 };
 
 
-int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
+int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
+		   int type)
 {
-	struct sysfs_dirent * parent_sd = dir->d_fsdata;
+	struct dentry *dir = dir_sd->s_dentry;
 	umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
 	struct sysfs_dirent *sd;
 	int error = 0;
 
 	mutex_lock(&dir->d_inode->i_mutex);
 
-	if (sysfs_find_dirent(parent_sd, attr->name)) {
+	if (sysfs_find_dirent(dir_sd, attr->name)) {
 		error = -EEXIST;
 		goto out_unlock;
 	}
@@ -432,7 +433,7 @@ int sysfs_add_file(struct dentry * dir,
 		goto out_unlock;
 	}
 	sd->s_elem.attr.attr = (void *)attr;
-	sysfs_attach_dirent(sd, parent_sd, NULL);
+	sysfs_attach_dirent(sd, dir_sd, NULL);
 
  out_unlock:
 	mutex_unlock(&dir->d_inode->i_mutex);
@@ -448,9 +449,9 @@ int sysfs_add_file(struct dentry * dir,
 
 int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
 {
-	BUG_ON(!kobj || !kobj->dentry || !attr);
+	BUG_ON(!kobj || !kobj->sd || !attr);
 
-	return sysfs_add_file(kobj->dentry, attr, SYSFS_KOBJ_ATTR);
+	return sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
 
 }
 
@@ -464,16 +465,16 @@ int sysfs_create_file(struct kobject * k
 int sysfs_add_file_to_group(struct kobject *kobj,
 		const struct attribute *attr, const char *group)
 {
-	struct dentry *dir;
+	struct sysfs_dirent *dir_sd;
 	int error;
 
-	dir = lookup_one_len(group, kobj->dentry, strlen(group));
-	if (IS_ERR(dir))
-		error = PTR_ERR(dir);
-	else {
-		error = sysfs_add_file(dir, attr, SYSFS_KOBJ_ATTR);
-		dput(dir);
-	}
+	dir_sd = sysfs_get_dirent(kobj->sd, group);
+	if (!dir_sd)
+		return -ENOENT;
+
+	error = sysfs_add_file(dir_sd, attr, SYSFS_KOBJ_ATTR);
+	sysfs_put(dir_sd);
+
 	return error;
 }
 EXPORT_SYMBOL_GPL(sysfs_add_file_to_group);
@@ -486,7 +487,7 @@ EXPORT_SYMBOL_GPL(sysfs_add_file_to_grou
  */
 int sysfs_update_file(struct kobject * kobj, const struct attribute * attr)
 {
-	struct dentry * dir = kobj->dentry;
+	struct dentry * dir = kobj->sd->s_dentry;
 	struct dentry * victim;
 	int res = -ENOENT;
 
@@ -522,7 +523,7 @@ int sysfs_update_file(struct kobject * k
  */
 int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)
 {
-	struct dentry *dir = kobj->dentry;
+	struct dentry *dir = kobj->sd->s_dentry;
 	struct dentry *victim;
 	struct inode * inode;
 	struct iattr newattrs;
@@ -560,7 +561,7 @@ EXPORT_SYMBOL_GPL(sysfs_chmod_file);
 
 void sysfs_remove_file(struct kobject * kobj, const struct attribute * attr)
 {
-	sysfs_hash_and_remove(kobj->dentry, attr->name);
+	sysfs_hash_and_remove(kobj->sd, attr->name);
 }
 
 
@@ -573,12 +574,12 @@ void sysfs_remove_file(struct kobject *
 void sysfs_remove_file_from_group(struct kobject *kobj,
 		const struct attribute *attr, const char *group)
 {
-	struct dentry *dir;
+	struct sysfs_dirent *dir_sd;
 
-	dir = lookup_one_len(group, kobj->dentry, strlen(group));
-	if (!IS_ERR(dir)) {
-		sysfs_hash_and_remove(dir, attr->name);
-		dput(dir);
+	dir_sd = sysfs_get_dirent(kobj->sd, group);
+	if (dir_sd) {
+		sysfs_hash_and_remove(dir_sd, attr->name);
+		sysfs_put(dir_sd);
 	}
 }
 EXPORT_SYMBOL_GPL(sysfs_remove_file_from_group);
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 52eed2a..383946e 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -18,26 +18,25 @@
 #include "sysfs.h"
 
 
-static void remove_files(struct dentry * dir, 
-			 const struct attribute_group * grp)
+static void remove_files(struct sysfs_dirent *dir_sd,
+			 const struct attribute_group *grp)
 {
 	struct attribute *const* attr;
 
 	for (attr = grp->attrs; *attr; attr++)
-		sysfs_hash_and_remove(dir,(*attr)->name);
+		sysfs_hash_and_remove(dir_sd, (*attr)->name);
 }
 
-static int create_files(struct dentry * dir,
-			const struct attribute_group * grp)
+static int create_files(struct sysfs_dirent *dir_sd,
+			const struct attribute_group *grp)
 {
 	struct attribute *const* attr;
 	int error = 0;
 
-	for (attr = grp->attrs; *attr && !error; attr++) {
-		error = sysfs_add_file(dir, *attr, SYSFS_KOBJ_ATTR);
-	}
+	for (attr = grp->attrs; *attr && !error; attr++)
+		error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
 	if (error)
-		remove_files(dir,grp);
+		remove_files(dir_sd, grp);
 	return error;
 }
 
@@ -45,44 +44,43 @@ static int create_files(struct dentry *
 int sysfs_create_group(struct kobject * kobj, 
 		       const struct attribute_group * grp)
 {
-	struct dentry * dir;
+	struct sysfs_dirent *sd;
 	int error;
 
-	BUG_ON(!kobj || !kobj->dentry);
+	BUG_ON(!kobj || !kobj->sd);
 
 	if (grp->name) {
-		error = sysfs_create_subdir(kobj,grp->name,&dir);
+		error = sysfs_create_subdir(kobj, grp->name, &sd);
 		if (error)
 			return error;
 	} else
-		dir = kobj->dentry;
-	dir = dget(dir);
-	if ((error = create_files(dir,grp))) {
+		sd = kobj->sd;
+	sysfs_get(sd);
+	if ((error = create_files(sd, grp))) {
 		if (grp->name)
-			sysfs_remove_subdir(dir);
+			sysfs_remove_subdir(sd);
 	}
-	dput(dir);
+	sysfs_put(sd);
 	return error;
 }
 
 void sysfs_remove_group(struct kobject * kobj, 
 			const struct attribute_group * grp)
 {
-	struct dentry * dir;
+	struct sysfs_dirent *dir_sd = kobj->sd;
+	struct sysfs_dirent *sd;
 
 	if (grp->name) {
-		dir = lookup_one_len_kern(grp->name, kobj->dentry,
-				strlen(grp->name));
-		BUG_ON(IS_ERR(dir));
-	}
-	else
-		dir = dget(kobj->dentry);
+		sd = sysfs_get_dirent(dir_sd, grp->name);
+		BUG_ON(!sd);
+	} else
+		sd = sysfs_get(dir_sd);
 
-	remove_files(dir,grp);
+	remove_files(sd, grp);
 	if (grp->name)
-		sysfs_remove_subdir(dir);
-	/* release the ref. taken in this routine */
-	dput(dir);
+		sysfs_remove_subdir(sd);
+
+	sysfs_put(sd);
 }
 
 
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index b67623d..4077ab5 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -281,21 +281,18 @@ void sysfs_drop_dentry(struct sysfs_dire
 		dput(parent);
 }
 
-int sysfs_hash_and_remove(struct dentry * dir, const char * name)
+int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
 {
+	struct dentry *dir = dir_sd->s_dentry;
 	struct sysfs_dirent **pos, *sd;
-	struct sysfs_dirent *parent_sd = dir->d_fsdata;
 	int found = 0;
 
-	if (!dir)
-		return -ENOENT;
-
 	if (dir->d_inode == NULL)
 		/* no inode means this hasn't been made visible yet */
 		return -ENOENT;
 
 	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
-	for (pos = &parent_sd->s_children; *pos; pos = &(*pos)->s_sibling) {
+	for (pos = &dir_sd->s_children; *pos; pos = &(*pos)->s_sibling) {
 		sd = *pos;
 
 		if (!sysfs_type(sd))
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 45b62e2..43cc522 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -66,7 +66,6 @@ static int sysfs_add_link(struct sysfs_d
  */
 int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char * name)
 {
-	struct dentry *dentry = NULL;
 	struct sysfs_dirent *parent_sd = NULL;
 	struct sysfs_dirent *target_sd = NULL;
 	int error = -EEXIST;
@@ -75,29 +74,28 @@ int sysfs_create_link(struct kobject * k
 
 	if (!kobj) {
 		if (sysfs_mount && sysfs_mount->mnt_sb)
-			dentry = sysfs_mount->mnt_sb->s_root;
+			parent_sd = sysfs_mount->mnt_sb->s_root->d_fsdata;
 	} else
-		dentry = kobj->dentry;
+		parent_sd = kobj->sd;
 
-	if (!dentry)
+	if (!parent_sd)
 		return -EFAULT;
-	parent_sd = dentry->d_fsdata;
 
-	/* target->dentry can go away beneath us but is protected with
+	/* target->sd can go away beneath us but is protected with
 	 * kobj_sysfs_assoc_lock.  Fetch target_sd from it.
 	 */
 	spin_lock(&kobj_sysfs_assoc_lock);
-	if (target->dentry)
-		target_sd = sysfs_get(target->dentry->d_fsdata);
+	if (target->sd)
+		target_sd = sysfs_get(target->sd);
 	spin_unlock(&kobj_sysfs_assoc_lock);
 
 	if (!target_sd)
 		return -ENOENT;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
-	if (!sysfs_find_dirent(dentry->d_fsdata, name))
+	mutex_lock(&parent_sd->s_dentry->d_inode->i_mutex);
+	if (!sysfs_find_dirent(parent_sd, name))
 		error = sysfs_add_link(parent_sd, name, target_sd);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+	mutex_unlock(&parent_sd->s_dentry->d_inode->i_mutex);
 
 	if (error)
 		sysfs_put(target_sd);
@@ -114,7 +112,7 @@ int sysfs_create_link(struct kobject * k
 
 void sysfs_remove_link(struct kobject * kobj, const char * name)
 {
-	sysfs_hash_and_remove(kobj->dentry,name);
+	sysfs_hash_and_remove(kobj->sd, name);
 }
 
 static int sysfs_get_target_path(struct sysfs_dirent * parent_sd,
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index f1629b4..27a5f4b 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -69,12 +69,14 @@ extern void sysfs_attach_dirent(struct s
 				struct sysfs_dirent *parent_sd,
 				struct dentry *dentry);
 
-extern int sysfs_add_file(struct dentry *, const struct attribute *, int);
-extern int sysfs_hash_and_remove(struct dentry * dir, const char * name);
+extern int sysfs_add_file(struct sysfs_dirent *dir_sd,
+			  const struct attribute *attr, int type);
+extern int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name);
 extern struct sysfs_dirent *sysfs_find(struct sysfs_dirent *dir, const char * name);
 
-extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
-extern void sysfs_remove_subdir(struct dentry *);
+extern int sysfs_create_subdir(struct kobject *kobj, const char *name,
+			       struct sysfs_dirent **p_sd);
+extern void sysfs_remove_subdir(struct sysfs_dirent *sd);
 
 extern void sysfs_drop_dentry(struct sysfs_dirent *sd);
 extern int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index c288e41..06cbf41 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -55,7 +55,7 @@ struct kobject {
 	struct kobject		* parent;
 	struct kset		* kset;
 	struct kobj_type	* ktype;
-	struct dentry		* dentry;
+	struct sysfs_dirent	* sd;
 	wait_queue_head_t	poll;
 };
 
@@ -71,13 +71,14 @@ extern void kobject_init(struct kobject
 extern void kobject_cleanup(struct kobject *);
 
 extern int __must_check kobject_add(struct kobject *);
-extern int __must_check kobject_shadow_add(struct kobject *, struct dentry *);
+extern int __must_check kobject_shadow_add(struct kobject *kobj,
+					   struct sysfs_dirent *shadow_parent);
 extern void kobject_del(struct kobject *);
 
 extern int __must_check kobject_rename(struct kobject *, const char *new_name);
 extern int __must_check kobject_shadow_rename(struct kobject *kobj,
-						struct dentry *new_parent,
-						const char *new_name);
+					      struct sysfs_dirent *new_parent,
+					      const char *new_name);
 extern int __must_check kobject_move(struct kobject *, struct kobject *);
 
 extern int __must_check kobject_register(struct kobject *);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 2a6df64..4c43030 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -19,6 +19,7 @@ struct kobject;
 struct module;
 struct nameidata;
 struct dentry;
+struct sysfs_dirent;
 
 /* FIXME
  * The *owner field is no longer used, but leave around
@@ -92,13 +93,14 @@ extern int sysfs_schedule_callback(struc
 		void (*func)(void *), void *data, struct module *owner);
 
 extern int __must_check
-sysfs_create_dir(struct kobject *, struct dentry *);
+sysfs_create_dir(struct kobject *kobj, struct sysfs_dirent *shadow_parent_sd);
 
 extern void
 sysfs_remove_dir(struct kobject *);
 
 extern int __must_check
-sysfs_rename_dir(struct kobject *, struct dentry *, const char *new_name);
+sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
+		 const char *new_name);
 
 extern int __must_check
 sysfs_move_dir(struct kobject *, struct kobject *);
@@ -138,8 +140,8 @@ void sysfs_notify(struct kobject * k, ch
 
 extern int sysfs_make_shadowed_dir(struct kobject *kobj,
 	void * (*follow_link)(struct dentry *, struct nameidata *));
-extern struct dentry *sysfs_create_shadow_dir(struct kobject *kobj);
-extern void sysfs_remove_shadow_dir(struct dentry *dir);
+extern struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj);
+extern void sysfs_remove_shadow_dir(struct sysfs_dirent *shadow_sd);
 
 extern int __must_check sysfs_init(void);
 
@@ -151,7 +153,8 @@ static inline int sysfs_schedule_callbac
 	return -ENOSYS;
 }
 
-static inline int sysfs_create_dir(struct kobject * k, struct dentry *shadow)
+static inline int sysfs_create_dir(struct kobject *kobj,
+				   struct sysfs_dirent *shadow_parent_sd)
 {
 	return 0;
 }
@@ -161,9 +164,9 @@ static inline void sysfs_remove_dir(stru
 	;
 }
 
-static inline int sysfs_rename_dir(struct kobject * k,
-					struct dentry *new_parent,
-					const char *new_name)
+static inline int sysfs_rename_dir(struct kobject *kobj,
+				   struct sysfs_dirent *new_parent_sd,
+				   const char *new_name)
 {
 	return 0;
 }
diff --git a/lib/kobject.c b/lib/kobject.c
index a85708c..2d26f07 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -46,7 +46,7 @@ static int populate_dir(struct kobject *
 	return error;
 }
 
-static int create_dir(struct kobject * kobj, struct dentry *shadow_parent)
+static int create_dir(struct kobject * kobj, struct sysfs_dirent *shadow_parent)
 {
 	int error = 0;
 	if (kobject_name(kobj)) {
@@ -206,7 +206,7 @@ static void unlink(struct kobject * kobj
  *	@shadow_parent: sysfs directory to add to.
  */
 
-int kobject_shadow_add(struct kobject * kobj, struct dentry *shadow_parent)
+int kobject_shadow_add(struct kobject *kobj, struct sysfs_dirent *shadow_parent)
 {
 	int error = 0;
 	struct kobject * parent;
@@ -382,7 +382,7 @@ int kobject_rename(struct kobject * kobj
 	/* Note : if we want to send the new name alone, not the full path,
 	 * we could probably use kobject_name(kobj); */
 
-	error = sysfs_rename_dir(kobj, kobj->parent->dentry, new_name);
+	error = sysfs_rename_dir(kobj, kobj->parent->sd, new_name);
 
 	/* This function is mostly/only used for network interface.
 	 * Some hotplug package track interfaces by their name and
@@ -405,8 +405,8 @@ out:
  *	@new_name: object's new name
  */
 
-int kobject_shadow_rename(struct kobject * kobj, struct dentry *new_parent,
-			  const char *new_name)
+int kobject_shadow_rename(struct kobject * kobj,
+			  struct sysfs_dirent *new_parent, const char *new_name)
 {
 	int error = 0;
 
-- 
1.4.3.4

[PATCH 5/6] sysfs: implement sysfs_get_dentry()

[Tejun Heo]

Some sysfs operations require dentry and inode.  sysfs_get_dentry()
looks up and gets dentry for the specified sysfs_dirent.  It finds the
first ancestor with dentry attached and starts looking up dentries
from there.

Looking up from the nearest ancestor is necessary to support shadowed
directories because we can't reliably lookup dentry for one of the
shadows.  Dentries for each shadow will be pinned in memory such that
they can serve as the starting point for dentry lookup.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/dir.c   |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/sysfs/sysfs.h |    1 +
 2 files changed, 93 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index a474e54..0242cbd 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -63,6 +63,98 @@ static void sysfs_unlink_sibling(struct
 }
 
 /**
+ *	sysfs_get_dentry - get dentry for the given sysfs_dirent
+ *	@sd: sysfs_dirent of interest
+ *
+ *	Get dentry for @sd.  Dentry is looked up if currently not
+ *	present.  This function climbs sysfs_dirent tree till it
+ *	reaches a sysfs_dirent with valid dentry attached and descends
+ *	down from there looking up dentry for each step.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	Pointer to found dentry on success, ERR_PTR() value on error.
+ */
+struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd)
+{
+	struct sysfs_dirent *cur;
+	struct dentry *parent_dentry, *dentry;
+	int i, depth;
+
+	/* Find the first parent which has valid s_dentry and get the
+	 * dentry.
+	 */
+	spin_lock(&sysfs_lock);
+ restart:
+	spin_lock(&dcache_lock);
+
+	dentry = NULL;
+	depth = 0;
+	cur = sd;
+	while (!cur->s_dentry || !cur->s_dentry->d_inode) {
+		if (cur->s_flags & SYSFS_FLAG_REMOVED) {
+			dentry = ERR_PTR(-ENOENT);
+			depth = 0;
+			break;
+		}
+		cur = cur->s_parent;
+		depth++;
+	}
+	if (!IS_ERR(dentry))
+		dentry = dget_locked(cur->s_dentry);
+
+	spin_unlock(&dcache_lock);
+
+	/* from the found dentry, look up depth times */
+	while (depth--) {
+		/* find and get depth'th ancestor */
+		for (cur = sd, i = 0; cur && i < depth; i++)
+			cur = cur->s_parent;
+
+		/* This can happen if tree structure was modified due
+		 * to move/rename.  Restart.
+		 */
+		if (i != depth) {
+			dput(dentry);
+			goto restart;
+		}
+
+		sysfs_get(cur);
+
+		spin_unlock(&sysfs_lock);
+
+		/* look it up */
+		parent_dentry = dentry;
+		dentry = lookup_one_len_kern(cur->s_name, parent_dentry,
+					     strlen(cur->s_name));
+		dput(parent_dentry);
+
+		if (IS_ERR(dentry)) {
+			sysfs_put(cur);
+			return dentry;
+		}
+
+		spin_lock(&sysfs_lock);
+
+		/* This, again, can happen if tree structure has
+		 * changed and we looked up the wrong thing.  Restart.
+		 */
+		if (cur->s_dentry != dentry) {
+			dput(dentry);
+			sysfs_put(cur);
+			goto restart;
+		}
+
+		sysfs_put(cur);
+	}
+
+	spin_unlock(&sysfs_lock);
+	return dentry;
+}
+
+/**
  *	sysfs_get_active - get an active reference to sysfs_dirent
  *	@sd: sysfs_dirent to get an active reference to
  *
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 476d58b..b11c448 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -47,6 +47,7 @@ struct sysfs_dirent {
 extern struct vfsmount * sysfs_mount;
 extern struct kmem_cache *sysfs_dir_cachep;
 
+extern struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd);
 extern struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd);
 extern void sysfs_put_active(struct sysfs_dirent *sd);
 extern struct sysfs_dirent *sysfs_get_active_two(struct sysfs_dirent *sd);
-- 
1.4.3.4

[PATCH 6/6] sysfs: make directory dentries and inodes reclaimable

[Tejun Heo]

This patch makes dentries and inodes for sysfs directories
reclaimable.

* sysfs_notify() is modified to walk sysfs_dirent tree instead of
  dentry tree.
* sysfs_update_file() and sysfs_chmod_file() use sysfs_get_dentry() to
  grab the victim dentry.
* sysfs_rename_dir() and sysfs_move_dir() grab all dentries using
  sysfs_get_dentry() on startup.
* Dentries for all shadowed directories are pinned in memory to serve
  as lookup start point.
* parent mtime/ctime update on inode instantiation is dropped.  I' not
  sure what to do about it yet.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/dir.c        |  238 ++++++++++++++++++++++++++++---------------------
 fs/sysfs/file.c       |  134 ++++++++++++---------------
 fs/sysfs/inode.c      |   15 +--
 fs/sysfs/mount.c      |    2 +-
 fs/sysfs/sysfs.h      |    1 +
 include/linux/sysfs.h |    1 -
 6 files changed, 202 insertions(+), 189 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 0242cbd..ae1f379 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -482,73 +482,46 @@ struct sysfs_dirent *sysfs_get_dirent(st
 static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
 		      const char *name, struct sysfs_dirent **p_sd)
 {
-	struct dentry *parent = parent_sd->s_dentry;
-	int error;
 	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
-	struct dentry *dentry;
-	struct inode *inode;
+	struct dentry *parent_dentry = NULL;
 	struct sysfs_dirent *sd;
-
-	mutex_lock(&parent->d_inode->i_mutex);
+	int error;
 
 	/* allocate */
-	dentry = lookup_one_len(name, parent, strlen(name));
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_unlock;
-	}
-
-	error = -EEXIST;
-	if (dentry->d_inode)
-		goto out_dput;
-
-	error = -ENOMEM;
 	sd = sysfs_new_dirent(name, mode, SYSFS_DIR);
 	if (!sd)
-		goto out_drop;
+		return -ENOMEM;
 	sd->s_elem.dir.kobj = kobj;
 
-	inode = sysfs_get_inode(sd);
-	if (!inode)
-		goto out_sput;
-
-	if (inode->i_state & I_NEW) {
-		inode->i_op = &sysfs_dir_inode_operations;
-		inode->i_fop = &sysfs_dir_operations;
-		/* directory inodes start off with i_nlink == 2 (for ".") */
-		inc_nlink(inode);
-	}
-
 	/* link in */
+	error = -EEXIST;
 	spin_lock(&sysfs_lock);
+	if (!sysfs_find_dirent(parent_sd, name)) {
+		sysfs_attach_dirent(sd, parent_sd, NULL);
+		error = 0;
 
-	error = -EEXIST;
-	if (sysfs_find_dirent(parent_sd, name)) {
-		spin_unlock(&sysfs_lock);
-		goto out_iput;
+		spin_lock(&dcache_lock);
+		if (parent_sd->s_dentry && parent_sd->s_dentry->d_inode)
+			parent_dentry = dget_locked(parent_sd->s_dentry);
+		spin_unlock(&dcache_lock);
 	}
+	spin_unlock(&sysfs_lock);
 
-	sysfs_instantiate(dentry, inode);
-	inc_nlink(parent->d_inode);
-	sysfs_attach_dirent(sd, parent_sd, dentry);
+	if (error) {
+		sysfs_put(sd);
+		return error;
+	}
 
-	spin_unlock(&sysfs_lock);
+	/* adjust nlink of parent */
+	if (parent_dentry) {
+		mutex_lock(&parent_dentry->d_inode->i_mutex);
+		inc_nlink(parent_dentry->d_inode);
+		mutex_unlock(&parent_dentry->d_inode->i_mutex);
+		dput(parent_dentry);
+	}
 
 	*p_sd = sd;
-	error = 0;
-	goto out_unlock;	/* pin directory dentry in core */
-
- out_iput:
-	iput(inode);
- out_sput:
-	sysfs_put(sd);
- out_drop:
-	d_drop(dentry);
- out_dput:
-	dput(dentry);
- out_unlock:
-	mutex_unlock(&parent->d_inode->i_mutex);
-	return error;
+	return 0;
 }
 
 int sysfs_create_subdir(struct kobject *kobj, const char *name,
@@ -582,9 +555,21 @@ int sysfs_create_dir(struct kobject * ko
 	error = create_dir(kobj, parent_sd, kobject_name(kobj), &sd);
 	if (!error)
 		kobj->sd = sd;
+
 	return error;
 }
 
+static int sysfs_nr_subdirs(struct sysfs_dirent *sd)
+{
+	struct sysfs_dirent *child;
+	int nr = 0;
+
+	for (child = sd->s_children; child; child = child->s_sibling)
+		if (sysfs_type(child) & SYSFS_DIR)
+			nr++;
+	return nr;
+}
+
 static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 				struct nameidata *nd)
 {
@@ -595,7 +580,7 @@ static struct dentry * sysfs_lookup(stru
 	int found = 0;
 
 	for (sd = parent_sd->s_children; sd; sd = sd->s_sibling) {
-		if ((sysfs_type(sd) & SYSFS_NOT_PINNED) &&
+		if (sysfs_type(sd) &&
 		    !strcmp(sd->s_name, dentry->d_name.name)) {
 			found = 1;
 			break;
@@ -616,6 +601,11 @@ static struct dentry * sysfs_lookup(stru
 	if (inode->i_state & I_NEW) {
 		/* initialize inode according to type */
 		switch (sysfs_type(sd)) {
+		case SYSFS_DIR:
+			inode->i_op = &sysfs_dir_inode_operations;
+			inode->i_fop = &sysfs_dir_operations;
+			inode->i_nlink = 2 + sysfs_nr_subdirs(sd);
+			break;
 		case SYSFS_KOBJ_ATTR:
 			inode->i_size = PAGE_SIZE;
 			inode->i_fop = &sysfs_file_operations;
@@ -680,7 +670,7 @@ static void __sysfs_remove_dir(struct sy
 	while (*pos) {
 		struct sysfs_dirent *sd = *pos;
 
-		if (sysfs_type(sd) && (sysfs_type(sd) & SYSFS_NOT_PINNED)) {
+		if (sysfs_type(sd) && !(sysfs_type(sd) & SYSFS_DIR)) {
 			sd->s_flags |= SYSFS_FLAG_REMOVED;
 			*pos = sd->s_sibling;
 			sd->s_sibling = removed;
@@ -728,14 +718,25 @@ int sysfs_rename_dir(struct kobject *kob
 		     const char *new_name)
 {
 	struct sysfs_dirent *sd = kobj->sd;
-	struct dentry *new_parent = new_parent_sd->s_dentry;
-	struct dentry *new_dentry;
-	char *dup_name;
+	struct dentry *new_parent = NULL;
+	struct dentry *old_dentry = NULL, *new_dentry = NULL;
+	const char *dup_name = NULL;
 	int error;
 
-	if (!new_parent_sd)
-		return -EFAULT;
+	/* get dentries */
+	old_dentry = sysfs_get_dentry(sd);
+	if (IS_ERR(old_dentry)) {
+		error = PTR_ERR(old_dentry);
+		goto out_dput;
+	}
+
+	new_parent = sysfs_get_dentry(new_parent_sd);
+	if (IS_ERR(new_parent)) {
+		error = PTR_ERR(new_parent);
+		goto out_dput;
+	}
 
+	/* lock new_parent and get dentry for new name */
 	mutex_lock(&new_parent->d_inode->i_mutex);
 
 	new_dentry = lookup_one_len(new_name, new_parent, strlen(new_name));
@@ -749,14 +750,14 @@ int sysfs_rename_dir(struct kobject *kob
 	 * shadows of the same directory
 	 */
 	error = -EINVAL;
-	if (sd->s_parent->s_dentry->d_inode != new_parent->d_inode ||
+	if (old_dentry->d_parent->d_inode != new_parent->d_inode ||
 	    new_dentry->d_parent->d_inode != new_parent->d_inode ||
-	    new_dentry == sd->s_dentry)
-		goto out_dput;
+	    old_dentry == new_dentry)
+		goto out_unlock;
 
 	error = -EEXIST;
 	if (new_dentry->d_inode)
-		goto out_dput;
+		goto out_unlock;
 
 	/* rename kobject and sysfs_dirent */
 	error = -ENOMEM;
@@ -766,9 +767,9 @@ int sysfs_rename_dir(struct kobject *kob
 
 	error = kobject_set_name(kobj, "%s", new_name);
 	if (error)
-		goto out_free;
+		goto out_drop;
 
-	kfree(sd->s_name);
+	dup_name = sd->s_name;
 	sd->s_name = new_name;
 
 	/* move under the new parent */
@@ -788,45 +789,58 @@ int sysfs_rename_dir(struct kobject *kob
 	error = 0;
 	goto out_unlock;
 
- out_free:
-	kfree(dup_name);
  out_drop:
 	d_drop(new_dentry);
- out_dput:
-	dput(new_dentry);
  out_unlock:
 	mutex_unlock(&new_parent->d_inode->i_mutex);
+ out_dput:
+	kfree(dup_name);
+	dput(new_parent);
+	dput(old_dentry);
+	dput(new_dentry);
 	return error;
 }
 
-int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent)
+int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)
 {
-	struct dentry *old_parent_dentry, *new_parent_dentry, *new_dentry;
-	struct sysfs_dirent *new_parent_sd, *sd;
+	struct sysfs_dirent *sd = kobj->sd;
+	struct sysfs_dirent *new_parent_sd;
+	struct dentry *old_parent, *new_parent = NULL;
+	struct dentry *old_dentry = NULL, *new_dentry = NULL;
 	int error;
 
-	old_parent_dentry = kobj->parent ?
-		kobj->parent->sd->s_dentry : sysfs_mount->mnt_sb->s_root;
-	new_parent_dentry = new_parent ?
-		new_parent->sd->s_dentry : sysfs_mount->mnt_sb->s_root;
+	BUG_ON(!sd->s_parent);
+	new_parent_sd = new_parent_kobj->sd ? new_parent_kobj->sd : &sysfs_root;
+
+	/* get dentries */
+	old_dentry = sysfs_get_dentry(sd);
+	if (IS_ERR(old_dentry)) {
+		error = PTR_ERR(old_dentry);
+		goto out_dput;
+	}
+	old_parent = sd->s_parent->s_dentry;
+
+	new_parent = sysfs_get_dentry(new_parent_sd);
+	if (IS_ERR(new_parent)) {
+		error = PTR_ERR(new_parent);
+		goto out_dput;
+	}
 
-	if (old_parent_dentry->d_inode == new_parent_dentry->d_inode)
-		return 0;	/* nothing to move */
+	if (old_parent->d_inode == new_parent->d_inode) {
+		error = 0;
+		goto out_dput;	/* nothing to move */
+	}
 again:
-	mutex_lock(&old_parent_dentry->d_inode->i_mutex);
-	if (!mutex_trylock(&new_parent_dentry->d_inode->i_mutex)) {
-		mutex_unlock(&old_parent_dentry->d_inode->i_mutex);
+	mutex_lock(&old_parent->d_inode->i_mutex);
+	if (!mutex_trylock(&new_parent->d_inode->i_mutex)) {
+		mutex_unlock(&old_parent->d_inode->i_mutex);
 		goto again;
 	}
 
-	new_parent_sd = new_parent_dentry->d_fsdata;
-	sd = kobj->sd;
-
-	new_dentry = lookup_one_len(kobj->name, new_parent_dentry,
-				    strlen(kobj->name));
+	new_dentry = lookup_one_len(kobj->name, new_parent, strlen(kobj->name));
 	if (IS_ERR(new_dentry)) {
 		error = PTR_ERR(new_dentry);
-		goto out;
+		goto out_unlock;
 	} else
 		error = 0;
 	d_add(new_dentry, NULL);
@@ -843,10 +857,14 @@ again:
 	sysfs_link_sibling(sd);
 
 	spin_unlock(&sysfs_lock);
-out:
-	mutex_unlock(&new_parent_dentry->d_inode->i_mutex);
-	mutex_unlock(&old_parent_dentry->d_inode->i_mutex);
 
+ out_unlock:
+	mutex_unlock(&new_parent->d_inode->i_mutex);
+	mutex_unlock(&old_parent->d_inode->i_mutex);
+ out_dput:
+	dput(new_parent);
+	dput(old_dentry);
+	dput(new_dentry);
 	return error;
 }
 
@@ -1006,12 +1024,20 @@ static loff_t sysfs_dir_lseek(struct fil
 int sysfs_make_shadowed_dir(struct kobject *kobj,
 	void * (*follow_link)(struct dentry *, struct nameidata *))
 {
+	struct dentry *dentry;
 	struct inode *inode;
 	struct inode_operations *i_op;
 
-	inode = kobj->sd->s_dentry->d_inode;
-	if (inode->i_op != &sysfs_dir_inode_operations)
+	/* get dentry for @kobj->sd, dentry of a shadowed dir is pinned */
+	dentry = sysfs_get_dentry(kobj->sd);
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+
+	inode = dentry->d_inode;
+	if (inode->i_op != &sysfs_dir_inode_operations) {
+		dput(dentry);
 		return -EINVAL;
+	}
 
 	i_op = kmalloc(sizeof(*i_op), GFP_KERNEL);
 	if (!i_op)
@@ -1038,16 +1064,22 @@ int sysfs_make_shadowed_dir(struct kobje
 
 struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj)
 {
-	struct dentry *dir = kobj->sd->s_dentry;
-	struct inode *inode = dir->d_inode;
-	struct dentry *parent = dir->d_parent;
-	struct sysfs_dirent *parent_sd = parent->d_fsdata;
-	struct dentry *shadow;
+	struct sysfs_dirent *parent_sd = kobj->sd->s_parent;
+	struct dentry *dir, *parent, *shadow;
+	struct inode *inode;
 	struct sysfs_dirent *sd;
 
+	dir = sysfs_get_dentry(kobj->sd);
+	if (IS_ERR(dir)) {
+		sd = (void *)dir;
+		goto out;
+	}
+	parent = dir->d_parent;
+
+	inode = dir->d_inode;
 	sd = ERR_PTR(-EINVAL);
 	if (!sysfs_is_shadowed_inode(inode))
-		goto out;
+		goto out_dput;
 
 	shadow = d_alloc(parent, &dir->d_name);
 	if (!shadow)
@@ -1066,15 +1098,17 @@ struct sysfs_dirent *sysfs_create_shadow
 	d_instantiate(shadow, igrab(inode));
 	inc_nlink(inode);
 	inc_nlink(parent->d_inode);
-
 	dget(shadow);		/* Extra count - pin the dentry in core */
 
-out:
-	return sd;
-nomem:
+	goto out_dput;
+
+ nomem:
 	dput(shadow);
 	sd = ERR_PTR(-ENOMEM);
-	goto out;
+ out_dput:
+	dput(dir);
+ out:
+	return sd;
 }
 
 /**
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index f2e3c88..1b0c441 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -362,43 +362,22 @@ static unsigned int sysfs_poll(struct fi
 	return POLLERR|POLLPRI;
 }
 
-
-static struct dentry *step_down(struct dentry *dir, const char * name)
-{
-	struct dentry * de;
-
-	if (dir == NULL || dir->d_inode == NULL)
-		return NULL;
-
-	mutex_lock(&dir->d_inode->i_mutex);
-	de = lookup_one_len(name, dir, strlen(name));
-	mutex_unlock(&dir->d_inode->i_mutex);
-	dput(dir);
-	if (IS_ERR(de))
-		return NULL;
-	if (de->d_inode == NULL) {
-		dput(de);
-		return NULL;
-	}
-	return de;
-}
-
 void sysfs_notify(struct kobject * k, char *dir, char *attr)
 {
-	struct dentry *de = k->sd->s_dentry;
-	if (de)
-		dget(de);
-	if (de && dir)
-		de = step_down(de, dir);
-	if (de && attr)
-		de = step_down(de, attr);
-	if (de) {
-		struct sysfs_dirent * sd = de->d_fsdata;
-		if (sd)
-			atomic_inc(&sd->s_event);
+	struct sysfs_dirent *sd = k->sd;
+
+	spin_lock(&sysfs_lock);
+
+	if (sd && dir)
+		sd = sysfs_find_dirent(sd, dir);
+	if (sd && attr)
+		sd = sysfs_find_dirent(sd, attr);
+	if (sd) {
+		atomic_inc(&sd->s_event);
 		wake_up_interruptible(&k->poll);
-		dput(de);
 	}
+
+	spin_unlock(&sysfs_lock);
 }
 EXPORT_SYMBOL_GPL(sysfs_notify);
 
@@ -486,30 +465,31 @@ EXPORT_SYMBOL_GPL(sysfs_add_file_to_grou
  */
 int sysfs_update_file(struct kobject * kobj, const struct attribute * attr)
 {
-	struct dentry * dir = kobj->sd->s_dentry;
-	struct dentry * victim;
-	int res = -ENOENT;
-
-	mutex_lock(&dir->d_inode->i_mutex);
-	victim = lookup_one_len(attr->name, dir, strlen(attr->name));
-	if (!IS_ERR(victim)) {
-		/* make sure dentry is really there */
-		if (victim->d_inode && 
-		    (victim->d_parent->d_inode == dir->d_inode)) {
-			victim->d_inode->i_mtime = CURRENT_TIME;
-			fsnotify_modify(victim);
-			res = 0;
-		} else
-			d_drop(victim);
-		
-		/**
-		 * Drop the reference acquired from lookup_one_len() above.
-		 */
-		dput(victim);
+	struct sysfs_dirent *victim_sd = NULL;
+	struct dentry *victim = NULL;
+	int rc;
+
+	rc = -ENOENT;
+	victim_sd = sysfs_get_dirent(kobj->sd, attr->name);
+	if (!victim_sd)
+		goto out;
+
+	victim = sysfs_get_dentry(victim_sd);
+	if (IS_ERR(victim)) {
+		rc = PTR_ERR(victim);
+		victim = NULL;
+		goto out;
 	}
-	mutex_unlock(&dir->d_inode->i_mutex);
 
-	return res;
+	mutex_lock(&victim->d_inode->i_mutex);
+	victim->d_inode->i_mtime = CURRENT_TIME;
+	fsnotify_modify(victim);
+	mutex_unlock(&victim->d_inode->i_mutex);
+	rc = 0;
+ out:
+	dput(victim);
+	sysfs_put(victim_sd);
+	return rc;
 }
 
 
@@ -522,30 +502,34 @@ int sysfs_update_file(struct kobject * k
  */
 int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)
 {
-	struct dentry *dir = kobj->sd->s_dentry;
-	struct dentry *victim;
+	struct sysfs_dirent *victim_sd = NULL;
+	struct dentry *victim = NULL;
 	struct inode * inode;
 	struct iattr newattrs;
-	int res = -ENOENT;
-
-	mutex_lock(&dir->d_inode->i_mutex);
-	victim = lookup_one_len(attr->name, dir, strlen(attr->name));
-	if (!IS_ERR(victim)) {
-		if (victim->d_inode &&
-		    (victim->d_parent->d_inode == dir->d_inode)) {
-			inode = victim->d_inode;
-			mutex_lock(&inode->i_mutex);
-			newattrs.ia_mode = (mode & S_IALLUGO) |
-						(inode->i_mode & ~S_IALLUGO);
-			newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-			res = notify_change(victim, &newattrs);
-			mutex_unlock(&inode->i_mutex);
-		}
-		dput(victim);
+	int rc;
+
+	rc = -ENOENT;
+	victim_sd = sysfs_get_dirent(kobj->sd, attr->name);
+	if (!victim_sd)
+		goto out;
+
+	victim = sysfs_get_dentry(victim_sd);
+	if (IS_ERR(victim)) {
+		rc = PTR_ERR(victim);
+		victim = NULL;
+		goto out;
 	}
-	mutex_unlock(&dir->d_inode->i_mutex);
 
-	return res;
+	inode = victim->d_inode;
+	mutex_lock(&inode->i_mutex);
+	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
+	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
+	rc = notify_change(victim, &newattrs);
+	mutex_unlock(&inode->i_mutex);
+ out:
+	dput(victim);
+	sysfs_put(victim_sd);
+	return rc;
 }
 EXPORT_SYMBOL_GPL(sysfs_chmod_file);
 
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 705c03d..e71d763 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -190,15 +190,9 @@ void sysfs_instantiate(struct dentry *de
 {
 	BUG_ON(!dentry || dentry->d_inode);
 
-	if (inode->i_state & I_NEW) {
+	if (inode->i_state & I_NEW)
 		unlock_new_inode(inode);
 
-		if (dentry->d_parent && dentry->d_parent->d_inode) {
-			struct inode *p_inode = dentry->d_parent->d_inode;
-			p_inode->i_mtime = p_inode->i_ctime = CURRENT_TIME;
-		}
-	}
-
 	d_instantiate(dentry, inode);
 }
 
@@ -232,7 +226,7 @@ void sysfs_drop_dentry(struct sysfs_dire
 		/* get dentry if it's there and dput() didn't kill it yet */
 		dentry = dget_locked(sd->s_dentry);
 		parent = dentry->d_parent;
-	} else if (sd->s_parent->s_dentry->d_inode) {
+	} else if (sd->s_parent->s_dentry && sd->s_parent->s_dentry->d_inode) {
 		/* We need to update the parent even if dentry for the
 		 * victim itself doesn't exist.
 		 */
@@ -267,8 +261,9 @@ void sysfs_drop_dentry(struct sysfs_dire
 		if (sysfs_type(sd) & SYSFS_DIR) {
 			drop_nlink(dentry->d_inode);
 			drop_nlink(dir);
-			/* XXX: unpin if directory, this will go away soon */
-			dput(dentry);
+			/* dentries for shadowed inode are pinned, unpin */
+			if (sysfs_is_shadowed_inode(dentry->d_inode))
+				dput(dentry);
 		}
 	}
 
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index d5ce3aa..f82c345 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -22,7 +22,7 @@ static const struct super_operations sys
 	.drop_inode	= sysfs_delete_inode,
 };
 
-static struct sysfs_dirent sysfs_root = {
+struct sysfs_dirent sysfs_root = {
 	.s_count	= ATOMIC_INIT(1),
 	.s_flags	= SYSFS_ROOT,
 	.s_mode		= S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index b11c448..0aa4b18 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -45,6 +45,7 @@ struct sysfs_dirent {
 #define SD_DEACTIVATED_BIAS	INT_MIN
 
 extern struct vfsmount * sysfs_mount;
+extern struct sysfs_dirent sysfs_root;
 extern struct kmem_cache *sysfs_dir_cachep;
 
 extern struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 4c43030..2f58ca1 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -81,7 +81,6 @@ struct sysfs_ops {
 #define SYSFS_KOBJ_ATTR 	0x0004
 #define SYSFS_KOBJ_BIN_ATTR	0x0008
 #define SYSFS_KOBJ_LINK 	0x0020
-#define SYSFS_NOT_PINNED	(SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)
 #define SYSFS_COPY_NAME		(SYSFS_DIR | SYSFS_KOBJ_LINK)
 
 #define SYSFS_FLAG_MASK		~SYSFS_TYPE_MASK
-- 
1.4.3.4

[PATCH 4/6] sysfs: use sysfs_lock to protect the sysfs_dirent tree

[Tejun Heo]

As kobj sysfs dentries and inodes are gonna be made reclaimable,
i_mutex can't be used to protect sysfs_dirent tree.  Use sysfs_lock
instead.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/dir.c     |  107 ++++++++++++++++++++++++++++++++++-----------------
 fs/sysfs/file.c    |   31 +++++++--------
 fs/sysfs/inode.c   |    9 +---
 fs/sysfs/symlink.c |   51 +++++++++++++------------
 fs/sysfs/sysfs.h   |    1 -
 5 files changed, 115 insertions(+), 84 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 58a92aa..a474e54 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -14,7 +14,6 @@
 #include <asm/semaphore.h>
 #include "sysfs.h"
 
-DECLARE_RWSEM(sysfs_rename_sem);
 spinlock_t sysfs_lock = SPIN_LOCK_UNLOCKED;
 spinlock_t kobj_sysfs_assoc_lock = SPIN_LOCK_UNLOCKED;
 
@@ -29,7 +28,7 @@ static DEFINE_IDA(sysfs_ino_ida);
  *	sd->s_parent->s_children.
  *
  *	Locking:
- *	mutex_lock(sd->s_parent->dentry->d_inode->i_mutex)
+ *	spin_lock(sysfs_lock)
  */
 static void sysfs_link_sibling(struct sysfs_dirent *sd)
 {
@@ -48,7 +47,7 @@ static void sysfs_link_sibling(struct sy
  *	sd->s_parent->s_children.
  *
  *	Locking:
- *	mutex_lock(sd->s_parent->dentry->d_inode->i_mutex)
+ *	spin_lock(sysfs_lock)
  */
 static void sysfs_unlink_sibling(struct sysfs_dirent *sd)
 {
@@ -216,6 +215,9 @@ void release_sysfs_dirent(struct sysfs_d
 	struct sysfs_dirent *parent_sd;
 
  repeat:
+	/* Moving/renaming is always done while holding reference.
+	 * sd->s_parent won't change beneath us.
+	 */
 	parent_sd = sd->s_parent;
 
 	if (sysfs_type(sd) & SYSFS_KOBJ_LINK)
@@ -292,19 +294,36 @@ struct sysfs_dirent *sysfs_new_dirent(co
 	return NULL;
 }
 
+/**
+ *	sysfs_attach_dentry - associate sysfs_dirent with dentry
+ *	@sd: target sysfs_dirent
+ *	@dentry: dentry to associate
+ *
+ *	Associate @sd with @dentry.  This is protected by sysfs_lock
+ *	to avoid race with sysfs_d_iput().
+ *
+ *	LOCKING:
+ *	spin_lock(sysfs_lock)
+ */
 static void sysfs_attach_dentry(struct sysfs_dirent *sd, struct dentry *dentry)
 {
 	dentry->d_op = &sysfs_dentry_ops;
 	dentry->d_fsdata = sysfs_get(sd);
-
-	/* protect sd->s_dentry against sysfs_d_iput */
-	spin_lock(&sysfs_lock);
-	sd->s_dentry = dentry;
-	spin_unlock(&sysfs_lock);
-
+	sd->s_dentry = dentry;	/* protected with sysfs_lock */
 	d_rehash(dentry);
 }
 
+/**
+ *	sysfs_attach_dirent - attach sysfs_dirent to its parent and dentry
+ *	@sd: sysfs_dirent to attach
+ *	@parent_sd: parent to attach to (optional)
+ *	@dentry: dentry to be associated to @sd (optional)
+ *
+ *	Attach @sd to @parent_sd and/or @dentry.  Both are optional.
+ *
+ *	LOCKING:
+ *	spin_lock(sysfs_lock)
+ */
 void sysfs_attach_dirent(struct sysfs_dirent *sd,
 			 struct sysfs_dirent *parent_sd, struct dentry *dentry)
 {
@@ -325,7 +344,7 @@ void sysfs_attach_dirent(struct sysfs_di
  *	Look for sysfs_dirent with name @name under @parent_sd.
  *
  *	LOCKING:
- *	mutex_lock(parent->i_mutex)
+ *	spin_lock(sysfs_lock)
  *
  *	RETURNS:
  *	Pointer to sysfs_dirent if found, NULL if not.
@@ -350,7 +369,7 @@ struct sysfs_dirent *sysfs_find_dirent(s
  *	it if found.
  *
  *	LOCKING:
- *	Kernel thread context (may sleep)
+ *	None.  Grabs sysfs_lock.
  *
  *	RETURNS:
  *	Pointer to sysfs_dirent if found, NULL if not.
@@ -360,10 +379,10 @@ struct sysfs_dirent *sysfs_get_dirent(st
 {
 	struct sysfs_dirent *sd;
 
-	mutex_lock(&parent_sd->s_dentry->d_inode->i_mutex);
+	spin_lock(&sysfs_lock);
 	sd = sysfs_find_dirent(parent_sd, name);
 	sysfs_get(sd);
-	mutex_unlock(&parent_sd->s_dentry->d_inode->i_mutex);
+	spin_unlock(&sysfs_lock);
 
 	return sd;
 }
@@ -409,14 +428,20 @@ static int create_dir(struct kobject *ko
 	}
 
 	/* link in */
+	spin_lock(&sysfs_lock);
+
 	error = -EEXIST;
-	if (sysfs_find_dirent(parent_sd, name))
+	if (sysfs_find_dirent(parent_sd, name)) {
+		spin_unlock(&sysfs_lock);
 		goto out_iput;
+	}
 
 	sysfs_instantiate(dentry, inode);
 	inc_nlink(parent->d_inode);
 	sysfs_attach_dirent(sd, parent_sd, dentry);
 
+	spin_unlock(&sysfs_lock);
+
 	*p_sd = sd;
 	error = 0;
 	goto out_unlock;	/* pin directory dentry in core */
@@ -494,6 +519,8 @@ static struct dentry * sysfs_lookup(stru
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
+	spin_lock(&sysfs_lock);
+
 	if (inode->i_state & I_NEW) {
 		/* initialize inode according to type */
 		switch (sysfs_type(sd)) {
@@ -517,6 +544,8 @@ static struct dentry * sysfs_lookup(stru
 	sysfs_instantiate(dentry, inode);
 	sysfs_attach_dentry(sd, dentry);
 
+	spin_unlock(&sysfs_lock);
+
 	return NULL;
 }
 
@@ -527,17 +556,13 @@ const struct inode_operations sysfs_dir_
 
 static void remove_dir(struct sysfs_dirent *sd)
 {
-	struct dentry *parent = sd->s_parent->s_dentry;
-
-	mutex_lock(&parent->d_inode->i_mutex);
-
+	spin_lock(&sysfs_lock);
 	sysfs_unlink_sibling(sd);
 	sd->s_flags |= SYSFS_FLAG_REMOVED;
+	spin_unlock(&sysfs_lock);
 
 	pr_debug(" o %s removing done\n", sd->s_name);
 
-	mutex_unlock(&parent->d_inode->i_mutex);
-
 	sysfs_drop_dentry(sd);
 	sysfs_deactivate(sd);
 	sysfs_put(sd);
@@ -553,15 +578,12 @@ static void __sysfs_remove_dir(struct sy
 {
 	struct sysfs_dirent *removed = NULL;
 	struct sysfs_dirent **pos;
-	struct dentry *dir;
 
 	if (!dir_sd)
 		return;
 
-	dir = dir_sd->s_dentry;
-
 	pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
-	mutex_lock(&dir->d_inode->i_mutex);
+	spin_lock(&sysfs_lock);
 	pos = &dir_sd->s_children;
 	while (*pos) {
 		struct sysfs_dirent *sd = *pos;
@@ -574,7 +596,7 @@ static void __sysfs_remove_dir(struct sy
 		} else
 			pos = &(*pos)->s_sibling;
 	}
-	mutex_unlock(&dir->d_inode->i_mutex);
+	spin_unlock(&sysfs_lock);
 
 	while (removed) {
 		struct sysfs_dirent *sd = removed;
@@ -622,7 +644,6 @@ int sysfs_rename_dir(struct kobject *kob
 	if (!new_parent_sd)
 		return -EFAULT;
 
-	down_write(&sysfs_rename_sem);
 	mutex_lock(&new_parent->d_inode->i_mutex);
 
 	new_dentry = lookup_one_len(new_name, new_parent, strlen(new_name));
@@ -662,12 +683,16 @@ int sysfs_rename_dir(struct kobject *kob
 	d_add(new_dentry, NULL);
 	d_move(sd->s_dentry, new_dentry);
 
+	spin_lock(&sysfs_lock);
+
 	sysfs_unlink_sibling(sd);
 	sysfs_get(new_parent_sd);
 	sysfs_put(sd->s_parent);
 	sd->s_parent = new_parent_sd;
 	sysfs_link_sibling(sd);
 
+	spin_unlock(&sysfs_lock);
+
 	error = 0;
 	goto out_unlock;
 
@@ -679,7 +704,6 @@ int sysfs_rename_dir(struct kobject *kob
 	dput(new_dentry);
  out_unlock:
 	mutex_unlock(&new_parent->d_inode->i_mutex);
-	up_write(&sysfs_rename_sem);
 	return error;
 }
 
@@ -718,12 +742,15 @@ again:
 	dput(new_dentry);
 
 	/* Remove from old parent's list and insert into new parent's list. */
+	spin_lock(&sysfs_lock);
+
 	sysfs_unlink_sibling(sd);
 	sysfs_get(new_parent_sd);
 	sysfs_put(sd->s_parent);
 	sd->s_parent = new_parent_sd;
 	sysfs_link_sibling(sd);
 
+	spin_unlock(&sysfs_lock);
 out:
 	mutex_unlock(&new_parent_dentry->d_inode->i_mutex);
 	mutex_unlock(&old_parent_dentry->d_inode->i_mutex);
@@ -737,11 +764,12 @@ static int sysfs_dir_open(struct inode *
 	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
 	struct sysfs_dirent * sd;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
 	sd = sysfs_new_dirent("_DIR_", 0, 0);
-	if (sd)
+	if (sd) {
+		spin_lock(&sysfs_lock);
 		sysfs_attach_dirent(sd, parent_sd, NULL);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+		spin_unlock(&sysfs_lock);
+	}
 
 	file->private_data = sd;
 	return sd ? 0 : -ENOMEM;
@@ -749,12 +777,11 @@ static int sysfs_dir_open(struct inode *
 
 static int sysfs_dir_close(struct inode *inode, struct file *file)
 {
-	struct dentry * dentry = file->f_path.dentry;
 	struct sysfs_dirent * cursor = file->private_data;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
+	spin_lock(&sysfs_lock);
 	sysfs_unlink_sibling(cursor);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+	spin_unlock(&sysfs_lock);
 
 	release_sysfs_dirent(cursor);
 
@@ -795,6 +822,8 @@ static int sysfs_readdir(struct file * f
 			i++;
 			/* fallthrough */
 		default:
+			spin_lock(&sysfs_lock);
+
 			pos = &parent_sd->s_children;
 			while (*pos != cursor)
 				pos = &(*pos)->s_sibling;
@@ -827,6 +856,8 @@ static int sysfs_readdir(struct file * f
 			/* put cursor back in */
 			cursor->s_sibling = *pos;
 			*pos = cursor;
+
+			spin_unlock(&sysfs_lock);
 	}
 	return 0;
 }
@@ -835,7 +866,6 @@ static loff_t sysfs_dir_lseek(struct fil
 {
 	struct dentry * dentry = file->f_path.dentry;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
 	switch (origin) {
 		case 1:
 			offset += file->f_pos;
@@ -843,10 +873,11 @@ static loff_t sysfs_dir_lseek(struct fil
 			if (offset >= 0)
 				break;
 		default:
-			mutex_unlock(&file->f_path.dentry->d_inode->i_mutex);
 			return -EINVAL;
 	}
 	if (offset != file->f_pos) {
+		spin_lock(&sysfs_lock);
+
 		file->f_pos = offset;
 		if (file->f_pos >= 2) {
 			struct sysfs_dirent *sd = dentry->d_fsdata;
@@ -867,8 +898,10 @@ static loff_t sysfs_dir_lseek(struct fil
 			cursor->s_sibling = *pos;
 			*pos = cursor;
 		}
+
+		spin_unlock(&sysfs_lock);
 	}
-	mutex_unlock(&dentry->d_inode->i_mutex);
+
 	return offset;
 }
 
@@ -934,7 +967,9 @@ struct sysfs_dirent *sysfs_create_shadow
 	sd->s_elem.dir.kobj = kobj;
 	/* point to parent_sd but don't attach to it */
 	sd->s_parent = sysfs_get(parent_sd);
+	spin_lock(&sysfs_lock);
 	sysfs_attach_dirent(sd, NULL, shadow);
+	spin_unlock(&sysfs_lock);
 
 	d_instantiate(shadow, igrab(inode));
 	inc_nlink(inode);
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index acc9890..f2e3c88 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -415,29 +415,28 @@ const struct file_operations sysfs_file_
 int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
 		   int type)
 {
-	struct dentry *dir = dir_sd->s_dentry;
 	umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
 	struct sysfs_dirent *sd;
-	int error = 0;
 
-	mutex_lock(&dir->d_inode->i_mutex);
+	sd = sysfs_new_dirent(attr->name, mode, type);
+	if (!sd)
+		return -ENOMEM;
+	sd->s_elem.attr.attr = (void *)attr;
 
-	if (sysfs_find_dirent(dir_sd, attr->name)) {
-		error = -EEXIST;
-		goto out_unlock;
-	}
+	spin_lock(&sysfs_lock);
 
-	sd = sysfs_new_dirent(attr->name, mode, type);
-	if (!sd) {
-		error = -ENOMEM;
-		goto out_unlock;
+	if (!sysfs_find_dirent(dir_sd, attr->name)) {
+		sysfs_attach_dirent(sd, dir_sd, NULL);
+		sd = NULL;
 	}
-	sd->s_elem.attr.attr = (void *)attr;
-	sysfs_attach_dirent(sd, dir_sd, NULL);
 
- out_unlock:
-	mutex_unlock(&dir->d_inode->i_mutex);
-	return error;
+	spin_unlock(&sysfs_lock);
+
+	if (sd) {
+		sysfs_put(sd);
+		return -EEXIST;
+	}
+	return 0;
 }
 
 
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 4077ab5..705c03d 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -283,15 +283,11 @@ void sysfs_drop_dentry(struct sysfs_dire
 
 int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
 {
-	struct dentry *dir = dir_sd->s_dentry;
 	struct sysfs_dirent **pos, *sd;
 	int found = 0;
 
-	if (dir->d_inode == NULL)
-		/* no inode means this hasn't been made visible yet */
-		return -ENOENT;
+	spin_lock(&sysfs_lock);
 
-	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
 	for (pos = &dir_sd->s_children; *pos; pos = &(*pos)->s_sibling) {
 		sd = *pos;
 
@@ -305,7 +301,8 @@ int sysfs_hash_and_remove(struct sysfs_d
 			break;
 		}
 	}
-	mutex_unlock(&dir->d_inode->i_mutex);
+
+	spin_unlock(&sysfs_lock);
 
 	if (!found)
 		return -ENOENT;
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 43cc522..b05f6a6 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -44,20 +44,6 @@ static void fill_object_path(struct sysf
 	}
 }
 
-static int sysfs_add_link(struct sysfs_dirent * parent_sd, const char * name,
-			  struct sysfs_dirent * target_sd)
-{
-	struct sysfs_dirent * sd;
-
-	sd = sysfs_new_dirent(name, S_IFLNK|S_IRWXUGO, SYSFS_KOBJ_LINK);
-	if (!sd)
-		return -ENOMEM;
-
-	sd->s_elem.symlink.target_sd = target_sd;
-	sysfs_attach_dirent(sd, parent_sd, NULL);
-	return 0;
-}
-
 /**
  *	sysfs_create_link - create symlink between two objects.
  *	@kobj:	object whose directory we're creating the link in.
@@ -68,7 +54,8 @@ int sysfs_create_link(struct kobject * k
 {
 	struct sysfs_dirent *parent_sd = NULL;
 	struct sysfs_dirent *target_sd = NULL;
-	int error = -EEXIST;
+	struct sysfs_dirent *sd = NULL;
+	int error;
 
 	BUG_ON(!name);
 
@@ -78,8 +65,9 @@ int sysfs_create_link(struct kobject * k
 	} else
 		parent_sd = kobj->sd;
 
+	error = -EFAULT;
 	if (!parent_sd)
-		return -EFAULT;
+		goto out_put;
 
 	/* target->sd can go away beneath us but is protected with
 	 * kobj_sysfs_assoc_lock.  Fetch target_sd from it.
@@ -89,17 +77,30 @@ int sysfs_create_link(struct kobject * k
 		target_sd = sysfs_get(target->sd);
 	spin_unlock(&kobj_sysfs_assoc_lock);
 
+	error = -ENOENT;
 	if (!target_sd)
-		return -ENOENT;
+		goto out_put;
+
+	error = -ENOMEM;
+	sd = sysfs_new_dirent(name, S_IFLNK|S_IRWXUGO, SYSFS_KOBJ_LINK);
+	if (!sd)
+		goto out_put;
+	sd->s_elem.symlink.target_sd = target_sd;
 
-	mutex_lock(&parent_sd->s_dentry->d_inode->i_mutex);
-	if (!sysfs_find_dirent(parent_sd, name))
-		error = sysfs_add_link(parent_sd, name, target_sd);
-	mutex_unlock(&parent_sd->s_dentry->d_inode->i_mutex);
+	spin_lock(&sysfs_lock);
+	error = -EEXIST;
+	if (sysfs_find_dirent(parent_sd, name))
+		goto out_unlock;
+	sysfs_attach_dirent(sd, parent_sd, NULL);
+	spin_unlock(&sysfs_lock);
 
-	if (error)
-		sysfs_put(target_sd);
+	return 0;
 
+ out_unlock:
+	spin_unlock(&sysfs_lock);
+ out_put:
+	sysfs_put(target_sd);
+	sysfs_put(sd);
 	return error;
 }
 
@@ -144,9 +145,9 @@ static int sysfs_getlink(struct dentry *
 	struct sysfs_dirent *target_sd = sd->s_elem.symlink.target_sd;
 	int error;
 
-	down_read(&sysfs_rename_sem);
+	spin_lock(&sysfs_lock);
 	error = sysfs_get_target_path(parent_sd, target_sd, path);
-	up_read(&sysfs_rename_sem);
+	spin_unlock(&sysfs_lock);
 
 	return error;
 }
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 27a5f4b..476d58b 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -83,7 +83,6 @@ extern int sysfs_setattr(struct dentry *
 
 extern spinlock_t sysfs_lock;
 extern spinlock_t kobj_sysfs_assoc_lock;
-extern struct rw_semaphore sysfs_rename_sem;
 extern struct super_block * sysfs_sb;
 extern const struct file_operations sysfs_dir_operations;
 extern const struct file_operations sysfs_file_operations;
-- 
1.4.3.4

Re: [PATCH 4/6] sysfs: use sysfs_lock to protect the sysfs_dirent tree

[Cornelia Huck]

On Tue, 29 May 2007 01:36:27 +0900,
Tejun Heo <htejun@gmail.com> wrote:

> @@ -795,6 +822,8 @@ static int sysfs_readdir(struct file * f
>  			i++;
>  			/* fallthrough */
>  		default:
> +			spin_lock(&sysfs_lock);
> +
>  			pos = &parent_sd->s_children;
>  			while (*pos != cursor)
>  				pos = &(*pos)->s_sibling;
> @@ -827,6 +856,8 @@ static int sysfs_readdir(struct file * f
>  			/* put cursor back in */
>  			cursor->s_sibling = *pos;
>  			*pos = cursor;
> +
> +			spin_unlock(&sysfs_lock);
>  	}
>  	return 0;
>  }

Here's the cause of the "sleeping function called" I saw. filldir() is
called under sysfs_lock, but calls copy_to_user()... This means you
can't use sysfs_lock for protection here.

Re: [PATCH 4/6] sysfs: use sysfs_lock to protect the sysfs_dirent tree

[Tejun Heo]

Cornelia Huck wrote:
> On Tue, 29 May 2007 01:36:27 +0900,
> Tejun Heo <htejun@gmail.com> wrote:
> 
>> @@ -795,6 +822,8 @@ static int sysfs_readdir(struct file * f
>>  			i++;
>>  			/* fallthrough */
>>  		default:
>> +			spin_lock(&sysfs_lock);
>> +
>>  			pos = &parent_sd->s_children;
>>  			while (*pos != cursor)
>>  				pos = &(*pos)->s_sibling;
>> @@ -827,6 +856,8 @@ static int sysfs_readdir(struct file * f
>>  			/* put cursor back in */
>>  			cursor->s_sibling = *pos;
>>  			*pos = cursor;
>> +
>> +			spin_unlock(&sysfs_lock);
>>  	}
>>  	return 0;
>>  }
> 
> Here's the cause of the "sleeping function called" I saw. filldir() is
> called under sysfs_lock, but calls copy_to_user()... This means you
> can't use sysfs_lock for protection here.

Ouch, right.  I think we can get away with a temp buffer there.  Thanks.

-- 
tejun

[PATCH 2/6] sysfs: implement sysfs_find_dirent() and sysfs_get_dirent()

[Tejun Heo]

Implement sysfs_find_dirent() and sysfs_get_dirent().
sysfs_dirent_exist() is replaced by sysfs_find_dirent().  These will
be used to make directory entries reclamiable.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 fs/sysfs/dir.c     |   61 +++++++++++++++++++++++++++++++++++++--------------
 fs/sysfs/file.c    |    2 +-
 fs/sysfs/symlink.c |    2 +-
 fs/sysfs/sysfs.h   |    5 +++-
 4 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 4a2bd8b..c05d04a 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -317,28 +317,55 @@ void sysfs_attach_dirent(struct sysfs_di
 	}
 }
 
-/*
+/**
+ *	sysfs_find_dirent - find sysfs_dirent with the given name
+ *	@parent_sd: sysfs_dirent to search under
+ *	@name: name to look for
  *
- * Return -EEXIST if there is already a sysfs element with the same name for
- * the same parent.
+ *	Look for sysfs_dirent with name @name under @parent_sd.
  *
- * called with parent inode's i_mutex held
+ *	LOCKING:
+ *	mutex_lock(parent->i_mutex)
+ *
+ *	RETURNS:
+ *	Pointer to sysfs_dirent if found, NULL if not.
  */
-int sysfs_dirent_exist(struct sysfs_dirent *parent_sd,
-			  const unsigned char *new)
+struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
+				       const unsigned char *name)
 {
-	struct sysfs_dirent * sd;
+	struct sysfs_dirent *sd;
 
-	for (sd = parent_sd->s_children; sd; sd = sd->s_sibling) {
-		if (sysfs_type(sd)) {
-			if (strcmp(sd->s_name, new))
-				continue;
-			else
-				return -EEXIST;
-		}
-	}
+	for (sd = parent_sd->s_children; sd; sd = sd->s_sibling)
+		if (sysfs_type(sd) && !strcmp(sd->s_name, name))
+			return sd;
+	return NULL;
+}
 
-	return 0;
+/**
+ *	sysfs_get_dirent - find and get sysfs_dirent with the given name
+ *	@parent_sd: sysfs_dirent to search under
+ *	@name: name to look for
+ *
+ *	Look for sysfs_dirent with name @name under @parent_sd and get
+ *	it if found.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	Pointer to sysfs_dirent if found, NULL if not.
+ */
+struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
+				      const unsigned char *name)
+{
+	struct sysfs_dirent *sd;
+
+	mutex_lock(&parent_sd->s_dentry->d_inode->i_mutex);
+	sd = sysfs_find_dirent(parent_sd, name);
+	sysfs_get(sd);
+	mutex_unlock(&parent_sd->s_dentry->d_inode->i_mutex);
+
+	return sd;
 }
 
 static int create_dir(struct kobject *kobj, struct dentry *parent,
@@ -382,7 +409,7 @@ static int create_dir(struct kobject *ko
 
 	/* link in */
 	error = -EEXIST;
-	if (sysfs_dirent_exist(parent->d_fsdata, name))
+	if (sysfs_find_dirent(parent->d_fsdata, name))
 		goto out_iput;
 
 	sysfs_instantiate(dentry, inode);
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a84b734..e448b88 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -421,7 +421,7 @@ int sysfs_add_file(struct dentry * dir,
 
 	mutex_lock(&dir->d_inode->i_mutex);
 
-	if (sysfs_dirent_exist(parent_sd, attr->name)) {
+	if (sysfs_find_dirent(parent_sd, attr->name)) {
 		error = -EEXIST;
 		goto out_unlock;
 	}
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index ff605d3..45b62e2 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -95,7 +95,7 @@ int sysfs_create_link(struct kobject * k
 		return -ENOENT;
 
 	mutex_lock(&dentry->d_inode->i_mutex);
-	if (!sysfs_dirent_exist(dentry->d_fsdata, name))
+	if (!sysfs_find_dirent(dentry->d_fsdata, name))
 		error = sysfs_add_link(parent_sd, name, target_sd);
 	mutex_unlock(&dentry->d_inode->i_mutex);
 
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 06b5085..f1629b4 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -59,7 +59,10 @@ extern struct inode * sysfs_get_inode(st
 extern void sysfs_instantiate(struct dentry *dentry, struct inode *inode);
 
 extern void release_sysfs_dirent(struct sysfs_dirent * sd);
-extern int sysfs_dirent_exist(struct sysfs_dirent *, const unsigned char *);
+extern struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
+					      const unsigned char *name);
+extern struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
+					     const unsigned char *name);
 extern struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode,
 					     int type);
 extern void sysfs_attach_dirent(struct sysfs_dirent *sd,
-- 
1.4.3.4

Re: [PATCHSET 2.6.22-rc2-mm1 REVIEW] sysfs: make directory dentries/inodes reclaimable

[Cornelia Huck]

On Tue, 29 May 2007 01:36:26 +0900,
Tejun Heo <htejun@gmail.com> wrote:

> Please review, test, scream... :-)

Kernel seems a bit unhappy:

BUG: sleeping function called from invalid context at
include/asm/uaccess.h:234 in_atomic():1, irqs_disabled():0
00000000018b8d80 0000000000000002 0000000000000000 00000000030c3d68 
       00000000030c3ce0 00000000003aa006 00000000003aa006 0000000000015528
       0000000000000000 000003ffffb15750 00000000018b8d80 0000000000000000
       000000000046de20 0000000000000000 00000000030c3d30 ffffffff0000000e
       0000000000333fb0 0000000000015572 00000000030c3cc8 00000000030c3d10
Call Trace:
([<00000000000154bc>] show_trace+0xbc/0xd0)
 [<0000000000031a6e>] __might_sleep+0xfe/0x158
 [<00000000000b6836>] filldir+0xf6/0x198
 [<0000000000103a82>] sysfs_readdir+0x10a/0x228
 [<00000000000b6220>] vfs_readdir+0xb8/0xd8
 [<00000000000b63e8>] sys_getdents+0x60/0xdc
 [<00000000000222c0>] sysc_noemu+0x10/0x16
 [<00000200000de78e>] 0x200000de78e

But at least it comes up :)

(Will take a look at the patches now)