Sysfs cleanups from Eric W. Biederman
PATCH 1/1 Sysfs cleanups from Eric W. Biederman
Hello, all.
This is subset of Eric W. Biederman's "Sysfs cleanups & tagged directory support" patchset[1] with the following modifications.
* fix-i_mutex-locking-in-sysfs_get_dentry patch is added at the top and #14-Don_t-use-lookup_one_len_kern and #15-vfs-Remove-lookup_one_len_kern are dropped. This is because #14 contained had a bug where it might created dentry/inode for an already deleted sysfs_dirent. I think it's benefitial to keep single lookup path.
* Rewrote simplify-sysfs_get_dentry patch and #08-Implement-__sysfs_get_dentry, #09-Move-sysfs_get_dentry-below-__sysfs_get_dentry and #10-Rewrite-sysfs_get_dentry-in-terms-of-__sysfs_get_dentry are omitted as __sysfs_get_dentry() isn't used by anyone.
* #16, 19-25 are omitted as it isn't clear yet how the tagged entry support will end up.
* readdir simplification fixed.
* sysfs_mutex double locking fixed.
The patchset is on top of the current -gregkh.
Thanks.
--
tejun
[1] http://thread.gmane.org/gmane.linux.kernel/564287/focus=519
PATCH 1/14 sysfs: fix i_mutex locking in sysfs_get_dentry()
lookup_one_len_kern() should be called with the parent's i_mutex locked. Fix it.
Spotted by Eric W. Biederman.
--- fs/sysfs/dir.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
fs/sysfs/dir.c +2 -0
@@ -130,8 +130,10 @@ struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd)/* look it up */ parent_dentry = dentry; +mutex_lock (&parent_dentry->d_inode->i_mutex); dentry =lookup_one_len_kern (cur->s_name, parent_dentry,strlen (cur->s_name)); +mutex_unlock (&parent_dentry->d_inode->i_mutex);dput (parent_dentry);if (IS_ERR (dentry)) {
Signature · 1 line
1.5.0.3
A message that is not in the archive (sent off-list or not yet received): 11876133893720-git-send-email-htejun-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
PATCH 2/14 sysfs: Move all of inode initialization into sysfs_init_inode
From: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
--- fs/sysfs/dir.c | 37 ------------------------------------- fs/sysfs/inode.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- fs/sysfs/mount.c | 5 ----- 3 files changed, 45 insertions(+), 45 deletions(-)
fs/sysfs/dir.c +0 -37
@@ -748,24 +748,12 @@ 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) {struct dentry *ret =NULL ;struct sysfs_dirent *parent_sd = dentry->d_parent->d_fsdata;struct sysfs_dirent *sd; -struct bin_attribute *bin_attr;struct inode *inode;mutex_lock (&sysfs_mutex);
@@ -785,31 +773,6 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, goto out_unlock; } -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; -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; -break ; -case SYSFS_KOBJ_LINK : - inode->i_op = &sysfs_symlink_inode_operations; -break ; -default : -BUG (); - } - } -sysfs_instantiate (dentry, inode);sysfs_attach_dentry (sd, dentry);
fs/sysfs/inode.c +45 -3
68 lines not on this page · raw message
fs/sysfs/mount.c +0 -5
@@ -49,11 +49,6 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent)return -ENOMEM ; } - inode->i_op = &sysfs_dir_inode_operations; - inode->i_fop = &sysfs_dir_operations; -inc_nlink (inode);/* directory, account for "." */ -unlock_new_inode (inode); -/* instantiate and link root dentry */ root =d_alloc_root (inode);if (!root) {
Signature · 1 line
1.5.0.3
PATCH 3/14 sysfs: Remove sysfs_instantiate
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Now that sysfs_get_inode is dropping the inode lock we no longer have a need from sysfs_instantiate.
--- fs/sysfs/dir.c | 2 +- fs/sysfs/inode.c | 17 ----------------- fs/sysfs/sysfs.h | 1 - 3 files changed, 1 insertions(+), 19 deletions(-)
fs/sysfs/dir.c +1 -1
@@ -773,7 +773,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, goto out_unlock; } -sysfs_instantiate (dentry, inode); +d_instantiate (dentry, inode);sysfs_attach_dentry (sd, dentry); out_unlock:
fs/sysfs/inode.c +0 -17
@@ -211,23 +211,6 @@ struct inode * sysfs_get_inode(struct sysfs_dirent *sd)return inode; } -/** -* sysfs_instantiate - instantiate dentry -* @dentry: dentry to be instantiated -* @inode: inode associated with @sd -* -* Unlock @inode if locked and instantiate @dentry with @inode. -* -* LOCKING: -* None. -*/ -void sysfs_instantiate (struct dentry *dentry,struct inode *inode) -{ -BUG_ON (!dentry || dentry->d_inode); - -d_instantiate (dentry, inode); -} -int sysfs_hash_and_remove (struct sysfs_dirent *dir_sd,const char *name) {struct sysfs_addrm_cxt acxt;
fs/sysfs/sysfs.h +0 -1
@@ -69,7 +69,6 @@ extern void sysfs_remove_one(struct sysfs_addrm_cxt *acxt,extern void sysfs_addrm_finish (struct sysfs_addrm_cxt *acxt);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 struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
Signature · 1 line
1.5.0.3
PATCH 4/14 sysfs: Use kill_anon_super
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Since sysfs no longer stores fs directory information in the dcache on a permanent basis kill_litter_super it is inappropriate and actively wrong. It will decrement the count on all dentries left in the dcache before trying to free them.
At the moment this is not biting us only because we never unmount sysfs.
--- fs/sysfs/mount.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
fs/sysfs/mount.c +1 -1
@@ -71,7 +71,7 @@ static int sysfs_get_sb(struct file_system_type *fs_type,static struct file_system_type sysfs_fs_type = { .name ="sysfs" , .get_sb = sysfs_get_sb, - .kill_sb = kill_litter_super, + .kill_sb = kill_anon_super, };int __init sysfs_init (void )
Signature · 1 line
1.5.0.3
PATCH 5/14 sysfs: Make sysfs_mount static
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
This patch modifies the users of sysfs_mount to use sysfs_root instead (which is what they are looking for). It then makes sysfs_mount static to keep people from using it by accident.
The net result is slightly faster and cleaner code.
--- fs/sysfs/dir.c | 4 +--- fs/sysfs/mount.c | 2 +- fs/sysfs/symlink.c | 7 +++---- fs/sysfs/sysfs.h | 1 - 4 files changed, 5 insertions(+), 9 deletions(-)
fs/sysfs/dir.c +1 -3
@@ -737,10 +737,8 @@ int sysfs_create_dir(struct kobject * kobj)if (kobj->parent) parent_sd = kobj->parent->sd; -else if (sysfs_mount && sysfs_mount->mnt_sb) - parent_sd = sysfs_mount->mnt_sb->s_root->d_fsdata;else -return -EFAULT ; + parent_sd = &sysfs_root; error =create_dir (kobj, parent_sd,kobject_name (kobj), &sd);if (!error)
fs/sysfs/mount.c +1 -1
@@ -14,7 +14,7 @@/* Random magic number */ #define SYSFS_MAGIC 0x62656572 -struct vfsmount *sysfs_mount; +static struct vfsmount *sysfs_mount;struct super_block * sysfs_sb =NULL ;struct kmem_cache *sysfs_dir_cachep;
fs/sysfs/symlink.c +3 -4
@@ -60,10 +60,9 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const charBUG_ON (!name); -if (!kobj) { -if (sysfs_mount && sysfs_mount->mnt_sb) - parent_sd = sysfs_mount->mnt_sb->s_root->d_fsdata; - }else +if (!kobj) + parent_sd = &sysfs_root; +else parent_sd = kobj->sd; error = -EFAULT ;
fs/sysfs/sysfs.h +0 -1
@@ -51,7 +51,6 @@ struct sysfs_addrm_cxt {int cnt; }; -extern struct vfsmount * sysfs_mount;extern struct sysfs_dirent sysfs_root;extern struct kmem_cache *sysfs_dir_cachep;
Signature · 1 line
1.5.0.3
PATCH 6/14 sysfs: In sysfs_lookup don't open code sysfs_find_dirent
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
This is a small cleanup patch that makes the code just a little bit cleaner.
--- fs/sysfs/dir.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
fs/sysfs/dir.c +1 -3
@@ -756,9 +756,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,mutex_lock (&sysfs_mutex); -for (sd = parent_sd->s_children; sd; sd = sd->s_sibling) -if (sysfs_type (sd) && !strcmp (sd->s_name, dentry->d_name.name)) -break ; + sd =sysfs_find_dirent (parent_sd, dentry->d_name.name);/* no such entry */ if (!sd)
Signature · 1 line
1.5.0.3
PATCH 7/14 sysfs: Simplify readdir.
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
At some point someone wrote sysfs_readdir to insert a cursor into the list of sysfs_dirents to ensure that sysfs_readdir would restart properly. That works but it is complex code and tends to be expensive.
The same effect can be achieved by keeping the sysfs_dirents in inode order and using the inode number as the f_pos. Then when we restart we just have to find the first dirent whose inode number is equal or greater then the last sysfs_dirent we attempted to return.
Removing the sysfs directory cursor also allows the remove of
all of the mysterious checks for sysfs_type(sd) != 0. Which
were nonbovious checks to see if a cursor was in a directory list.
tj: offset marker for EOF is changed from UINT_MAX to INT_MAX to avoid
overflow in case offset is 32bit.
--- fs/sysfs/dir.c | 175 ++++++++++++++------------------------------------------ 1 files changed, 44 insertions(+), 131 deletions(-)
fs/sysfs/dir.c +44 -131
@@ -33,10 +33,20 @@ static DEFINE_IDA(sysfs_ino_ida);static void sysfs_link_sibling (struct sysfs_dirent *sd) {struct sysfs_dirent *parent_sd = sd->s_parent; +struct sysfs_dirent **pos;BUG_ON (sd->s_sibling); - sd->s_sibling = parent_sd->s_children; - parent_sd->s_children = sd; + +/* Store directory entries in order by ino. This allows +* readdir to properly restart without having to add a +* cursor into the s_children list. +*/ +for (pos = &parent_sd->s_children; *pos; pos = &(*pos)->s_sibling) { +if (sd->s_ino < (*pos)->s_ino) +break ; + } + sd->s_sibling = *pos; + *pos = sd; }/**
@@ -659,7 +669,7 @@ struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,struct sysfs_dirent *sd;for (sd = parent_sd->s_children; sd; sd = sd->s_sibling) -if (sysfs_type (sd) && !strcmp (sd->s_name, name)) +if (!strcmp (sd->s_name, name))return sd;return NULL ; }
@@ -811,7 +821,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_DIR ) +if (sysfs_type (sd) !=SYSFS_DIR )sysfs_remove_one (&acxt, sd);else pos = &(*pos)->s_sibling;
@@ -976,37 +986,6 @@ again:return error; } -static int sysfs_dir_open (struct inode *inode,struct file *file) -{ -struct dentry * dentry = file->f_path.dentry; -struct sysfs_dirent * parent_sd = dentry->d_fsdata; -struct sysfs_dirent * sd; - - sd =sysfs_new_dirent ("_DIR_" ,0 ,0 ); -if (sd) { -mutex_lock (&sysfs_mutex); - sd->s_parent =sysfs_get (parent_sd); -sysfs_link_sibling (sd); -mutex_unlock (&sysfs_mutex); - } - - file->private_data = sd; -return sd ?0 : -ENOMEM ; -} - -static int sysfs_dir_close (struct inode *inode,struct file *file) -{ -struct sysfs_dirent * cursor = file->private_data; - -mutex_lock (&sysfs_mutex); -sysfs_unlink_sibling (cursor); -mutex_unlock (&sysfs_mutex); - -release_sysfs_dirent (cursor); - -return 0 ; -} -/* Relationship between s_mode and the DT_xxx types */ static inline unsigned char dt_type (struct sysfs_dirent *sd) {
@@ -1017,117 +996,51 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir) {struct dentry *dentry = filp->f_path.dentry;struct sysfs_dirent * parent_sd = dentry->d_fsdata; -struct sysfs_dirent *cursor = filp->private_data; -struct sysfs_dirent **pos; +struct sysfs_dirent *pos;ino_t ino; -int i = filp->f_pos; -switch (i) { -case 0 : - ino = parent_sd->s_ino; -if (filldir (dirent,"." ,1 , i, ino,DT_DIR ) <0 ) -break ; +if (filp->f_pos ==0 ) { + ino = parent_sd->s_ino; +if (filldir (dirent,"." ,1 , filp->f_pos, ino,DT_DIR ) ==0 ) filp->f_pos++; - i++; -/* fallthrough */ -case 1 : -if (parent_sd->s_parent) - ino = parent_sd->s_parent->s_ino; -else - ino = parent_sd->s_ino; -if (filldir (dirent,".." ,2 , i, ino,DT_DIR ) <0 ) -break ; + } +if (filp->f_pos ==1 ) { +if (parent_sd->s_parent) + ino = parent_sd->s_parent->s_ino; +else + ino = parent_sd->s_ino; +if (filldir (dirent,".." ,2 , filp->f_pos, ino,DT_DIR ) ==0 ) filp->f_pos++; - i++; -/* fallthrough */ -default : -mutex_lock (&sysfs_mutex); - - pos = &parent_sd->s_children; -while (*pos != cursor) - pos = &(*pos)->s_sibling; - -/* unlink cursor */ - *pos = cursor->s_sibling; - -if (filp->f_pos ==2 ) - pos = &parent_sd->s_children; - -for ( ; *pos; pos = &(*pos)->s_sibling) { -struct sysfs_dirent *next = *pos; -const char * name; -int len; - -if (!sysfs_type (next)) -continue ; - - name = next->s_name; - len =strlen (name); - ino = next->s_ino; - -if (filldir (dirent, name, len, filp->f_pos, ino, -dt_type (next)) <0 ) -break ; - - filp->f_pos++; - } + } +if ((filp->f_pos >1 ) && (filp->f_pos <INT_MAX )) { +mutex_lock (&sysfs_mutex); -/* put cursor back in */ - cursor->s_sibling = *pos; - *pos = cursor; +/* Skip the dentries we have already reported */ + pos = parent_sd->s_children; +while (pos && (filp->f_pos > pos->s_ino)) + pos = pos->s_sibling; -mutex_unlock (&sysfs_mutex); - } -return 0 ; -} +for ( ; pos; pos = pos->s_sibling) { +const char * name; +int len; -static loff_t sysfs_dir_lseek (struct file * file,loff_t offset,int origin) -{ -struct dentry * dentry = file->f_path.dentry; + name = pos->s_name; + len =strlen (name); + filp->f_pos = ino = pos->s_ino; -switch (origin) { -case 1 : - offset += file->f_pos; -case 0 : -if (offset >=0 ) +if (filldir (dirent, name, len, filp->f_pos, ino, +dt_type (pos)) <0 )break ; -default : -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; -struct sysfs_dirent *cursor = file->private_data; -struct sysfs_dirent **pos; -loff_t n = file->f_pos -2 ; - -sysfs_unlink_sibling (cursor); - - pos = &sd->s_children; -while (n && *pos) { -struct sysfs_dirent *next = *pos; -if (sysfs_type (next)) - n--; - pos = &(*pos)->s_sibling; - } - - cursor->s_sibling = *pos; - *pos = cursor; } - +if (!pos) + filp->f_pos =INT_MAX ;mutex_unlock (&sysfs_mutex); } - -return offset; +return 0 ; } +const struct file_operations sysfs_dir_operations = { - .open = sysfs_dir_open, - .release = sysfs_dir_close, - .llseek = sysfs_dir_lseek, .read = generic_read_dir, .readdir = sysfs_readdir, };
Signature · 1 line
1.5.0.3
PATCH 8/14 sysfs: Rewrite sysfs_drop_dentry.
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Currently we find the dentry to drop by looking at sd->s_dentry. We can just as easily accomplish the same task by looking up the sysfs inode and finding all of the dentries from there, with the added bonus that we don't need to play with the sysfs_assoc_lock.
--- fs/sysfs/dir.c | 53 ++++++++++++++++++++++++++--------------------------- 1 files changed, 26 insertions(+), 27 deletions(-)
fs/sysfs/dir.c +26 -27
@@ -565,50 +565,49 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)* 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; +struct dentry *dentry; + + inode =ilookup (sysfs_sb, sd->s_ino); +if (!inode) +return ; -/* We're not holding a reference to ->s_dentry dentry but the -* field will stay valid as long as sysfs_assoc_lock is held. +/* Drop any existing dentries associated with sd. +* +* For the dentry to be properly freed we need to grab a +* reference to the dentry under the dcache lock, unhash it, +* and then put it. The playing with the dentry count allows +* dput to immediately free the dentry if it is not in use. */ -spin_lock (&sysfs_assoc_lock); +repeat: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); +list_for_each_entry (dentry, &inode->i_dentry, d_alias) { +if (d_unhashed (dentry)) +continue ; +dget_locked (dentry);spin_lock (&dentry->d_lock);__d_drop (dentry);spin_unlock (&dentry->d_lock); +spin_unlock (&dcache_lock); +dput (dentry); + goto repeat; } -spin_unlock (&dcache_lock); -spin_unlock (&sysfs_assoc_lock); - -dput (dentry);/* adjust nlink and update timestamp */ - inode =ilookup (sysfs_sb, sd->s_ino); -if (inode) { -mutex_lock (&inode->i_mutex); +mutex_lock (&inode->i_mutex); - inode->i_ctime =CURRENT_TIME ; + inode->i_ctime =CURRENT_TIME ; +drop_nlink (inode); +if (sysfs_type (sd) ==SYSFS_DIR )drop_nlink (inode); -if (sysfs_type (sd) ==SYSFS_DIR ) -drop_nlink (inode); -mutex_unlock (&inode->i_mutex); -iput (inode); - } +mutex_unlock (&inode->i_mutex); + +iput (inode); }/**
Signature · 1 line
1.5.0.3
PATCH 9/14 sysfs: Introduce sysfs_rename_mutex
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Looking carefully at the rename code we have a subtle dependency that the structure of sysfs not change while we are performing a rename. If the parent directory of the object we are renaming changes while the rename is being performed nasty things could happen when we go to release our locks.
So introduce a sysfs_rename_mutex to prevent this highly unlikely theoretical issue.
In addition hold sysfs_rename_mutex over all calls to sysfs_get_dentry. Allowing sysfs_get_dentry to be simplified in the future.
--- fs/sysfs/dir.c | 8 +++++++- fs/sysfs/file.c | 4 ++++ fs/sysfs/sysfs.h | 1 + 3 files changed, 12 insertions(+), 1 deletions(-)
fs/sysfs/dir.c +7 -1
@@ -15,6 +15,7 @@#include "sysfs.h" DEFINE_MUTEX (sysfs_mutex); +DEFINE_MUTEX (sysfs_rename_mutex);spinlock_t sysfs_assoc_lock =SPIN_LOCK_UNLOCKED ;static spinlock_t sysfs_ino_lock =SPIN_LOCK_UNLOCKED ;
@@ -82,7 +83,7 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd)* down from there looking up dentry for each step. * * LOCKING: -* Kernel thread context (may sleep) +* mutex_lock(sysfs_rename_mutex) * * RETURNS: * Pointer to found dentry on success, ERR_PTR() value on error.
@@ -858,6 +859,8 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)const char *dup_name =NULL ;int error; +mutex_lock (&sysfs_rename_mutex); +/* get the original dentry */ sd = kobj->sd; old_dentry =sysfs_get_dentry (sd);
@@ -915,6 +918,7 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)kfree (dup_name);dput (old_dentry);dput (new_dentry); +mutex_unlock (&sysfs_rename_mutex);return error; }
@@ -926,6 +930,7 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)struct dentry *old_dentry =NULL , *new_dentry =NULL ;int error; +mutex_lock (&sysfs_rename_mutex);BUG_ON (!sd->s_parent); new_parent_sd = new_parent_kobj->sd ? new_parent_kobj->sd : &sysfs_root;
@@ -982,6 +987,7 @@ again:dput (new_parent);dput (old_dentry);dput (new_dentry); +mutex_unlock (&sysfs_rename_mutex);return error; }
fs/sysfs/file.c +4 -0
@@ -470,7 +470,9 @@ int sysfs_update_file(struct kobject * kobj, const struct attribute * attr)if (!victim_sd) goto out; +mutex_lock (&sysfs_rename_mutex); victim =sysfs_get_dentry (victim_sd); +mutex_unlock (&sysfs_rename_mutex);if (IS_ERR (victim)) { rc =PTR_ERR (victim); victim =NULL ;
@@ -509,7 +511,9 @@ int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)if (!victim_sd) goto out; +mutex_lock (&sysfs_rename_mutex); victim =sysfs_get_dentry (victim_sd); +mutex_unlock (&sysfs_rename_mutex);if (IS_ERR (victim)) { rc =PTR_ERR (victim); victim =NULL ;
fs/sysfs/sysfs.h +1 -0
@@ -90,6 +90,7 @@ extern int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);extern spinlock_t sysfs_assoc_lock;extern struct mutex sysfs_mutex; +extern struct mutex sysfs_rename_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
PATCH 11/14 sysfs: Remove s_dentry
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
The only uses of s_dentry left are the code that maintains s_dentry and trivial users that don't actually need it. So this patch removes the s_dentry maintenance code and restructures the trivial uses to use something else.
--- fs/sysfs/dir.c | 32 ++++---------------------------- fs/sysfs/mount.c | 1 - fs/sysfs/sysfs.h | 1 - 3 files changed, 4 insertions(+), 30 deletions(-)
fs/sysfs/dir.c +4 -28
@@ -289,22 +289,7 @@ 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_assoc_lock. -* This allows sysfs_drop_dentry() to dereference it. -*/ -spin_lock (&sysfs_assoc_lock); - -/* The dentry might have been deleted or another -* lookup could have happened updating sd->s_dentry to -* point the new dentry. Ignore if it isn't pointing -* to this dentry. -*/ -if (sd->s_dentry == dentry) - sd->s_dentry =NULL ; -spin_unlock (&sysfs_assoc_lock); -sysfs_put (sd); - } +sysfs_put (sd);iput (inode); }
@@ -352,9 +337,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)* @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) */
@@ -362,12 +344,6 @@ 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_assoc_lock); - sd->s_dentry = dentry; -spin_unlock (&sysfs_assoc_lock); -d_rehash (dentry); }
@@ -846,7 +822,7 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)/* rename */ d_add (new_dentry,NULL ); -d_move (sd->s_dentry, new_dentry); +d_move (old_dentry, new_dentry); error =0 ; goto out_unlock;
@@ -881,7 +857,7 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj) error =PTR_ERR (old_dentry); goto out_dput; } - old_parent = sd->s_parent->s_dentry; + old_parent = old_dentry->d_parent; new_parent =sysfs_get_dentry (new_parent_sd);if (IS_ERR (new_parent)) {
@@ -907,7 +883,7 @@ again: }else error =0 ;d_add (new_dentry,NULL ); -d_move (sd->s_dentry, new_dentry); +d_move (old_dentry, new_dentry);dput (new_dentry);/* Remove from old parent's list and insert into new parent's list. */
fs/sysfs/mount.c +0 -1
@@ -56,7 +56,6 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent)iput (inode);return -ENOMEM ; } - sysfs_root.s_dentry = root; root->d_fsdata = &sysfs_root; sb->s_root = root;return 0 ;
fs/sysfs/sysfs.h +0 -1
@@ -37,7 +37,6 @@ struct sysfs_dirent {unsigned int s_flags;umode_t s_mode;ino_t s_ino; -struct dentry * s_dentry;struct iattr * s_iattr;atomic_t s_event; };
Signature · 1 line
1.5.0.3
PATCH 13/14 sysfs: Rewrite rename in terms of sysfs dirents
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
This patch rewrites sysfs_rename_dir to perform it's checks as much as possible on the underlying sysfs_dirents instead of the contents of the dcache. It turns out that this version is a little simpler, and a little more like the rest of the sysfs directory modification code.
tj: fixed double locking of sysfs_mutex
--- fs/sysfs/dir.c | 38 ++++++++++++++++---------------------- 1 files changed, 16 insertions(+), 22 deletions(-)
fs/sysfs/dir.c +16 -22
@@ -767,7 +767,7 @@ void sysfs_remove_dir(struct kobject * kobj)int sysfs_rename_dir (struct kobject * kobj,const char *new_name) { -struct sysfs_dirent *sd; +struct sysfs_dirent *sd = kobj->sd;struct dentry *parent =NULL ;struct dentry *old_dentry =NULL , *new_dentry =NULL ;const char *dup_name =NULL ;
@@ -775,63 +775,57 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)mutex_lock (&sysfs_rename_mutex); + error =0 ; +if (strcmp (sd->s_name, new_name) ==0 ) + goto out;/* nothing to rename */ +/* get the original dentry */ - sd = kobj->sd; old_dentry =sysfs_get_dentry (sd);if (IS_ERR (old_dentry)) { error =PTR_ERR (old_dentry); - goto out_dput; + goto out; } parent = old_dentry->d_parent;/* lock parent and get dentry for new name */ mutex_lock (&parent->d_inode->i_mutex); +mutex_lock (&sysfs_mutex); - new_dentry =lookup_one_len (new_name, parent,strlen (new_name)); -if (IS_ERR (new_dentry)) { - error =PTR_ERR (new_dentry); - goto out_unlock; - } - - error = -EINVAL ; -if (old_dentry == new_dentry) + error = -EEXIST ; +if (sysfs_find_dirent (sd->s_parent, new_name)) goto out_unlock; - error = -EEXIST ; -if (new_dentry->d_inode) + error = -ENOMEM ; + new_dentry =d_alloc_name (parent, new_name); +if (!new_dentry) goto out_unlock;/* rename kobject and sysfs_dirent */ error = -ENOMEM ; new_name = dup_name =kstrdup (new_name,GFP_KERNEL );if (!new_name) - goto out_drop; + goto out_unlock; error =kobject_set_name (kobj,"%s" , new_name);if (error) - goto out_drop; + goto out_unlock; -mutex_lock (&sysfs_mutex); dup_name = sd->s_name; sd->s_name = new_name; -mutex_unlock (&sysfs_mutex);/* rename */ d_add (new_dentry,NULL );d_move (old_dentry, new_dentry); error =0 ; - goto out_unlock; - - out_drop: -d_drop (new_dentry); out_unlock: +mutex_unlock (&sysfs_mutex);mutex_unlock (&parent->d_inode->i_mutex); - out_dput:kfree (dup_name);dput (old_dentry);dput (new_dentry); + out:mutex_unlock (&sysfs_rename_mutex);return error; }
Signature · 1 line
1.5.0.3
PATCH 14/14 sysfs: Rewrite sysfs_move_dir in terms of sysfs dirents
In reply to a message not in the archiveFrom: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
This patch rewrites sysfs_move_dir to perform it's checks as much as possible on the underlying sysfs_dirents instead of the contents of the dcache, making sysfs_move_dir more like the rest of the sysfs directory modification code.
--- fs/sysfs/dir.c | 34 ++++++++++++++++++---------------- 1 files changed, 18 insertions(+), 16 deletions(-)
fs/sysfs/dir.c +18 -16
@@ -842,56 +842,58 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)BUG_ON (!sd->s_parent); new_parent_sd = new_parent_kobj->sd ? new_parent_kobj->sd : &sysfs_root; + error =0 ; +if (sd->s_parent == new_parent_sd) + goto out;/* nothing to move */ +/* get dentries */ old_dentry =sysfs_get_dentry (sd);if (IS_ERR (old_dentry)) { error =PTR_ERR (old_dentry); - goto out_dput; + goto out; } old_parent = old_dentry->d_parent; new_parent =sysfs_get_dentry (new_parent_sd);if (IS_ERR (new_parent)) { error =PTR_ERR (new_parent); - goto out_dput; + goto out; } -if (old_parent->d_inode == new_parent->d_inode) { - error =0 ; - goto out_dput;/* nothing to move */ - } again: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; } +mutex_lock (&sysfs_mutex); - new_dentry =lookup_one_len (kobj->name, new_parent,strlen (kobj->name)); -if (IS_ERR (new_dentry)) { - error =PTR_ERR (new_dentry); + error = -EEXIST ; +if (sysfs_find_dirent (new_parent_sd, sd->s_name)) goto out_unlock; - }else - error =0 ; + + error = -ENOMEM ; + new_dentry =d_alloc_name (new_parent, sd->s_name); +if (!new_dentry) + goto out_unlock; + + error =0 ;d_add (new_dentry,NULL );d_move (old_dentry, new_dentry);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_unlock: +mutex_unlock (&sysfs_mutex);mutex_unlock (&new_parent->d_inode->i_mutex);mutex_unlock (&old_parent->d_inode->i_mutex); - out_dput: + out:dput (new_parent);dput (old_dentry);dput (new_dentry);
Signature · 1 line
1.5.0.3
PATCH 10/14 sysfs: simply sysfs_get_dentry
In reply to Tejun HeoNow that we know the sysfs tree structure cannot change under us and sysfs shadow support is dropped, sysfs_get_dentry() can be simplified greatly. It can just look up from the root and there's no need to retry on failure.
--- fs/sysfs/dir.c | 91 ++++++++++---------------------------------------------- 1 files changed, 16 insertions(+), 75 deletions(-)
fs/sysfs/dir.c +16 -75
@@ -78,9 +78,8 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd)* @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. +* present. This function descends from the root looking up +* dentry for each step. * * LOCKING: * mutex_lock(sysfs_rename_mutex)
@@ -90,86 +89,28 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd)*/ struct dentry *sysfs_get_dentry (struct sysfs_dirent *sd) { -struct sysfs_dirent *cur; -struct dentry *parent_dentry, *dentry; -int i, depth; +struct dentry *dentry =dget (sysfs_sb->s_root); -/* 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); +while (dentry->d_fsdata != sd) { +struct sysfs_dirent *cur; +struct dentry *parent; -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++) +/* find the first ancestor which hasn't been looked up */ + cur = sd; +while (cur->s_parent != dentry->d_fsdata) 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; -mutex_lock (&parent_dentry->d_inode->i_mutex); - dentry =lookup_one_len_kern (cur->s_name, parent_dentry, + parent = dentry; +mutex_lock (&parent->d_inode->i_mutex); + dentry =lookup_one_len_kern (cur->s_name, parent,strlen (cur->s_name)); -mutex_unlock (&parent_dentry->d_inode->i_mutex); -dput (parent_dentry); - -if (IS_ERR (dentry)) { -sysfs_put (cur); -return dentry; - } +mutex_unlock (&parent->d_inode->i_mutex); +dput (parent); -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); +if (IS_ERR (dentry)) +break ; } - -mutex_unlock (&sysfs_mutex);return dentry; }
Signature · 1 line
1.5.0.3
PATCH 12/14 sysfs: kill SYSFS_FLAG_REMOVED
In reply to Tejun HeoWith sysfs_get_dentry() simplified, there's no user of SYSFS_FLAG_REMOVED left. Kill it.
--- fs/sysfs/dir.c | 5 +---- include/linux/sysfs.h | 1 - 2 files changed, 1 insertions(+), 5 deletions(-)
fs/sysfs/dir.c +1 -4
@@ -222,7 +222,7 @@ static void sysfs_deactivate(struct sysfs_dirent *sd)DECLARE_COMPLETION_ONSTACK (wait);int v; -BUG_ON (sd->s_sibling || !(sd->s_flags &SYSFS_FLAG_REMOVED )); +BUG_ON (sd->s_sibling); sd->s_sibling = (void *)&wait;/* atomic_add_return() is a mb(), put_active() will always see
@@ -462,11 +462,8 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)*/ void sysfs_remove_one (struct sysfs_addrm_cxt *acxt,struct sysfs_dirent *sd) { -BUG_ON (sd->s_flags &SYSFS_FLAG_REMOVED ); -sysfs_unlink_sibling (sd); - sd->s_flags |=SYSFS_FLAG_REMOVED ; sd->s_sibling = acxt->removed; acxt->removed = sd;
include/linux/sysfs.h +0 -1
@@ -83,7 +83,6 @@ 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
Quoted 18 lines · Tejun Heo
Hello, all.
This is subset of Eric W. Biederman's "Sysfs cleanups & tagged directory support" patchset[1] with the following modifications.
* fix-i_mutex-locking-in-sysfs_get_dentry patch is added at the top and #14-Don_t-use-lookup_one_len_kern and #15-vfs-Remove-lookup_one_len_kern are dropped. This is because #14 contained had a bug where it might created dentry/inode for an already deleted
sysfs_dirent. I think it's benefitial to keep single lookup path.* Rewrote simplify-sysfs_get_dentry patch and #08-Implement-__sysfs_get_dentry, #09-Move-sysfs_get_dentry-below-__sysfs_get_dentry and #10-Rewrite-sysfs_get_dentry-in-terms-of-__sysfs_get_dentry are omitted as
__sysfs_get_dentry()isn't used by anyone.
Quoted 8 lines
* #16, 19-25 are omitted as it isn't clear yet how the tagged entry support will end up.
* readdir simplification fixed.
*
sysfs_mutexdouble locking fixed.The patchset is on top of the current -gregkh.
Survives some casual testing (including moving devices). Patches look sane to me at a cursory glance as well (sorry, don't have much time at the moment).
Quoted 26 lines · Cornelia Huck
Quoted 23 lines · Tejun Heo
Hello, all.
This is subset of Eric W. Biederman's "Sysfs cleanups & tagged directory support" patchset[1] with the following modifications.
* fix-i_mutex-locking-in-sysfs_get_dentry patch is added at the top and #14-Don_t-use-lookup_one_len_kern and #15-vfs-Remove-lookup_one_len_kern are dropped. This is because #14 contained had a bug where it might created dentry/inode for an already deleted
sysfs_dirent. I think it's benefitial to keep single lookup path.* Rewrote simplify-sysfs_get_dentry patch and #08-Implement-__sysfs_get_dentry, #09-Move-sysfs_get_dentry-below-__sysfs_get_dentry and #10-Rewrite-sysfs_get_dentry-in-terms-of-__sysfs_get_dentry are omitted as
__sysfs_get_dentry()isn't used by anyone.* #16, 19-25 are omitted as it isn't clear yet how the tagged entry support will end up.
* readdir simplification fixed.
Quoted 7 lines
Quoted 3 lines
*
sysfs_mutexdouble locking fixed.The patchset is on top of the current -gregkh.
Survives some casual testing (including moving devices). Patches look sane to me at a cursory glance as well (sorry, don't have much time at the moment).
Thanks.
Signature · 1 line
tejun
Quoted 4 lines · Tejun Heo
Hello, all.
This is subset of Eric W. Biederman's "Sysfs cleanups & tagged directory support" patchset[1] with the following modifications.
As a base:
Quoted 7 lines
* fix-i_mutex-locking-in-sysfs_get_dentry patch is added at the top and #14-Don_t-use-lookup_one_len_kern and #15-vfs-Remove-lookup_one_len_kern are dropped. This is because #14 contained had a bug where it might created dentry/inode for an already deleted
sysfs_dirent. I think it's benefitial to keep single lookup path.
I think I disagree with the bug spotting.
At least in net we the sysfs_rename_mutex which keeps parent directories from disappearing. Further we have a reference to the leaf sysfs_dirent and are actively manipulating it, so the sysfs_dirent should not disappear on us.
Quoted 5 lines
* Rewrote simplify-sysfs_get_dentry patch and #08-Implement-__sysfs_get_dentry, #09-Move-sysfs_get_dentry-below-__sysfs_get_dentry and #10-Rewrite-sysfs_get_dentry-in-terms-of-__sysfs_get_dentry are omitted as
__sysfs_get_dentry()isn't used by anyone.
Right. __sysfs_get_dentry is an optimization that has makes the best case for sysfs_get_dentry O(1) instead of O(depth). However this doesn't matter because sysfs_get_dentry is not on any fast path and the maximum depth of sysfs directories is fairly shallow and programmer controlled.
The only user other user of __sysfs_get_dentry is in the tagged directory support, and even that user doesn't strictly need it. Although it is a bit silly to populate the dcache just so you can invalidate it a moment later...
Just doing the dget(sysfs_sb->s_root) is a bit clearer in sysfs_get_dentry then knowing implicitly that is what __sysfs_get_dentry does in the worst cased.
Quoted 8 lines
* #16, 19-25 are omitted as it isn't clear yet how the tagged entry support will end up.
* readdir simplification fixed.
*
sysfs_mutexdouble locking fixed.The patchset is on top of the current -gregkh.
Eric
Hello,
Quoted 7 lines · Eric W. Biederman
Quoted 6 lines
* fix-i_mutex-locking-in-sysfs_get_dentry patch is added at the top and #14-Don_t-use-lookup_one_len_kern and #15-vfs-Remove-lookup_one_len_kern are dropped. This is because #14 contained had a bug where it might created dentry/inode for an already deleted
sysfs_dirent. I think it's benefitial to keep single lookup path.
Quoted 6 lines
I think I disagree with the bug spotting.
At least in net we the
sysfs_rename_mutexwhich keeps parent directories from disappearing. Further we have a reference to the leafsysfs_direntand are actively manipulating it, so thesysfs_direntshould not disappear on us.
sysfs_rename_mutex() keeps out renaming and moving not removing. Also, reference prevents sysfs_dirent from being released not from being removed. The different in lookup path is that it searches the children list - sd's are unlinked from children list on removal.
Quoted 11 lines
Quoted 5 lines
* Rewrote simplify-sysfs_get_dentry patch and #08-Implement-__sysfs_get_dentry, #09-Move-sysfs_get_dentry-below-__sysfs_get_dentry and #10-Rewrite-sysfs_get_dentry-in-terms-of-__sysfs_get_dentry are omitted as
__sysfs_get_dentry()isn't used by anyone.Right.
__sysfs_get_dentryis an optimization that has makes the best case forsysfs_get_dentryO(1) instead of O(depth). However this doesn't matter becausesysfs_get_dentryis not on any fast path and the maximum depth of sysfs directories is fairly shallow and programmer controlled.
The reason why sysfs_get_dentry() climbed up first then climbed down was not because of performance. It was to support the original shadow implementation. Because there was no reliable way to reach a leaf node from the root, dentries of all shadows are pinned such that they can serve as the starting point for dentry lookup and the climing up was to reach that starting point. Now that the dentry-multiplexing shadow support is gone, there's no need to do the climing up.
Quoted 4 lines
The only user other user of
__sysfs_get_dentryis in the tagged directory support, and even that user doesn't strictly need it. Although it is a bit silly to populate the dcache just so you can invalidate it a moment later...
Yeah, actually the only essential user of that kind of look up is sysfs_drop_dentry() which is in deletion path and can't fail due to allocation failure and has the logic open-coded. I have nothing against __sysfs_get_dentry(). It was just not needed by the patches I forwarded this time. Feel free to include it as you see fit.
I'm currently working on kobj/sysfs separation. As most internal implementation is sd based already, the changes are mostly confined to interface functions but it's still a big change. I think I'll be able to post the patches in this week or early next week at the latest.
Thanks.
Signature · 1 line
tejun
Quoted 17 lines · Tejun Heo
Hello,
Quoted 7 lines · Eric W. Biederman
Quoted 6 lines
* fix-i_mutex-locking-in-sysfs_get_dentry patch is added at the top and #14-Don_t-use-lookup_one_len_kern and #15-vfs-Remove-lookup_one_len_kern are dropped. This is because #14 contained had a bug where it might created dentry/inode for an already deleted
sysfs_dirent. I think it's benefitial to keep single lookup path.Quoted 6 lines
I think I disagree with the bug spotting.
At least in net we the
sysfs_rename_mutexwhich keeps parent directories from disappearing. Further we have a reference to the leafsysfs_direntand are actively manipulating it, so thesysfs_direntshould not disappear on us.
Quoted 4 lines
sysfs_rename_mutex()keeps out renaming and moving not removing. Also, reference preventssysfs_direntfrom being released not from being removed. The different in lookup path is that it searches the children list - sd's are unlinked from children list on removal.
True. I guess what I was seeing was not a code but a usages
restriction which may not be enforced.
If we are using sysfs_get_dentry the sd is not deleted because it is alive and being used, likewise the path to it should also be alive. Once you add in classic unix delete behaviour allowing for alive but deleted files this no longer applies. But I don't think that makes sense for sysfs, but I may be confused.
Quoted 12 lines
Quoted 11 lines
Quoted 5 lines
* Rewrote simplify-sysfs_get_dentry patch and #08-Implement-__sysfs_get_dentry, #09-Move-sysfs_get_dentry-below-__sysfs_get_dentry and #10-Rewrite-sysfs_get_dentry-in-terms-of-__sysfs_get_dentry are omitted as
__sysfs_get_dentry()isn't used by anyone.Right.
__sysfs_get_dentryis an optimization that has makes the best case forsysfs_get_dentryO(1) instead of O(depth). However this doesn't matter becausesysfs_get_dentryis not on any fast path and the maximum depth of sysfs directories is fairly shallow and programmer controlled.
Quoted 7 lines
The reason why
sysfs_get_dentry()climbed up first then climbed down was not because of performance. It was to support the original shadow implementation. Because there was no reliable way to reach a leaf node from the root, dentries of all shadows are pinned such that they can serve as the starting point for dentry lookup and the climing up was to reach that starting point. Now that the dentry-multiplexing shadow support is gone, there's no need to do the climing up.
Reasonable. The reason I kept the climbing up was the performance benefit I saw.
Quoted 10 lines
Quoted 4 lines
The only user other user of
__sysfs_get_dentryis in the tagged directory support, and even that user doesn't strictly need it. Although it is a bit silly to populate the dcache just so you can invalidate it a moment later...Yeah, actually the only essential user of that kind of look up is
sysfs_drop_dentry()which is in deletion path and can't fail due to allocation failure and has the logic open-coded. I have nothing against__sysfs_get_dentry(). It was just not needed by the patches I forwarded this time. Feel free to include it as you see fit.
Sure.
Quoted 4 lines
I'm currently working on kobj/sysfs separation. As most internal implementation is sd based already, the changes are mostly confined to interface functions but it's still a big change. I think I'll be able to post the patches in this week or early next week at the latest.
Ok.
I'd like to make certain we have a path to tagged support or something like it if we can.
Eric