libata: implement ata_link, take 4

PATCH In review Tejun Heo 1 participant 8 messages Jul 1, 2007

Subsystems (3)

Library code libata Driver core

lore

Share & tools

lore

Tejun Heo

author

2007-07-01 10:04 UTC #0 lore raw

PATCH 2/4 libata: implement ata_link, take 4

Hello, all.

This is the fourth take of implement ata_link patchset. This patchset contains 16 patches which can be categorized as follows.

#01-02: prep
#03-04: implement ata_link
#05-08: make libata deal with link instead of port #09-11: misc link stuff (link init, reset_tries, ata_link_abort()) #12-16: add ap->pmp_link[] and update libata to deal with multiple
links

ata_link abstracts PHY and sits between ata_port and ata_device. The following attributes are moved to ata_link from ata_port.

- active command state (active_tag, sactive)
- [hw_]sata_spd_limit
- eh_info and eh_context
- device array

With above and a few extra fields, a link can handle attached devices including qc management and EH/hotplug. This patchset makes libata ready to handle PMP links.

Changes from the last take[L] are.

* updated to fit new #upstream
* SError failure handling updated and moved to misc-updates patchset
* ata_dev_configure() update is unnecessary now and dropped

This patchset is against

libata-dev#upstream (4e1ae96828f6cee7b89ab8ca474c150fe211afd8)
+ [1] misc-updates patchset

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.ide/13478
[1] http://thread.gmane.org/gmane.linux.ide/20081

Tejun Heo

author

2007-07-01 10:04 UTC #1 lore raw

PATCH 1/6 sysfs: implement sysfs flags and SYSFS_FLAG_REMOVED

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 <[email protected]>

---
 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(-)

fs/sysfs/dir.c +23 ~~-14~~

index b4074ad..4a2bd8b 100644
@@ -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;
 			}

fs/sysfs/inode.c +3 -2

index 4d79a61..b67623d 100644
@@ -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;

fs/sysfs/mount.c +1 -1

index 2e66701..d5ce3aa 100644
@@ -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,
 };

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_
 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 +4 -0

index 161e19a..2a6df64 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,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,

Signature · 1 line

1.4.3.4

Tejun Heo

author

2007-07-01 10:04 UTC #2 lore raw

PATCH 2/6 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.

Signed-off-by Tejun Heo <[email protected]>

---
 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 4a2bd8b..c05d04a 100644
@@ -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);

fs/sysfs/file.c +1 -1

index a84b734..e448b88 100644
@@ -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;
 	}

fs/sysfs/symlink.c +1 -1

index ff605d3..45b62e2 100644
@@ -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);

fs/sysfs/sysfs.h +4 -1

index 06b5085..f1629b4 100644
@@ -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,

Signature · 1 line

1.4.3.4

Tejun Heo

author

2007-07-01 10:04 UTC #3 lore raw

PATCH 3/6 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.

Signed-off-by Tejun Heo <[email protected]>

---
 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(-)

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
 
 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 +3 -6

index b67623d..4077ab5 100644
@@ -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))

fs/sysfs/symlink.c +10 ~~-12~~

index 45b62e2..43cc522 100644
@@ -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,

fs/sysfs/sysfs.h +6 -4

index f1629b4..27a5f4b 100644
@@ -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);

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(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;
 }

lib/kobject.c +5 -5

index a85708c..2d26f07 100644
@@ -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;

Signature · 1 line

1.4.3.4

Tejun Heo

author

2007-07-01 10:04 UTC #4 lore raw

PATCH 4/6 sysfs: use sysfs_lock 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_lock instead.

Signed-off-by Tejun Heo <[email protected]>

---
 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(-)

fs/sysfs/dir.c +71 ~~-36~~

index 58a92aa..a474e54 100644
@@ -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);

fs/sysfs/file.c +15 ~~-16~~

index acc9890..f2e3c88 100644
@@ -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;
 }

fs/sysfs/inode.c +3 -6

index 4077ab5..705c03d 100644
@@ -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;

fs/sysfs/symlink.c +26 ~~-25~~

87 lines not on this page · raw message

fs/sysfs/sysfs.h +0 -1

index 27a5f4b..476d58b 100644
@@ -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;

Signature · 1 line

1.4.3.4

Tejun Heo

author

2007-07-01 10:04 UTC #5 lore raw

PATCH 5/6 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.

Signed-off-by Tejun Heo <[email protected]>

---
 fs/sysfs/dir.c   |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/sysfs/sysfs.h |    1 +
 2 files changed, 93 insertions(+), 0 deletions(-)

fs/sysfs/dir.c +92 -0

index a474e54..0242cbd 100644
@@ -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
  *

fs/sysfs/sysfs.h +1 -0

index 476d58b..b11c448 100644
@@ -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);

Signature · 1 line

1.4.3.4

Tejun Heo

author

2007-07-01 10:04 UTC #6 lore raw

PATCH 6/6 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.
* parent mtime/ctime update on inode instantiation is dropped. I' not sure what to do about it yet.

Signed-off-by Tejun Heo <[email protected]>

---
 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(-)

fs/sysfs/dir.c +136 ~~-102~~

index 0242cbd..ae1f379 100644
@@ -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;
 }
 
 /**

fs/sysfs/file.c +59 ~~-75~~

162 lines not on this page · raw message

fs/sysfs/inode.c +5 ~~-10~~

index 705c03d..e71d763 100644
@@ -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);
 		}
 	}

fs/sysfs/mount.c +1 -1

index d5ce3aa..f82c345 100644
@@ -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,

fs/sysfs/sysfs.h +1 -0

index b11c448..0aa4b18 100644
@@ -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);

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.4.3.4

Tejun Heo

author

2007-07-01 10:05 UTC #7 lore raw

Re: Oops, scrap this. Wrong patchset.

In reply to Tejun Heo

-EWRNGDIR. Sorry.

Signature · 1 line

tejun