Skip to contentSkip to conversation

sysfs: make directory dentries/inodes reclaimable, take#2

PATCH New Tejun Heo 1 participant 12 messages Jun 13, 2007

Subsystems (2)
lore
Share & tools
lore
author
#2 lore raw

PATCH 2/11 sysfs: rename sysfs_dirent->s_type to s_flags and make room for flags

In reply to Tejun Heo

Rename sysfs_dirent->s_type to s_flags, pack type into lower eight bits and reserve the rest for flags. sysfs_type() can used to access the type. All existing sd->s_type accesses are converted to use sysfs_type(). While at it, type test is changed to equality test instead of bit-and test where appropriate.

---
 fs/sysfs/dir.c        |   33 ++++++++++++++++++++-------------
 fs/sysfs/inode.c      |    8 ++++----
 fs/sysfs/mount.c      |    2 +-
 fs/sysfs/sysfs.h      |    7 ++++++-
 include/linux/sysfs.h |    3 +++
 5 files changed, 34 insertions(+), 19 deletions(-)
fs/sysfs/dir.c +20 -13
index b4074ad..eb9bc0a 100644
@@ -218,9 +218,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
  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(const char *name, umode_t mode, int type)
 
 	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_dirent *parent_sd,
 	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(struct inode *dir, struct dentry *dentry,
 {
 	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(struct inode *dir, struct dentry *dentry,
 
 	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);
@@ -532,7 +539,7 @@ static void __sysfs_remove_dir(struct dentry *dentry)
 	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)) {
 			*pos = sd->s_sibling;
 			sd->s_sibling = removed;
 			removed = sd;
@@ -775,7 +782,7 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 				const char * name;
 				int len;
 
-				if (!next->s_type)
+				if (!sysfs_type(next))
 					continue;
 
 				name = next->s_name;
@@ -824,7 +831,7 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
 			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;
 			}
fs/sysfs/inode.c +4 -4
index 63daa06..ee3a5d9 100644
@@ -242,7 +242,7 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 
 	dput(dentry);
 	/* XXX: unpin if directory, this will go away soon */
-	if (sd->s_type & SYSFS_DIR)
+	if (sysfs_type(sd) == SYSFS_DIR)
 		dput(dentry);
 
 	/* adjust nlink and update timestamp */
@@ -254,7 +254,7 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 
 		inode->i_ctime = curtime;
 		drop_nlink(inode);
-		if (sd->s_type & SYSFS_DIR)
+		if (sysfs_type(sd) == SYSFS_DIR)
 			drop_nlink(inode);
 
 		mutex_unlock(&inode->i_mutex);
@@ -267,7 +267,7 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 		mutex_lock(&inode->i_mutex);
 
 		inode->i_ctime = inode->i_mtime = curtime;
-		if (sd->s_type & SYSFS_DIR)
+		if (sysfs_type(sd) == SYSFS_DIR)
 			drop_nlink(inode);
 
 		mutex_unlock(&inode->i_mutex);
