From: Eric W. Biederman <ebiederm@xxxxxxxxxxxx> With lazy inode updates and dentry operations bringing everything into sync on demand there is no longer any need to immediately update the vfs or grab i_mutex to protect those updates as we make changes to sysfs. So stop updating the vfs inodes and move what remains of sysfs_addrm_start and sysfs_addrm_finsih (just barely more than taking the sysfs_mutex) into sysfs_add_one and sysfs_remove_one. Acked-by: Tejun Heo <tj@xxxxxxxxxx> Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxxxxxxxx> --- fs/sysfs/dir.c | 194 +++++++--------------------------------------------- fs/sysfs/file.c | 6 +-- fs/sysfs/inode.c | 16 ++--- fs/sysfs/symlink.c | 6 +-- fs/sysfs/sysfs.h | 17 +---- 5 files changed, 34 insertions(+), 205 deletions(-) diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index b75c938..3caf9c6 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -382,62 +382,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) return NULL; } -static int sysfs_ilookup_test(struct inode *inode, void *arg) -{ - struct sysfs_dirent *sd = arg; - return inode->i_ino == sd->s_ino; -} - -/** - * sysfs_addrm_start - prepare for sysfs_dirent add/remove - * @acxt: pointer to sysfs_addrm_cxt to be used - * @parent_sd: parent sysfs_dirent - * - * This function is called when the caller is about to add or - * remove sysfs_dirent under @parent_sd. This function acquires - * sysfs_mutex, grabs inode for @parent_sd if available and lock - * i_mutex of it. @acxt is used to keep and pass context to - * other addrm functions. - * - * LOCKING: - * Kernel thread context (may sleep). sysfs_mutex is locked on - * return. i_mutex of parent inode is locked on return if - * available. - */ -void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, - struct sysfs_dirent *parent_sd) -{ - struct inode *inode; - - memset(acxt, 0, sizeof(*acxt)); - acxt->parent_sd = parent_sd; - - /* Lookup parent inode. inode initialization is protected by - * sysfs_mutex, so inode existence can be determined by - * looking up inode while holding sysfs_mutex. - */ - mutex_lock(&sysfs_mutex); - - inode = ilookup5(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test, - parent_sd); - if (inode) { - WARN_ON(inode->i_state & I_NEW); - - /* parent inode available */ - acxt->parent_inode = inode; - - /* sysfs_mutex is below i_mutex in lock hierarchy. - * First, trylock i_mutex. If fails, unlock - * sysfs_mutex and lock them in order. - */ - if (!mutex_trylock(&inode->i_mutex)) { - mutex_unlock(&sysfs_mutex); - mutex_lock(&inode->i_mutex); - mutex_lock(&sysfs_mutex); - } - } -} - /** * sysfs_pathname - return full path to sysfs dirent * @sd: sysfs_dirent whose path we want @@ -460,161 +404,83 @@ static char *sysfs_pathname(struct sysfs_dirent *sd, char *path) /** * sysfs_add_one - add sysfs_dirent to parent - * @acxt: addrm context to use + * @parent_sd: directory to add @sd into * @sd: sysfs_dirent to be added * - * Get @acxt->parent_sd and set sd->s_parent to it and increment + * Get @parent_sd and set sd->s_parent to it and increment * nlink of parent inode if @sd is a directory and link into the * children list of the parent. * - * This function should be called between calls to - * sysfs_addrm_start() and sysfs_addrm_finish() and should be - * passed the same @acxt as passed to sysfs_addrm_start(). - * * LOCKING: - * Determined by sysfs_addrm_start(). + * Kernel thread context (may sleep). Grabs sysfs_mutex. * * RETURNS: * 0 on success, -EEXIST if entry with the given name already * exists. */ -int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) +int sysfs_add_one(struct sysfs_dirent *parent_sd, struct sysfs_dirent *sd) { struct iattr *ps_iattr; - if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) { - char *path = kzalloc(PATH_MAX, GFP_KERNEL); + mutex_lock(&sysfs_mutex); + if (sysfs_find_dirent(parent_sd, sd->s_name)) { + char *path; + mutex_unlock(&sysfs_mutex); + + path = kzalloc(PATH_MAX, GFP_KERNEL); WARN(1, KERN_WARNING "sysfs: cannot create duplicate filename '%s'\n", (path == NULL) ? sd->s_name : - strcat(strcat(sysfs_pathname(acxt->parent_sd, path), "/"), + strcat(strcat(sysfs_pathname(parent_sd, path), "/"), sd->s_name)); kfree(path); return -EEXIST; } - sd->s_parent = sysfs_get(acxt->parent_sd); - - if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode) - inc_nlink(acxt->parent_inode); - - acxt->cnt++; - + sd->s_parent = sysfs_get(parent_sd); sysfs_link_sibling(sd); /* Update timestamps on the parent */ - ps_iattr = acxt->parent_sd->s_iattr; + ps_iattr = parent_sd->s_iattr; if (ps_iattr) ps_iattr->ia_ctime = ps_iattr->ia_mtime = CURRENT_TIME; + mutex_unlock(&sysfs_mutex); return 0; } /** * sysfs_remove_one - remove sysfs_dirent from parent - * @acxt: addrm context to use * @sd: sysfs_dirent to be removed * * Mark @sd removed and drop nlink of parent inode if @sd is a * directory. @sd is unlinked from the children list. * - * This function should be called between calls to - * sysfs_addrm_start() and sysfs_addrm_finish() and should be - * passed the same @acxt as passed to sysfs_addrm_start(). - * * LOCKING: - * Determined by sysfs_addrm_start(). + * Kernel thread context (may sleep). Grabs sysfs_mutex. */ -void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) +void sysfs_remove_one(struct sysfs_dirent *sd) { struct iattr *ps_iattr; BUG_ON(sd->s_flags & SYSFS_FLAG_REMOVED); + mutex_lock(&sysfs_mutex); + sysfs_unlink_sibling(sd); /* Update timestamps on the parent */ - ps_iattr = acxt->parent_sd->s_iattr; + ps_iattr = sd->s_parent->s_iattr; if (ps_iattr) ps_iattr->ia_ctime = ps_iattr->ia_mtime = CURRENT_TIME; sd->s_flags |= SYSFS_FLAG_REMOVED; - sd->s_sibling = acxt->removed; - acxt->removed = sd; - - if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode) - drop_nlink(acxt->parent_inode); - acxt->cnt++; -} - -/** - * sysfs_dec_nlink - Decrement link count for the specified sysfs_dirent - * @sd: target sysfs_dirent - * - * Decrement nlink for @sd. @sd must have been unlinked from its - * parent on entry to this function such that it can't be looked - * up anymore. - */ -static void sysfs_dec_nlink(struct sysfs_dirent *sd) -{ - struct inode *inode; - - inode = ilookup(sysfs_sb, sd->s_ino); - if (!inode) - return; - - /* adjust nlink and update timestamp */ - mutex_lock(&inode->i_mutex); - - inode->i_ctime = CURRENT_TIME; - drop_nlink(inode); - if (sysfs_type(sd) == SYSFS_DIR) - drop_nlink(inode); - - mutex_unlock(&inode->i_mutex); - - iput(inode); -} - -/** - * sysfs_addrm_finish - finish up sysfs_dirent add/remove - * @acxt: addrm context to finish up - * - * Finish up sysfs_dirent add/remove. Resources acquired by - * sysfs_addrm_start() are released and removed sysfs_dirents are - * cleaned up. Timestamps on the parent inode are updated. - * - * LOCKING: - * All mutexes acquired by sysfs_addrm_start() are released. - */ -void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt) -{ - /* release resources acquired by sysfs_addrm_start() */ mutex_unlock(&sysfs_mutex); - if (acxt->parent_inode) { - struct inode *inode = acxt->parent_inode; - - /* if added/removed, update timestamps on the parent */ - if (acxt->cnt) - inode->i_ctime = inode->i_mtime = CURRENT_TIME; - mutex_unlock(&inode->i_mutex); - iput(inode); - } - - /* kill removed sysfs_dirents */ - while (acxt->removed) { - struct sysfs_dirent *sd = acxt->removed; - - acxt->removed = sd->s_sibling; - sd->s_sibling = NULL; - - sysfs_dec_nlink(sd); - sysfs_deactivate(sd); - unmap_bin_file(sd); - sysfs_put(sd); - } + sysfs_deactivate(sd); + unmap_bin_file(sd); + sysfs_put(sd); } /** @@ -673,7 +539,6 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd, const char *name, struct sysfs_dirent **p_sd) { umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO; - struct sysfs_addrm_cxt acxt; struct sysfs_dirent *sd; int rc; @@ -684,10 +549,8 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd, sd->s_dir.kobj = kobj; /* link in */ - sysfs_addrm_start(&acxt, parent_sd); - rc = sysfs_add_one(&acxt, sd); - sysfs_addrm_finish(&acxt); + rc = sysfs_add_one(parent_sd, sd); if (rc == 0) *p_sd = sd; else @@ -769,8 +632,6 @@ const struct inode_operations sysfs_dir_inode_operations = { static void remove_dir(struct sysfs_dirent *dir_sd) { - struct sysfs_addrm_cxt acxt; - pr_debug("sysfs %s: removing dir\n", dir_sd->s_name); /* Removing non-empty directories is not valid complain! */ @@ -787,9 +648,7 @@ static void remove_dir(struct sysfs_dirent *dir_sd) mutex_unlock(&sysfs_mutex); } - sysfs_addrm_start(&acxt, dir_sd->s_parent); - sysfs_remove_one(&acxt, dir_sd); - sysfs_addrm_finish(&acxt); + sysfs_remove_one(dir_sd); } void sysfs_remove_subdir(struct sysfs_dirent *sd) @@ -818,14 +677,11 @@ static struct sysfs_dirent *get_dirent_to_remove(struct sysfs_dirent *dir_sd) static void __sysfs_remove_dir(struct sysfs_dirent *dir_sd) { - struct sysfs_addrm_cxt acxt; struct sysfs_dirent *sd; /* Remove children that we think are safe */ while ((sd = get_dirent_to_remove(dir_sd))) { - sysfs_addrm_start(&acxt, sd->s_parent); - sysfs_remove_one(&acxt, sd); - sysfs_addrm_finish(&acxt); + sysfs_remove_one(sd); sysfs_put(sd); } diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 31cfe1d..b512ce6 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -499,7 +499,6 @@ int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, const struct attribute *attr, int type, mode_t amode) { umode_t mode = (amode & S_IALLUGO) | S_IFREG; - struct sysfs_addrm_cxt acxt; struct sysfs_dirent *sd; int rc; @@ -508,10 +507,7 @@ int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, return -ENOMEM; sd->s_attr.attr = (void *)attr; - sysfs_addrm_start(&acxt, dir_sd); - rc = sysfs_add_one(&acxt, sd); - sysfs_addrm_finish(&acxt); - + rc = sysfs_add_one(dir_sd, sd); if (rc) sysfs_put(sd); diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 1b7ed3c..ad9a30d 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -263,23 +263,17 @@ void sysfs_delete_inode(struct inode *inode) int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name) { - struct sysfs_addrm_cxt acxt; struct sysfs_dirent *sd; if (!dir_sd) return -ENOENT; - sysfs_addrm_start(&acxt, dir_sd); - - sd = sysfs_find_dirent(dir_sd, name); - if (sd) - sysfs_remove_one(&acxt, sd); - - sysfs_addrm_finish(&acxt); - - if (sd) + sd = sysfs_get_dirent(dir_sd, name); + if (sd) { + sysfs_remove_one(sd); + sysfs_put(sd); return 0; - else + } else return -ENOENT; } diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c index 05e4984..fc5fc86 100644 --- a/fs/sysfs/symlink.c +++ b/fs/sysfs/symlink.c @@ -31,7 +31,6 @@ int sysfs_create_link(struct kobject *kobj, struct kobject *target, struct sysfs_dirent *parent_sd = NULL; struct sysfs_dirent *target_sd = NULL; struct sysfs_dirent *sd = NULL; - struct sysfs_addrm_cxt acxt; int error; BUG_ON(!name); @@ -65,10 +64,7 @@ int sysfs_create_link(struct kobject *kobj, struct kobject *target, sd->s_symlink.target_sd = target_sd; target_sd = NULL; /* reference is now owned by the symlink */ - sysfs_addrm_start(&acxt, parent_sd); - error = sysfs_add_one(&acxt, sd); - sysfs_addrm_finish(&acxt); - + error = sysfs_add_one(parent_sd, sd); if (error) goto out_put; diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index f5b53cf..f17ebb8 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -77,16 +77,6 @@ static inline unsigned int sysfs_type(struct sysfs_dirent *sd) } /* - * Context structure to be used while adding/removing nodes. - */ -struct sysfs_addrm_cxt { - struct sysfs_dirent *parent_sd; - struct inode *parent_inode; - struct sysfs_dirent *removed; - int cnt; -}; - -/* * mount.c */ extern struct sysfs_dirent sysfs_root; @@ -106,11 +96,8 @@ extern const struct inode_operations sysfs_dir_inode_operations; struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd); struct sysfs_dirent *sysfs_get_active_two(struct sysfs_dirent *sd); void sysfs_put_active_two(struct sysfs_dirent *sd); -void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, - struct sysfs_dirent *parent_sd); -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); -void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt); +int sysfs_add_one(struct sysfs_dirent *parent_sd, struct sysfs_dirent *sd); +void sysfs_remove_one(struct sysfs_dirent *sd); struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd, const unsigned char *name); -- 1.6.3.1.54.g99dd.dirty -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html