@@ -293,7 +293,7 @@ int sysfs_hash_and_remove(struct dentry * dir, const char * name)
 	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)) {
 			*pos = sd->s_sibling;
fs/sysfs/mount.c +1 -1
index 4be9593..078537e 100644
@@ -26,7 +26,7 @@ static const struct super_operations sysfs_ops = {
 
 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,
 };
fs/sysfs/sysfs.h +6 -1
index 6f8aaf3..06b5085 100644
@@ -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_fops;
 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) {
include/linux/sysfs.h +3 -0
index 161e19a..5813550 100644
@@ -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,8 @@ 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
+
 #ifdef CONFIG_SYSFS
 
 extern int sysfs_schedule_callback(struct kobject *kobj,
Signature · 1 line
1.5.0.3

Tejun Heo

author
#0 lore raw

PATCH 1/1 sysfs: make directory dentries/inodes reclaimable, take#2

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

#01: make-sysfs_drop_dentry-access-inodes-using-ilookup #02: rename-sysfs_dirent-s_type-to-s_flags-and-make-room-for-flags #03: implement-SYSFS_FLAG_REMOVED-flag
#04: implement-sysfs_find_dirent-and-sysfs_get_dirent #05: make-kobj-point-to-sysfs_dirent-instead-of-dentry #06: consolidate-sysfs-spinlocks
#07: use-sysfs_mutex-to-protect-the-sysfs_dirent-tree #08: restructure-add-remove-paths-and-fix-inode-up
#09: move-sysfs_drop_dentry-to-dir.c-and-make-it-static #10: implement-sysfs_get_dentry
#11: make-directory-dentries-and-inodes-reclaimable

API changes...

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

Changes from the last take[L] are...

* #01 added.
* #02 and #03 splitted from the first patch of the last take.
* #06 added. sysfs_lock isn't used as global sysfs_dirent tree lock. merge it with kobj_sysfs_assoc_lock.
* #07 modified to use sysfs_mutex instead of sysfs_lock to protect sysfs_dirent tree. This resolves the problem Cornelia was seeing in the last take.
* #08 and #09 added. This is primarily to keep the parent inode's timestamps and i_nlink in sync. Code looks better after the change too.

I'm running stress test for several hours now and things look pretty good. This will save quite some amount of memory on big machines.

This patchset applies on top of 2.6.22-rc4-mm2 or

current linux-2.6#master (a0e1d1d075cc0efe9a3ac8579bce9393d070e09f)
+ regenerated sysfs rework patchset (forgot to cc lkml when posting. the end result is practically the same to 2.6.22-rc4-mm2)

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.kernel/535388

author
#1 lore raw

PATCH 1/11 sysfs: make sysfs_drop_dentry() access inodes using ilookup()

sysfs_drop_dentry() used to go through sd->s_dentry and sd->s_parent->s_dentry to access the inodes. This is incorrect because inode can be cached without dentry.

This patch makes sysfs_drop_dentry() access inodes using ilookup() on sd->s_ino. This is both correct and simpler.

---
 fs/sysfs/inode.c |   63 ++++++++++++++++++++++++------------------------------
 1 files changed, 28 insertions(+), 35 deletions(-)
fs/sysfs/inode.c +28 -35
index ee31bf3..63daa06 100644
@@ -219,9 +219,9 @@ void sysfs_instantiate(struct dentry *dentry, struct inode *inode)
  */
 void sysfs_drop_dentry(struct sysfs_dirent *sd)
 {
-	struct dentry *dentry = NULL, *parent = NULL;
-	struct inode *dir;
+	struct dentry *dentry = NULL;
 	struct timespec curtime;
+	struct inode *inode;
 
 	/* We're not holding a reference to ->s_dentry dentry but the
 	 * field will stay valid as long as sysfs_lock is held.
@@ -229,19 +229,9 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 	spin_lock(&sysfs_lock);
 	spin_lock(&dcache_lock);
 
+	/* drop dentry if it's there and dput() didn't kill it yet */
 	if (sd->s_dentry && sd->s_dentry->d_inode) {
-		/* 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) {
-		/* We need to update the parent even if dentry for the
-		 * victim itself doesn't exist.
-		 */
-		parent = dget_locked(sd->s_parent->s_dentry);
-	}
-
-	/* drop */
-	if (dentry) {
 		spin_lock(&dentry->d_lock);
 		__d_drop(dentry);
 		spin_unlock(&dentry->d_lock);
@@ -250,36 +240,39 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 	spin_unlock(&dcache_lock);
 	spin_unlock(&sysfs_lock);
 
-	/* nothing to do if the parent isn't in dcache */
-	if (!parent)
-		return;
+	dput(dentry);
+	/* XXX: unpin if directory, this will go away soon */
+	if (sd->s_type & SYSFS_DIR)
+		dput(dentry);
 
 	/* adjust nlink and update timestamp */
-	dir = parent->d_inode;
-	mutex_lock(&dir->i_mutex);
-
 	curtime = CURRENT_TIME;
 
-	dir->i_ctime = dir->i_mtime = curtime;
+	inode = ilookup(sysfs_sb, sd->s_ino);
+	if (inode) {
+		mutex_lock(&inode->i_mutex);
 
-	if (dentry) {
-		dentry->d_inode->i_ctime = curtime;
-		drop_nlink(dentry->d_inode);
-		if (sd->s_type & SYSFS_DIR) {
-			drop_nlink(dentry->d_inode);
-			drop_nlink(dir);
-			/* XXX: unpin if directory, this will go away soon */
-			dput(dentry);
-		}
+		inode->i_ctime = curtime;
+		drop_nlink(inode);
+		if (sd->s_type & SYSFS_DIR)
+			drop_nlink(inode);
+
+		mutex_unlock(&inode->i_mutex);
+		iput(inode);
 	}
 
-	mutex_unlock(&dir->i_mutex);
+	/* adjust nlink and udpate timestamp of the parent */
+	inode = ilookup(sysfs_sb, sd->s_parent->s_ino);
+	if (inode) {
+		mutex_lock(&inode->i_mutex);
 
-	/* bye bye */
-	if (dentry)
-		dput(dentry);
-	else
-		dput(parent);
+		inode->i_ctime = inode->i_mtime = curtime;
+		if (sd->s_type & SYSFS_DIR)
+			drop_nlink(inode);
+
+		mutex_unlock(&inode->i_mutex);
+		iput(inode);
+	}
 }
 
 int sysfs_hash_and_remove(struct dentry * dir, const char * name)
Signature · 1 line
1.5.0.3
author
#4 lore raw

PATCH 4/11 sysfs: implement sysfs_find_dirent() and sysfs_get_dirent()

In reply to 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.

---
 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(-)
fs/sysfs/dir.c +44 -17
index f2ea006..4762a9a 100644
@@ -317,28 +317,55 @@ void sysfs_attach_dirent(struct sysfs_dirent *sd,
 	}
 }
 
-/*
+/**
+ *	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 *kobj, struct dentry *parent,
 
 	/* 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);
fs/sysfs/file.c +1 -1
index a84b734..e448b88 100644
@@ -421,7 +421,7 @@ int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
 
 	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;
 	}
fs/sysfs/symlink.c +1 -1
index ff605d3..45b62e2 100644
@@ -95,7 +95,7 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 		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);
 
fs/sysfs/sysfs.h +4 -1
index 06b5085..f1629b4 100644
@@ -59,7 +59,10 @@ extern struct inode * sysfs_get_inode(struct sysfs_dirent *sd);
 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,
Signature · 1 line
1.5.0.3
author
#3 lore raw

PATCH 3/11 sysfs: implement SYSFS_FLAG_REMOVED flag

In reply to Tejun Heo

Implement SYSFS_FLAG_REMOVED flag which currently is used only to improve sanity check in sysfs_deactivate(). The flag will be used to make directory entries reclamiable.

---
 fs/sysfs/dir.c        |    4 +++-
 fs/sysfs/inode.c      |    1 +
 include/linux/sysfs.h |    1 +
 3 files changed, 5 insertions(+), 1 deletions(-)
fs/sysfs/dir.c +3 -1
index eb9bc0a..f2ea006 100644
@@ -171,7 +171,7 @@ void sysfs_deactivate(struct sysfs_dirent *sd)
 	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
@@ -506,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));
@@ -540,6 +541,7 @@ static void __sysfs_remove_dir(struct dentry *dentry)
 		struct sysfs_dirent *sd = *pos;
 
 		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;
fs/sysfs/inode.c +1 -0
index ee3a5d9..e2f6ef1 100644
@@ -296,6 +296,7 @@ int sysfs_hash_and_remove(struct dentry * dir, const char * name)
 		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;
include/linux/sysfs.h +1 -0
index 5813550..2a6df64 100644
@@ -84,6 +84,7 @@ struct sysfs_ops {
 #define SYSFS_COPY_NAME		(SYSFS_DIR | SYSFS_KOBJ_LINK)
 
 #define SYSFS_FLAG_MASK		~SYSFS_TYPE_MASK
+#define SYSFS_FLAG_REMOVED	0x0100
 
 #ifdef CONFIG_SYSFS
 
Signature · 1 line
1.5.0.3
author
#7 lore raw

PATCH 7/11 sysfs: use sysfs_mutex to protect the sysfs_dirent tree

In reply to 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_mutex globally instead. As the whole tree is protected with sysfs_mutex, there is no reason to keep sysfs_rename_sem. Drop it.

While at it, add docbook comments to functions which require sysfs_mutex locking.

---
 fs/sysfs/dir.c     |  101 ++++++++++++++++++++++++++++++++++++---------------
 fs/sysfs/file.c    |   31 ++++++++--------
 fs/sysfs/inode.c   |   11 ++----
 fs/sysfs/symlink.c |   51 +++++++++++++-------------
 fs/sysfs/sysfs.h   |    2 +-
 5 files changed, 116 insertions(+), 80 deletions(-)
fs/sysfs/dir.c +71 -30
index 663acee..f10b7b8 100644
@@ -14,7 +14,7 @@
 #include <asm/semaphore.h>
 #include "sysfs.h"
 
-DECLARE_RWSEM(sysfs_rename_sem);
+DEFINE_MUTEX(sysfs_mutex);
 spinlock_t sysfs_assoc_lock = SPIN_LOCK_UNLOCKED;
 
 static spinlock_t sysfs_ino_lock = SPIN_LOCK_UNLOCKED;
@@ -28,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)
+ *	mutex_lock(sysfs_mutex)
  */
 static void sysfs_link_sibling(struct sysfs_dirent *sd)
 {
@@ -47,7 +47,7 @@ static void sysfs_link_sibling(struct sysfs_dirent *sd)
  *	sd->s_parent->s_children.
  *
  *	Locking:
- *	mutex_lock(sd->s_parent->dentry->d_inode->i_mutex)
+ *	mutex_lock(sysfs_mutex)
  */
 static void sysfs_unlink_sibling(struct sysfs_dirent *sd)
 {
@@ -215,6 +215,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
 	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)
@@ -291,6 +294,17 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)
 	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_assoc_lock to avoid race with sysfs_d_iput().
+ *
+ *	LOCKING:
+ *	mutex_lock(sysfs_mutex)
+ */
 static void sysfs_attach_dentry(struct sysfs_dirent *sd, struct dentry *dentry)
 {
 	dentry->d_op = &sysfs_dentry_ops;
@@ -304,6 +318,17 @@ static void sysfs_attach_dentry(struct sysfs_dirent *sd, struct dentry *dentry)
 	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:
+ *	mutex_lock(sysfs_mutex)
+ */
 void sysfs_attach_dirent(struct sysfs_dirent *sd,
 			 struct sysfs_dirent *parent_sd, struct dentry *dentry)
 {
@@ -324,7 +349,7 @@ void sysfs_attach_dirent(struct sysfs_dirent *sd,
  *	Look for sysfs_dirent with name @name under @parent_sd.
  *
  *	LOCKING:
- *	mutex_lock(parent->i_mutex)
+ *	mutex_lock(sysfs_mutex)
  *
  *	RETURNS:
  *	Pointer to sysfs_dirent if found, NULL if not.
@@ -349,7 +374,7 @@ struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
  *	it if found.
  *
  *	LOCKING:
- *	Kernel thread context (may sleep)
+ *	Kernel thread context (may sleep).  Grabs sysfs_mutex.
  *
  *	RETURNS:
  *	Pointer to sysfs_dirent if found, NULL if not.
@@ -359,10 +384,10 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
 {
 	struct sysfs_dirent *sd;
 
-	mutex_lock(&parent_sd->s_dentry->d_inode->i_mutex);
+	mutex_lock(&sysfs_mutex);
 	sd = sysfs_find_dirent(parent_sd, name);
 	sysfs_get(sd);
-	mutex_unlock(&parent_sd->s_dentry->d_inode->i_mutex);
+	mutex_unlock(&sysfs_mutex);
 
 	return sd;
 }
@@ -408,14 +433,20 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
 	}
 
 	/* link in */
+	mutex_lock(&sysfs_mutex);
+
 	error = -EEXIST;
-	if (sysfs_find_dirent(parent_sd, name))
+	if (sysfs_find_dirent(parent_sd, name)) {
+		mutex_unlock(&sysfs_mutex);
 		goto out_iput;
+	}
 
 	sysfs_instantiate(dentry, inode);
 	inc_nlink(parent->d_inode);
 	sysfs_attach_dirent(sd, parent_sd, dentry);
 
+	mutex_unlock(&sysfs_mutex);
+
 	*p_sd = sd;
 	error = 0;
 	goto out_unlock;	/* pin directory dentry in core */
@@ -493,6 +524,8 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
+	mutex_lock(&sysfs_mutex);
+
 	if (inode->i_state & I_NEW) {
 		/* initialize inode according to type */
 		switch (sysfs_type(sd)) {
@@ -516,6 +549,8 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 	sysfs_instantiate(dentry, inode);
 	sysfs_attach_dentry(sd, dentry);
 
+	mutex_unlock(&sysfs_mutex);
+
 	return NULL;
 }
 
@@ -526,17 +561,13 @@ const struct inode_operations sysfs_dir_inode_operations = {
 
 static void remove_dir(struct sysfs_dirent *sd)
 {
-	struct dentry *parent = sd->s_parent->s_dentry;
-
-	mutex_lock(&parent->d_inode->i_mutex);
-
+	mutex_lock(&sysfs_mutex);
 	sysfs_unlink_sibling(sd);
 	sd->s_flags |= SYSFS_FLAG_REMOVED;
+	mutex_unlock(&sysfs_mutex);
 
 	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);
@@ -552,15 +583,12 @@ static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
 {
 	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);
+	mutex_lock(&sysfs_mutex);
 	pos = &dir_sd->s_children;
 	while (*pos) {
 		struct sysfs_dirent *sd = *pos;
@@ -573,7 +601,7 @@ static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
 		} else
 			pos = &(*pos)->s_sibling;
 	}
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&sysfs_mutex);
 
 	while (removed) {
 		struct sysfs_dirent *sd = removed;
@@ -621,7 +649,6 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 	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));
@@ -661,12 +688,16 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 	d_add(new_dentry, NULL);
 	d_move(sd->s_dentry, new_dentry);
 
+	mutex_lock(&sysfs_mutex);
+
 	sysfs_unlink_sibling(sd);
 	sysfs_get(new_parent_sd);
 	sysfs_put(sd->s_parent);
 	sd->s_parent = new_parent_sd;
 	sysfs_link_sibling(sd);
 
+	mutex_unlock(&sysfs_mutex);
+
 	error = 0;
 	goto out_unlock;
 
@@ -678,7 +709,6 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 	dput(new_dentry);
  out_unlock:
 	mutex_unlock(&new_parent->d_inode->i_mutex);
-	up_write(&sysfs_rename_sem);
 	return error;
 }
 
@@ -717,12 +747,15 @@ again:
 	dput(new_dentry);
 
 	/* Remove from old parent's list and insert into new parent's list. */
+	mutex_lock(&sysfs_mutex);
+
 	sysfs_unlink_sibling(sd);
 	sysfs_get(new_parent_sd);
 	sysfs_put(sd->s_parent);
 	sd->s_parent = new_parent_sd;
 	sysfs_link_sibling(sd);
 
+	mutex_unlock(&sysfs_mutex);
 out:
 	mutex_unlock(&new_parent_dentry->d_inode->i_mutex);
 	mutex_unlock(&old_parent_dentry->d_inode->i_mutex);
@@ -736,11 +769,12 @@ static int sysfs_dir_open(struct inode *inode, struct file *file)
 	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) {
+		mutex_lock(&sysfs_mutex);
 		sysfs_attach_dirent(sd, parent_sd, NULL);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+		mutex_unlock(&sysfs_mutex);
+	}
 
 	file->private_data = sd;
 	return sd ? 0 : -ENOMEM;
@@ -748,12 +782,11 @@ static int sysfs_dir_open(struct inode *inode, struct file *file)
 
 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);
+	mutex_lock(&sysfs_mutex);
 	sysfs_unlink_sibling(cursor);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+	mutex_unlock(&sysfs_mutex);
 
 	release_sysfs_dirent(cursor);
 
@@ -794,6 +827,8 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 			i++;
 			/* fallthrough */
 		default:
+			mutex_lock(&sysfs_mutex);
+
 			pos = &parent_sd->s_children;
 			while (*pos != cursor)
 				pos = &(*pos)->s_sibling;
@@ -826,6 +861,8 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 			/* put cursor back in */
 			cursor->s_sibling = *pos;
 			*pos = cursor;
+
+			mutex_unlock(&sysfs_mutex);
 	}
 	return 0;
 }
@@ -834,7 +871,6 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
 {
 	struct dentry * dentry = file->f_path.dentry;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
 	switch (origin) {
 		case 1:
 			offset += file->f_pos;
@@ -842,10 +878,11 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
 			if (offset >= 0)
 				break;
 		default:
-			mutex_unlock(&file->f_path.dentry->d_inode->i_mutex);
 			return -EINVAL;
 	}
 	if (offset != file->f_pos) {
+		mutex_lock(&sysfs_mutex);
+
 		file->f_pos = offset;
 		if (file->f_pos >= 2) {
 			struct sysfs_dirent *sd = dentry->d_fsdata;
@@ -866,8 +903,10 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
 			cursor->s_sibling = *pos;
 			*pos = cursor;
 		}
+
+		mutex_unlock(&sysfs_mutex);
 	}
-	mutex_unlock(&dentry->d_inode->i_mutex);
+
 	return offset;
 }
 
@@ -933,7 +972,9 @@ struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj)
 	sd->s_elem.dir.kobj = kobj;
 	/* point to parent_sd but don't attach to it */
 	sd->s_parent = sysfs_get(parent_sd);
+	mutex_lock(&sysfs_mutex);
 	sysfs_attach_dirent(sd, NULL, shadow);
+	mutex_unlock(&sysfs_mutex);
 
 	d_instantiate(shadow, igrab(inode));
 	inc_nlink(inode);
fs/sysfs/file.c +15 -16
index acc9890..c14e607 100644
@@ -415,29 +415,28 @@ const struct file_operations sysfs_file_operations = {
 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;
-	}
+	mutex_lock(&sysfs_mutex);
 
-	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;
+	mutex_unlock(&sysfs_mutex);
+
+	if (sd) {
+		sysfs_put(sd);
+		return -EEXIST;
+	}
+	return 0;
 }
 
 
fs/sysfs/inode.c +3 -8
index e4c2393..d439c0b 100644
@@ -277,20 +277,14 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 
 int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
 {
-	struct dentry *dir;
 	struct sysfs_dirent **pos, *sd;
 	int found = 0;
 
 	if (!dir_sd)
 		return -ENOENT;
 
-	dir = dir_sd->s_dentry;
+	mutex_lock(&sysfs_mutex);
 
-	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 = &dir_sd->s_children; *pos; pos = &(*pos)->s_sibling) {
 		sd = *pos;
 
@@ -304,7 +298,8 @@ int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
 			break;
 		}
 	}
-	mutex_unlock(&dir->d_inode->i_mutex);
+
+	mutex_unlock(&sysfs_mutex);
 
 	if (!found)
 		return -ENOENT;
fs/sysfs/symlink.c +26 -25

87 lines not on this page · raw message

fs/sysfs/sysfs.h +1 -1
index 4572677..2605161 100644
@@ -82,7 +82,7 @@ extern void sysfs_drop_dentry(struct sysfs_dirent *sd);
 extern int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
 
 extern spinlock_t sysfs_assoc_lock;
-extern struct rw_semaphore sysfs_rename_sem;
+extern struct mutex sysfs_mutex;
 extern struct super_block * sysfs_sb;
 extern const struct file_operations sysfs_dir_operations;
 extern const struct file_operations sysfs_file_operations;
Signature · 1 line
1.5.0.3
author
#6 lore raw

PATCH 6/11 sysfs: consolidate sysfs spinlocks

In reply to Tejun Heo

Replace sysfs_lock and kobj_sysfs_assoc_lock with sysfs_assoc_lock. sysfs_lock was originally to be used to protect sysfs_dirent tree but mutex seems better choice, so there is no reason to keep sysfs_lock separate. Merge the two spinlocks into one.

---
 fs/sysfs/dir.c     |   19 +++++++++----------
 fs/sysfs/inode.c   |   16 ++++++++--------
 fs/sysfs/symlink.c |    6 +++---
 fs/sysfs/sysfs.h   |    3 +--
 4 files changed, 21 insertions(+), 23 deletions(-)
fs/sysfs/dir.c +9 -10
index 950e679..663acee 100644
@@ -15,8 +15,7 @@
 #include "sysfs.h"
 
 DECLARE_RWSEM(sysfs_rename_sem);
-spinlock_t sysfs_lock = SPIN_LOCK_UNLOCKED;
-spinlock_t kobj_sysfs_assoc_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t sysfs_assoc_lock = SPIN_LOCK_UNLOCKED;
 
 static spinlock_t sysfs_ino_lock = SPIN_LOCK_UNLOCKED;
 static DEFINE_IDA(sysfs_ino_ida);
@@ -236,10 +235,10 @@ static void sysfs_d_iput(struct dentry * dentry, struct inode * inode)
 	struct sysfs_dirent * sd = dentry->d_fsdata;
 
 	if (sd) {
-		/* sd->s_dentry is protected with sysfs_lock.  This
-		 * allows sysfs_drop_dentry() to dereference it.
+		/* sd->s_dentry is protected with sysfs_assoc_lock.
+		 * This allows sysfs_drop_dentry() to dereference it.
 		 */
-		spin_lock(&sysfs_lock);
+		spin_lock(&sysfs_assoc_lock);
 
 		/* The dentry might have been deleted or another
 		 * lookup could have happened updating sd->s_dentry to
@@ -248,7 +247,7 @@ static void sysfs_d_iput(struct dentry * dentry, struct inode * inode)
 		 */
 		if (sd->s_dentry == dentry)
 			sd->s_dentry = NULL;
-		spin_unlock(&sysfs_lock);
+		spin_unlock(&sysfs_assoc_lock);
 		sysfs_put(sd);
 	}
 	iput(inode);
@@ -298,9 +297,9 @@ static void sysfs_attach_dentry(struct sysfs_dirent *sd, struct dentry *dentry)
 	dentry->d_fsdata = sysfs_get(sd);
 
 	/* protect sd->s_dentry against sysfs_d_iput */
-	spin_lock(&sysfs_lock);
+	spin_lock(&sysfs_assoc_lock);
 	sd->s_dentry = dentry;
-	spin_unlock(&sysfs_lock);
+	spin_unlock(&sysfs_assoc_lock);
 
 	d_rehash(dentry);
 }
@@ -603,9 +602,9 @@ void sysfs_remove_dir(struct kobject * kobj)
 {
 	struct sysfs_dirent *sd = kobj->sd;
 
-	spin_lock(&kobj_sysfs_assoc_lock);
+	spin_lock(&sysfs_assoc_lock);
 	kobj->sd = NULL;
-	spin_unlock(&kobj_sysfs_assoc_lock);
+	spin_unlock(&sysfs_assoc_lock);
 
 	__sysfs_remove_dir(sd);
 }
fs/sysfs/inode.c +8 -8
index 1be8537..e4c2393 100644
@@ -211,11 +211,11 @@ void sysfs_instantiate(struct dentry *dentry, struct inode *inode)
  *	parent on entry to this function such that it can't be looked
  *	up anymore.
  *
- *	@sd->s_dentry which is protected with sysfs_lock points to the
- *	currently associated dentry but we're not holding a reference
- *	to it and racing with dput().  Grab dcache_lock and verify
- *	dentry before dropping it.  If @sd->s_dentry is NULL or dput()
- *	beats us, no need to bother.
+ *	@sd->s_dentry which is protected with sysfs_assoc_lock points
+ *	to the currently associated dentry but we're not holding a
+ *	reference to it and racing with dput().  Grab dcache_lock and
+ *	verify dentry before dropping it.  If @sd->s_dentry is NULL or
+ *	dput() beats us, no need to bother.
  */
 void sysfs_drop_dentry(struct sysfs_dirent *sd)
 {
@@ -224,9 +224,9 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 	struct inode *inode;
 
 	/* We're not holding a reference to ->s_dentry dentry but the
-	 * field will stay valid as long as sysfs_lock is held.
+	 * field will stay valid as long as sysfs_assoc_lock is held.
 	 */
-	spin_lock(&sysfs_lock);
+	spin_lock(&sysfs_assoc_lock);
 	spin_lock(&dcache_lock);
 
 	/* drop dentry if it's there and dput() didn't kill it yet */
@@ -238,7 +238,7 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 	}
 
 	spin_unlock(&dcache_lock);
-	spin_unlock(&sysfs_lock);
+	spin_unlock(&sysfs_assoc_lock);
 
 	dput(dentry);
 	/* XXX: unpin if directory, this will go away soon */
fs/sysfs/symlink.c +3 -3
index 43cc522..cbd95a4 100644
@@ -82,12 +82,12 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 		return -EFAULT;
 
 	/* target->sd can go away beneath us but is protected with
-	 * kobj_sysfs_assoc_lock.  Fetch target_sd from it.
+	 * sysfs_assoc_lock.  Fetch target_sd from it.
 	 */
-	spin_lock(&kobj_sysfs_assoc_lock);
+	spin_lock(&sysfs_assoc_lock);
 	if (target->sd)
 		target_sd = sysfs_get(target->sd);
-	spin_unlock(&kobj_sysfs_assoc_lock);
+	spin_unlock(&sysfs_assoc_lock);
 
 	if (!target_sd)
 		return -ENOENT;
fs/sysfs/sysfs.h +1 -2
index 27a5f4b..4572677 100644
@@ -81,8 +81,7 @@ 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);
 
-extern spinlock_t sysfs_lock;
-extern spinlock_t kobj_sysfs_assoc_lock;
+extern spinlock_t sysfs_assoc_lock;
 extern struct rw_semaphore sysfs_rename_sem;
 extern struct super_block * sysfs_sb;
 extern const struct file_operations sysfs_dir_operations;
Signature · 1 line
1.5.0.3
author
#8 lore raw

PATCH 8/11 sysfs: restructure add/remove paths and fix inode update

In reply to Tejun Heo

The original add/remove code had the following problems.

* parent's timestamps are updated on dentry instantiation. this is incorrect with reclaimable files.

* updating parent's timestamps isn't synchronized.

* parent nlink update assumes the inode is accessible which won't be true once directory dentries are made reclaimable.

This patch restructures add/remove paths to resolve the above problems. Add/removal are done in the following steps.

1. sysfs_addrm_start() : acquire locks including sysfs_mutex and other resources.

2-a. sysfs_add_one() : add new sd. linking the new sd into the
children list is caller's responsibility.

2-b. sysfs_remove_one() : remove a sd. unlinking the sd from the
children list is caller's responsibility.

3. sysfs_addrm_finish() : release all resources and clean up.

Steps 2-a and/or 2-b can be repeated multiple times.

Parent's inode is looked up during sysfs_addrm_start(). If available (always at the moment), it's pinned and nlink is updated as sd's are added and removed. Timestamps are updated during finish if any sd has been added or removed. If parent's inode is not available during start, sysfs_mutex ensures that parent inode is not created till add/remove is complete.

All the complexity is contained inside the helper functions. Especially, dentry/inode handling is properly hidden from the rest of sysfs which now mostly operate on sysfs_dirents. As an added bonus, codes which use these helpers to add and remove sysfs_dirents are now more structured and simpler.

---
 fs/sysfs/dir.c     |  250 ++++++++++++++++++++++++++++++++++++++-------------
 fs/sysfs/file.c    |   17 ++--
 fs/sysfs/inode.c   |   46 ++--------
 fs/sysfs/symlink.c |   20 +++--
 fs/sysfs/sysfs.h   |   20 ++++-
 5 files changed, 229 insertions(+), 124 deletions(-)
fs/sysfs/dir.c +186 -64
index f10b7b8..dce74ef 100644
@@ -30,7 +30,7 @@ static DEFINE_IDA(sysfs_ino_ida);
  *	Locking:
  *	mutex_lock(sysfs_mutex)
  */
-static void sysfs_link_sibling(struct sysfs_dirent *sd)
+void sysfs_link_sibling(struct sysfs_dirent *sd)
 {
 	struct sysfs_dirent *parent_sd = sd->s_parent;
 
@@ -49,7 +49,7 @@ static void sysfs_link_sibling(struct sysfs_dirent *sd)
  *	Locking:
  *	mutex_lock(sysfs_mutex)
  */
-static void sysfs_unlink_sibling(struct sysfs_dirent *sd)
+void sysfs_unlink_sibling(struct sysfs_dirent *sd)
 {
 	struct sysfs_dirent **pos;
 
@@ -165,7 +165,7 @@ void sysfs_put_active_two(struct sysfs_dirent *sd)
  *
  *	Deny new active references and drain existing ones.
  */
-void sysfs_deactivate(struct sysfs_dirent *sd)
+static void sysfs_deactivate(struct sysfs_dirent *sd)
 {
 	DECLARE_COMPLETION_ONSTACK(wait);
 	int v;
@@ -318,27 +318,164 @@ static void sysfs_attach_dentry(struct sysfs_dirent *sd, struct dentry *dentry)
 	d_rehash(dentry);
 }
 
+static int sysfs_ilookup_test(struct inode *inode, void *arg)
+{
+	struct sysfs_dirent *sd = arg;
+	return inode->i_ino == sd->s_ino;
+}
+
 /**
- *	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)
+ *	sysfs_addrm_start - prepare for sysfs_dirent add/remove
+ *	@acxt: pointer to sysfs_addrm_cxt to be used
+ *	@parent_sd: parent sysfs_dirent
  *
- *	Attach @sd to @parent_sd and/or @dentry.  Both are optional.
+ *	This function is called when the caller is about to add or
+ *	remove sysfs_dirent under @parent_sd.  This function acquires
+ *	sysfs_mutex, grabs inode for @parent_sd if available and lock
+ *	i_mutex of it.  @acxt is used to keep and pass context to
+ *	other addrm functions.
  *
  *	LOCKING:
- *	mutex_lock(sysfs_mutex)
+ *	Kernel thread context (may sleep).  sysfs_mutex is locked on
+ *	return.  i_mutex of parent inode is locked on return if
+ *	available.
  */
-void sysfs_attach_dirent(struct sysfs_dirent *sd,
-			 struct sysfs_dirent *parent_sd, struct dentry *dentry)
+void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
+		       struct sysfs_dirent *parent_sd)
 {
-	if (dentry)
-		sysfs_attach_dentry(sd, dentry);
+	struct inode *inode;
 
-	if (parent_sd) {
-		sd->s_parent = sysfs_get(parent_sd);
-		sysfs_link_sibling(sd);
+	memset(acxt, 0, sizeof(*acxt));
+	acxt->parent_sd = parent_sd;
+
+	/* Lookup parent inode.  inode initialization and I_NEW
+	 * clearing are protected by sysfs_mutex.  By grabbing it and
+	 * looking up with _nowait variant, inode state can be
+	 * determined reliably.
+	 */
+	mutex_lock(&sysfs_mutex);
+
+	inode = ilookup5_nowait(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test,
+				parent_sd);
+
+	if (inode && !(inode->i_state & I_NEW)) {
+		/* parent inode available */
+		acxt->parent_inode = inode;
+
+		/* sysfs_mutex is below i_mutex in lock hierarchy.
+		 * First, trylock i_mutex.  If fails, unlock
+		 * sysfs_mutex and lock them in order.
+		 */
+		if (!mutex_trylock(&inode->i_mutex)) {
+			mutex_unlock(&sysfs_mutex);
+			mutex_lock(&inode->i_mutex);
+			mutex_lock(&sysfs_mutex);
+		}
+	} else
+		iput(inode);
+}
+
+/**
+ *	sysfs_add_one - add sysfs_dirent to parent
+ *	@acxt: addrm context to use
+ *	@sd: sysfs_dirent to be added
+ *
+ *	Get @acxt->parent_sd and set sd->s_parent to it and increment
+ *	nlink of parent inode if @sd is a directory.  @sd is NOT
+ *	linked into the children list of the parent.  The caller
+ *	should invoke sysfs_link_sibling() after this function
+ *	completes if @sd needs to be on the children list.
+ *
+ *	This function should be called between calls to
+ *	sysfs_addrm_start() and sysfs_addrm_finish() and should be
+ *	passed the same @acxt as passed to sysfs_addrm_start().
+ *
+ *	LOCKING:
+ *	Determined by sysfs_addrm_start().
+ */
+void sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
+{
+	sd->s_parent = sysfs_get(acxt->parent_sd);
+
+	if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
+		inc_nlink(acxt->parent_inode);
+
+	acxt->cnt++;
+}
+
+/**
+ *	sysfs_remove_one - remove sysfs_dirent from parent
+ *	@acxt: addrm context to use
+ *	@sd: sysfs_dirent to be added
+ *
+ *	Mark @sd removed and drop nlink of parent inode if @sd is a
+ *	directory.  @sd is NOT unlinked from the children list of the
+ *	parent.  The caller is repsonsible for removing @sd from the
+ *	children list before calling this function.
+ *
+ *	This function should be called between calls to
+ *	sysfs_addrm_start() and sysfs_addrm_finish() and should be
+ *	passed the same @acxt as passed to sysfs_addrm_start().
+ *
+ *	LOCKING:
+ *	Determined by sysfs_addrm_start().
+ */
+void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
+{
+	BUG_ON(sd->s_sibling || (sd->s_flags & SYSFS_FLAG_REMOVED));
+
+	sd->s_flags |= SYSFS_FLAG_REMOVED;
+	sd->s_sibling = acxt->removed;
+	acxt->removed = sd;
+
+	if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
+		drop_nlink(acxt->parent_inode);
+
+	acxt->cnt++;
+}
+
+/**
+ *	sysfs_addrm_finish - finish up sysfs_dirent add/remove
+ *	@acxt: addrm context to finish up
+ *
+ *	Finish up sysfs_dirent add/remove.  Resources acquired by
+ *	sysfs_addrm_start() are released and removed sysfs_dirents are
+ *	cleaned up.  Timestamps on the parent inode are updated.
+ *
+ *	LOCKING:
+ *	All mutexes acquired by sysfs_addrm_start() are released.
+ *
+ *	RETURNS:
+ *	Number of added/removed sysfs_dirents since sysfs_addrm_start().
+ */
+int sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
+{
+	/* release resources acquired by sysfs_addrm_start() */
+	mutex_unlock(&sysfs_mutex);
+	if (acxt->parent_inode) {
+		struct inode *inode = acxt->parent_inode;
+
+		/* if added/removed, update timestamps on the parent */
+		if (acxt->cnt)
+			inode->i_ctime = inode->i_mtime = CURRENT_TIME;
+
+		mutex_unlock(&inode->i_mutex);
+		iput(inode);
+	}
+
+	/* kill removed sysfs_dirents */
+	while (acxt->removed) {
+		struct sysfs_dirent *sd = acxt->removed;
+
+		acxt->removed = sd->s_sibling;
+		sd->s_sibling = NULL;
+
+		sysfs_drop_dentry(sd);
+		sysfs_deactivate(sd);
+		sysfs_put(sd);
 	}
+
+	return acxt->cnt;
 }
 
 /**
@@ -396,19 +533,20 @@ 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;
+	struct sysfs_addrm_cxt acxt;
 	int error;
 	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
 	struct dentry *dentry;
 	struct inode *inode;
 	struct sysfs_dirent *sd;
 
-	mutex_lock(&parent->d_inode->i_mutex);
+	sysfs_addrm_start(&acxt, parent_sd);
 
 	/* allocate */
 	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry)) {
 		error = PTR_ERR(dentry);
-		goto out_unlock;
+		goto out_finish;
 	}
 
 	error = -EEXIST;
@@ -433,23 +571,18 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
 	}
 
 	/* link in */
-	mutex_lock(&sysfs_mutex);
-
 	error = -EEXIST;
-	if (sysfs_find_dirent(parent_sd, name)) {
-		mutex_unlock(&sysfs_mutex);
+	if (sysfs_find_dirent(parent_sd, name))
 		goto out_iput;
-	}
 
+	sysfs_add_one(&acxt, sd);
+	sysfs_link_sibling(sd);
 	sysfs_instantiate(dentry, inode);
-	inc_nlink(parent->d_inode);
-	sysfs_attach_dirent(sd, parent_sd, dentry);
-
-	mutex_unlock(&sysfs_mutex);
+	sysfs_attach_dentry(sd, dentry);
 
 	*p_sd = sd;
 	error = 0;
-	goto out_unlock;	/* pin directory dentry in core */
+	goto out_finish;	/* pin directory dentry in core */
 
  out_iput:
 	iput(inode);
@@ -459,8 +592,8 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
 	d_drop(dentry);
  out_dput:
 	dput(dentry);
- out_unlock:
-	mutex_unlock(&parent->d_inode->i_mutex);
+ out_finish:
+	sysfs_addrm_finish(&acxt);
 	return error;
 }
 
@@ -561,16 +694,12 @@ const struct inode_operations sysfs_dir_inode_operations = {
 
 static void remove_dir(struct sysfs_dirent *sd)
 {
-	mutex_lock(&sysfs_mutex);
-	sysfs_unlink_sibling(sd);
-	sd->s_flags |= SYSFS_FLAG_REMOVED;
-	mutex_unlock(&sysfs_mutex);
+	struct sysfs_addrm_cxt acxt;
 
-	pr_debug(" o %s removing done\n", sd->s_name);
-
-	sysfs_drop_dentry(sd);
-	sysfs_deactivate(sd);
-	sysfs_put(sd);
+	sysfs_addrm_start(&acxt, sd->s_parent);
+	sysfs_unlink_sibling(sd);
+	sysfs_remove_one(&acxt, sd);
+	sysfs_addrm_finish(&acxt);
 }
 
 void sysfs_remove_subdir(struct sysfs_dirent *sd)
@@ -581,38 +710,26 @@ void sysfs_remove_subdir(struct sysfs_dirent *sd)
 
 static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
 {
-	struct sysfs_dirent *removed = NULL;
+	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent **pos;
 
 	if (!dir_sd)
 		return;
 
 	pr_debug("sysfs %s: removing dir\n", dir_sd->s_name);
-	mutex_lock(&sysfs_mutex);
+	sysfs_addrm_start(&acxt, dir_sd);
 	pos = &dir_sd->s_children;
 	while (*pos) {
 		struct sysfs_dirent *sd = *pos;
 
 		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;
+			sd->s_sibling = NULL;
+			sysfs_remove_one(&acxt, sd);
 		} else
 			pos = &(*pos)->s_sibling;
 	}
-	mutex_unlock(&sysfs_mutex);
-
-	while (removed) {
-		struct sysfs_dirent *sd = removed;
-
-		removed = sd->s_sibling;
-		sd->s_sibling = NULL;
-
-		sysfs_drop_dentry(sd);
-		sysfs_deactivate(sd);
-		sysfs_put(sd);
-	}
+	sysfs_addrm_finish(&acxt);
 
 	remove_dir(dir_sd);
 }
@@ -772,7 +889,8 @@ static int sysfs_dir_open(struct inode *inode, struct file *file)
 	sd = sysfs_new_dirent("_DIR_", 0, 0);
 	if (sd) {
 		mutex_lock(&sysfs_mutex);
-		sysfs_attach_dirent(sd, parent_sd, NULL);
+		sd->s_parent = sysfs_get(parent_sd);
+		sysfs_link_sibling(sd);
 		mutex_unlock(&sysfs_mutex);
 	}
 
@@ -957,6 +1075,7 @@ struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj)
 	struct sysfs_dirent *parent_sd = parent->d_fsdata;
 	struct dentry *shadow;
 	struct sysfs_dirent *sd;
+	struct sysfs_addrm_cxt acxt;
 
 	sd = ERR_PTR(-EINVAL);
 	if (!sysfs_is_shadowed_inode(inode))
@@ -970,15 +1089,18 @@ struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj)
 	if (!sd)
 		goto nomem;
 	sd->s_elem.dir.kobj = kobj;
-	/* point to parent_sd but don't attach to it */
-	sd->s_parent = sysfs_get(parent_sd);
-	mutex_lock(&sysfs_mutex);
-	sysfs_attach_dirent(sd, NULL, shadow);
-	mutex_unlock(&sysfs_mutex);
 
+	sysfs_addrm_start(&acxt, parent_sd);
+
+	/* add but don't link into children list */
+	sysfs_add_one(&acxt, sd);
+
+	/* attach and instantiate dentry */
+	sysfs_attach_dentry(sd, shadow);
 	d_instantiate(shadow, igrab(inode));
-	inc_nlink(inode);
-	inc_nlink(parent->d_inode);
+	inc_nlink(inode);	/* tj: synchronization? */
+
+	sysfs_addrm_finish(&acxt);
 
 	dget(shadow);		/* Extra count - pin the dentry in core */
 
fs/sysfs/file.c +8 -9
index c14e607..ccda618 100644
@@ -416,6 +416,7 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
 		   int type)
 {
 	umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
+	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent *sd;
 
 	sd = sysfs_new_dirent(attr->name, mode, type);
@@ -423,20 +424,18 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
 		return -ENOMEM;
 	sd->s_elem.attr.attr = (void *)attr;
 
-	mutex_lock(&sysfs_mutex);
+	sysfs_addrm_start(&acxt, dir_sd);
 
 	if (!sysfs_find_dirent(dir_sd, attr->name)) {
-		sysfs_attach_dirent(sd, dir_sd, NULL);
-		sd = NULL;
+		sysfs_add_one(&acxt, sd);
+		sysfs_link_sibling(sd);
 	}
 
-	mutex_unlock(&sysfs_mutex);
+	if (sysfs_addrm_finish(&acxt))
+		return 0;
 
-	if (sd) {
-		sysfs_put(sd);
-		return -EEXIST;
-	}
-	return 0;
+	sysfs_put(sd);
+	return -EEXIST;
 }
 
 
fs/sysfs/inode.c +8 -38

95 lines not on this page · raw message

fs/sysfs/symlink.c +11 -9
index 683316f..2f86e04 100644
@@ -55,6 +55,7 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 	struct sysfs_dirent *parent_sd = NULL;
 	struct sysfs_dirent *target_sd = NULL;
 	struct sysfs_dirent *sd = NULL;
+	struct sysfs_addrm_cxt acxt;
 	int error;
 
 	BUG_ON(!name);
@@ -87,17 +88,18 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 		goto out_put;
 	sd->s_elem.symlink.target_sd = target_sd;
 
-	mutex_lock(&sysfs_mutex);
-	error = -EEXIST;
-	if (sysfs_find_dirent(parent_sd, name))
-		goto out_unlock;
-	sysfs_attach_dirent(sd, parent_sd, NULL);
-	mutex_unlock(&sysfs_mutex);
+	sysfs_addrm_start(&acxt, parent_sd);
 
-	return 0;
+	if (!sysfs_find_dirent(parent_sd, name)) {
+		sysfs_add_one(&acxt, sd);
+		sysfs_link_sibling(sd);
+	}
 
- out_unlock:
-	mutex_unlock(&sysfs_mutex);
+	if (sysfs_addrm_finish(&acxt))
+		return 0;
+
+	error = -EEXIST;
+	/* fall through */
  out_put:
 	sysfs_put(target_sd);
 	sysfs_put(sd);
fs/sysfs/sysfs.h +16 -4
index 2605161..3e9a5ee 100644
@@ -44,14 +44,29 @@ struct sysfs_dirent {
 
 #define SD_DEACTIVATED_BIAS	INT_MIN
 
+struct sysfs_addrm_cxt {
+	struct sysfs_dirent	*parent_sd;
+	struct inode		*parent_inode;
+	struct sysfs_dirent	*removed;
+	int			cnt;
+};
+
 extern struct vfsmount * sysfs_mount;
 extern struct kmem_cache *sysfs_dir_cachep;
 
+extern void sysfs_link_sibling(struct sysfs_dirent *sd);
+extern void sysfs_unlink_sibling(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);
 extern void sysfs_put_active_two(struct sysfs_dirent *sd);
-extern void sysfs_deactivate(struct sysfs_dirent *sd);
+extern void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
+			      struct sysfs_dirent *parent_sd);
+extern void sysfs_add_one(struct sysfs_addrm_cxt *acxt,
+			  struct sysfs_dirent *sd);
+extern void sysfs_remove_one(struct sysfs_addrm_cxt *acxt,
+			     struct sysfs_dirent *sd);
+extern int sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt);
 
 extern void sysfs_delete_inode(struct inode *inode);
 extern void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode);
@@ -65,9 +80,6 @@ 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,
-				struct sysfs_dirent *parent_sd,
-				struct dentry *dentry);
 
 extern int sysfs_add_file(struct sysfs_dirent *dir_sd,
 			  const struct attribute *attr, int type);
Signature · 1 line
1.5.0.3
author
#5 lore raw

PATCH 5/11 sysfs: make kobj point to sysfs_dirent instead of dentry

In reply to 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.

---
 fs/sysfs/bin.c          |    6 +-
 fs/sysfs/dir.c          |  119 +++++++++++++++++++++++------------------------
 fs/sysfs/file.c         |   47 +++++++++---------
 fs/sysfs/group.c        |   54 ++++++++++-----------
 fs/sysfs/inode.c        |   11 ++--
 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, 155 insertions(+), 152 deletions(-)
fs/sysfs/bin.c +3 -3
index 3c5574a..55796bd 100644
@@ -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 * kobj, struct bin_attribute * attr)
 
 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);
fs/sysfs/dir.c +59 -60

294 lines not on this page · raw message

fs/sysfs/file.c +24 -23

112 lines not on this page · raw message

fs/sysfs/group.c +26 -28

96 lines not on this page · raw message

fs/sysfs/inode.c +6 -5
index e2f6ef1..1be8537 100644
@@ -275,22 +275,23 @@ void sysfs_drop_dentry(struct sysfs_dirent *sd)
 	}
 }
 
-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;
 	struct sysfs_dirent **pos, *sd;
-	struct sysfs_dirent *parent_sd;
 	int found = 0;
 
-	if (!dir)
+	if (!dir_sd)
 		return -ENOENT;
 
+	dir = dir_sd->s_dentry;
+
 	if (dir->d_inode == NULL)
 		/* no inode means this hasn't been made visible yet */
 		return -ENOENT;
 
-	parent_sd = dir->d_fsdata;
 	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))
fs/sysfs/symlink.c +10 -12
index 45b62e2..43cc522 100644
@@ -66,7 +66,6 @@ static int sysfs_add_link(struct sysfs_dirent * parent_sd, const char * name,
  */
 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 * kobj, struct kobject * target, const char
 
 	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 * kobj, struct kobject * target, const char
 
 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,
fs/sysfs/sysfs.h +6 -4
index f1629b4..27a5f4b 100644
@@ -69,12 +69,14 @@ extern void sysfs_attach_dirent(struct sysfs_dirent *sd,
 				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);
include/linux/kobject.h +5 -4
index c288e41..06cbf41 100644
@@ -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 *);
include/linux/sysfs.h +11 -8
index 2a6df64..4c43030 100644
@@ -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(struct kobject *kobj,
 		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, char *dir, char *attr);
 
 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_callback(struct kobject *kobj,
 	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(struct kobject * k)
 	;
 }
 
-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;
 }
lib/kobject.c +5 -5
index ac15206..dc2bfbb 100644
@@ -44,7 +44,7 @@ static int populate_dir(struct kobject * kobj)
 	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)) {
@@ -162,7 +162,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;
@@ -338,7 +338,7 @@ int kobject_rename(struct kobject * kobj, const char *new_name)
 	/* 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
@@ -361,8 +361,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;
 
Signature · 1 line
1.5.0.3
author
#9 lore raw

PATCH 9/11 sysfs: move sysfs_drop_dentry() to dir.c and make it static

In reply to Tejun Heo

After add/remove path restructuring, the only user of sysfs_drop_dentry() is sysfs_addrm_finish(). Move sysfs_drop_dentry() to dir.c and make it static.

---
 fs/sysfs/dir.c   |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/sysfs/inode.c |   56 ------------------------------------------------------
 fs/sysfs/sysfs.h |    1 -
 3 files changed, 56 insertions(+), 57 deletions(-)
fs/sysfs/dir.c +56 -0
index dce74ef..c141391 100644
@@ -435,6 +435,62 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
 }
 
 /**
+ *	sysfs_drop_dentry - drop dentry for the specified sysfs_dirent
+ *	@sd: target sysfs_dirent
+ *
+ *	Drop dentry for @sd.  @sd must have been unlinked from its
+ *	parent on entry to this function such that it can't be looked
+ *	up anymore.
+ *
+ *	@sd->s_dentry which is protected with sysfs_assoc_lock points
+ *	to the currently associated dentry but we're not holding a
+ *	reference to it and racing with dput().  Grab dcache_lock and
+ *	verify dentry before dropping it.  If @sd->s_dentry is NULL or
+ *	dput() beats us, no need to bother.
+ */
+static void sysfs_drop_dentry(struct sysfs_dirent *sd)
+{
+	struct dentry *dentry = NULL;
+	struct inode *inode;
+
+	/* We're not holding a reference to ->s_dentry dentry but the
+	 * field will stay valid as long as sysfs_assoc_lock is held.
+	 */
+	spin_lock(&sysfs_assoc_lock);
+	spin_lock(&dcache_lock);
+
+	/* drop dentry if it's there and dput() didn't kill it yet */
+	if (sd->s_dentry && sd->s_dentry->d_inode) {
+		dentry = dget_locked(sd->s_dentry);
+		spin_lock(&dentry->d_lock);
+		__d_drop(dentry);
+		spin_unlock(&dentry->d_lock);
+	}
+
+	spin_unlock(&dcache_lock);
+	spin_unlock(&sysfs_assoc_lock);
+
+	dput(dentry);
+	/* XXX: unpin if directory, this will go away soon */
+	if (sysfs_type(sd) == SYSFS_DIR)
+		dput(dentry);
+
+	/* adjust nlink and update timestamp */
+	inode = ilookup(sysfs_sb, sd->s_ino);
+	if (inode) {
+		mutex_lock(&inode->i_mutex);
+
+		inode->i_ctime = CURRENT_TIME;
+		drop_nlink(inode);
+		if (sysfs_type(sd) == SYSFS_DIR)
+			drop_nlink(inode);
+
+		mutex_unlock(&inode->i_mutex);
+		iput(inode);
+	}
+}
+
+/**
  *	sysfs_addrm_finish - finish up sysfs_dirent add/remove
  *	@acxt: addrm context to finish up
  *
fs/sysfs/inode.c +0 -56

62 lines not on this page · raw message

fs/sysfs/sysfs.h +0 -1
index 3e9a5ee..92fe1e5 100644
@@ -90,7 +90,6 @@ 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);
 
 extern spinlock_t sysfs_assoc_lock;
Signature · 1 line
1.5.0.3
author
#10 lore raw

PATCH 10/11 sysfs: implement sysfs_get_dentry()

In reply to 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.

---
 fs/sysfs/dir.c   |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/sysfs/sysfs.h |    1 +
 2 files changed, 99 insertions(+), 0 deletions(-)
fs/sysfs/dir.c +98 -0
index c141391..0cd91ba 100644
@@ -63,6 +63,104 @@ void sysfs_unlink_sibling(struct sysfs_dirent *sd)
 }
 
 /**
+ *	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.
+	 */
+	mutex_lock(&sysfs_mutex);
+ restart0:
+	spin_lock(&sysfs_assoc_lock);
+ restart1:
+	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);
+	spin_unlock(&sysfs_assoc_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 restart0;
+		}
+
+		sysfs_get(cur);
+
+		mutex_unlock(&sysfs_mutex);
+
+		/* 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;
+		}
+
+		mutex_lock(&sysfs_mutex);
+		spin_lock(&sysfs_assoc_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 restart1;
+		}
+
+		spin_unlock(&sysfs_assoc_lock);
+
+		sysfs_put(cur);
+	}
+
+	mutex_unlock(&sysfs_mutex);
+	return dentry;
+}
+
+/**
  *	sysfs_get_active - get an active reference to sysfs_dirent
  *	@sd: sysfs_dirent to get an active reference to
  *
fs/sysfs/sysfs.h +1 -0
index 92fe1e5..72530dc 100644
@@ -54,6 +54,7 @@ struct sysfs_addrm_cxt {
 extern struct vfsmount * sysfs_mount;
 extern struct kmem_cache *sysfs_dir_cachep;
 
+extern struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd);
 extern void sysfs_link_sibling(struct sysfs_dirent *sd);
 extern void sysfs_unlink_sibling(struct sysfs_dirent *sd);
 extern struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd);
Signature · 1 line
1.5.0.3
author
#11 lore raw

PATCH 11/11 sysfs: make directory dentries and inodes reclaimable

In reply to 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.

---
 fs/sysfs/dir.c        |  231 +++++++++++++++++++++++++++----------------------
 fs/sysfs/file.c       |  134 +++++++++++++----------------
 fs/sysfs/mount.c      |    2 +-
 fs/sysfs/sysfs.h      |    1 +
 include/linux/sysfs.h |    1 -
 5 files changed, 187 insertions(+), 182 deletions(-)
fs/sysfs/dir.c +126 -105
index 0cd91ba..9d30fa9 100644
@@ -568,10 +568,10 @@ static void sysfs_drop_dentry(struct sysfs_dirent *sd)
 	spin_unlock(&dcache_lock);
 	spin_unlock(&sysfs_assoc_lock);
 
-	dput(dentry);
-	/* XXX: unpin if directory, this will go away soon */
-	if (sysfs_type(sd) == SYSFS_DIR)
+	/* dentries for shadowed inodes are pinned, unpin */
+	if (dentry && sysfs_is_shadowed_inode(dentry->d_inode))
 		dput(dentry);
+	dput(dentry);
 
 	/* adjust nlink and update timestamp */
 	inode = ilookup(sysfs_sb, sd->s_ino);
@@ -686,69 +686,29 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
 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;
-	struct sysfs_addrm_cxt acxt;
-	int error;
 	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
-	struct dentry *dentry;
-	struct inode *inode;
+	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent *sd;
 
-	sysfs_addrm_start(&acxt, parent_sd);
-
 	/* allocate */
-	dentry = lookup_one_len(name, parent, strlen(name));
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_finish;
-	}
-
-	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;
-	if (sysfs_find_dirent(parent_sd, name))
-		goto out_iput;
-
-	sysfs_add_one(&acxt, sd);
-	sysfs_link_sibling(sd);
-	sysfs_instantiate(dentry, inode);
-	sysfs_attach_dentry(sd, dentry);
-
-	*p_sd = sd;
-	error = 0;
-	goto out_finish;	/* pin directory dentry in core */
+	sysfs_addrm_start(&acxt, parent_sd);
+	if (!sysfs_find_dirent(parent_sd, name)) {
+		sysfs_add_one(&acxt, sd);
+		sysfs_link_sibling(sd);
+	}
+	if (sysfs_addrm_finish(&acxt)) {
+		*p_sd = sd;
+		return 0;
+	}
 
- out_iput:
-	iput(inode);
- out_sput:
 	sysfs_put(sd);
- out_drop:
-	d_drop(dentry);
- out_dput:
-	dput(dentry);
- out_finish:
-	sysfs_addrm_finish(&acxt);
-	return error;
+	return -EEXIST;
 }
 
 int sysfs_create_subdir(struct kobject *kobj, const char *name,
@@ -785,6 +745,17 @@ int sysfs_create_dir(struct kobject * kobj,
 	return error;
 }
 
+static int sysfs_count_nlink(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 + 2;
+}
+
 static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 				struct nameidata *nd)
 {
@@ -795,7 +766,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 	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;
@@ -816,6 +787,11 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 	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 = sysfs_count_nlink(sd);
+			break;
 		case SYSFS_KOBJ_ATTR:
 			inode->i_size = PAGE_SIZE;
 			inode->i_fop = &sysfs_file_operations;
@@ -876,7 +852,7 @@ static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd)
 	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) {
 			*pos = sd->s_sibling;
 			sd->s_sibling = NULL;
 			sysfs_remove_one(&acxt, sd);
@@ -912,14 +888,25 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 		     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));
@@ -933,14 +920,14 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 	 * 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;
@@ -950,9 +937,9 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 
 	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 */
@@ -972,45 +959,58 @@ int sysfs_rename_dir(struct kobject *kobj, struct sysfs_dirent *new_parent_sd,
 	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);
@@ -1027,10 +1027,14 @@ again:
 	sysfs_link_sibling(sd);
 
 	mutex_unlock(&sysfs_mutex);
-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;
 }
 
@@ -1191,12 +1195,20 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
 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)
@@ -1223,17 +1235,23 @@ int sysfs_make_shadowed_dir(struct kobject *kobj,
 
 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;
 	struct sysfs_addrm_cxt acxt;
 
+	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)
@@ -1258,12 +1276,15 @@ struct sysfs_dirent *sysfs_create_shadow_dir(struct kobject *kobj)
 
 	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;
 }
 
 /**
fs/sysfs/file.c +59 -75

162 lines not on this page · raw message

fs/sysfs/mount.c +1 -1
index 078537e..402cc35 100644
@@ -24,7 +24,7 @@ static const struct super_operations sysfs_ops = {
 	.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,
fs/sysfs/sysfs.h +1 -0
index 72530dc..6a37f23 100644
@@ -52,6 +52,7 @@ struct sysfs_addrm_cxt {
 };
 
 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);
include/linux/sysfs.h +0 -1
index 4c43030..2f58ca1 100644
@@ -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
Signature · 1 line
1.5.0.3