From htejun@gmail.com Wed Jun 13 11:45:32 2007 From: Tejun Heo Cc: Tejun Heo , Greg Kroah-Hartman Subject: [PATCH 20/31] sysfs: reimplement sysfs_drop_dentry() Date: Thu, 14 Jun 2007 03:45:16 +0900 Message-Id: <11817603163525-git-send-email-htejun@gmail.com> To: greg@kroah.com, rjw@sisk.pl, akpm@linux-foundation.org, htejun@gmail.com From: Tejun Heo This patch reimplements sysfs_drop_dentry() such that remove_dir() can use it to drop dentry instead of using a separate mechanism. With this change, making directories reclaimable is much easier. This patch used to contain fixes for two race conditions around sd->s_dentry but that part has been separated out and included into mainline early as commit 6aa054aadfea613a437ad0b15d38eca2b963fc0a and dd14cbc994709a1c5a64ed3621f583c49a27e521. Signed-off-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 18 +++--------- fs/sysfs/inode.c | 82 ++++++++++++++++++++++++++++++++++++++++--------------- fs/sysfs/sysfs.h | 2 - 3 files changed, 67 insertions(+), 35 deletions(-) --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -372,22 +372,19 @@ const struct inode_operations sysfs_dir_ static void remove_dir(struct dentry * d) { - struct dentry * parent = dget(d->d_parent); - struct sysfs_dirent * sd; + struct dentry *parent = d->d_parent; + struct sysfs_dirent *sd = d->d_fsdata; mutex_lock(&parent->d_inode->i_mutex); - d_delete(d); - sd = d->d_fsdata; + list_del_init(&sd->s_sibling); - if (d->d_inode) - simple_rmdir(parent->d_inode,d); pr_debug(" o %s removing done (%d)\n",d->d_name.name, atomic_read(&d->d_count)); mutex_unlock(&parent->d_inode->i_mutex); - dput(parent); + sysfs_drop_dentry(sd); sysfs_deactivate(sd); sysfs_put(sd); } @@ -404,7 +401,6 @@ static void __sysfs_remove_dir(struct de struct sysfs_dirent * parent_sd; struct sysfs_dirent * sd, * tmp; - dget(dentry); if (!dentry) return; @@ -415,21 +411,17 @@ static void __sysfs_remove_dir(struct de if (!sd->s_type || !(sd->s_type & SYSFS_NOT_PINNED)) continue; list_move(&sd->s_sibling, &removed); - sysfs_drop_dentry(sd, dentry); } mutex_unlock(&dentry->d_inode->i_mutex); list_for_each_entry_safe(sd, tmp, &removed, s_sibling) { list_del_init(&sd->s_sibling); + sysfs_drop_dentry(sd); sysfs_deactivate(sd); sysfs_put(sd); } remove_dir(dentry); - /** - * Drop reference from dget() on entrance. - */ - dput(dentry); } /** --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -191,13 +191,25 @@ int sysfs_create(struct sysfs_dirent *sd return error; } -/* - * Unhashes the dentry corresponding to given sysfs_dirent - * Called with parent inode's i_mutex held. +/** + * sysfs_drop_dentry - drop dentry for the specified sysfs_dirent + * @sd: target sysfs_dirent + * + * Drop dentry for @sd. @sd must have been unlinked from its + * parent on entry to this function such that it can't be looked + * up anymore. + * + * @sd->s_dentry which is protected with sysfs_lock points to the + * currently associated dentry but we're not holding a reference + * to it and racing with dput(). Grab dcache_lock and verify + * dentry before dropping it. If @sd->s_dentry is NULL or dput() + * beats us, no need to bother. */ -void sysfs_drop_dentry(struct sysfs_dirent * sd, struct dentry * parent) +void sysfs_drop_dentry(struct sysfs_dirent *sd) { - struct dentry *dentry = NULL; + struct dentry *dentry = NULL, *parent = NULL; + struct inode *dir; + struct timespec curtime; /* We're not holding a reference to ->s_dentry dentry but the * field will stay valid as long as sysfs_lock is held. @@ -205,30 +217,57 @@ void sysfs_drop_dentry(struct sysfs_dire spin_lock(&sysfs_lock); spin_lock(&dcache_lock); - /* dget dentry if it's still alive */ - if (sd->s_dentry && sd->s_dentry->d_inode) + if (sd->s_dentry && sd->s_dentry->d_inode) { + /* get dentry if it's there and dput() didn't kill it yet */ dentry = dget_locked(sd->s_dentry); + parent = dentry->d_parent; + } else if (sd->s_parent->s_dentry->d_inode) { + /* We need to update the parent even if dentry for the + * victim itself doesn't exist. + */ + parent = dget_locked(sd->s_parent->s_dentry); + } + + /* drop */ + if (dentry) { + spin_lock(&dentry->d_lock); + __d_drop(dentry); + spin_unlock(&dentry->d_lock); + } spin_unlock(&dcache_lock); spin_unlock(&sysfs_lock); - /* drop dentry */ + /* nothing to do if the parent isn't in dcache */ + if (!parent) + return; + + /* adjust nlink and update timestamp */ + dir = parent->d_inode; + mutex_lock(&dir->i_mutex); + + curtime = CURRENT_TIME; + + dir->i_ctime = dir->i_mtime = curtime; + if (dentry) { - spin_lock(&dcache_lock); - spin_lock(&dentry->d_lock); - if (!d_unhashed(dentry) && dentry->d_inode) { - dget_locked(dentry); - __d_drop(dentry); - spin_unlock(&dentry->d_lock); - spin_unlock(&dcache_lock); - simple_unlink(parent->d_inode, dentry); - } else { - spin_unlock(&dentry->d_lock); - spin_unlock(&dcache_lock); + dentry->d_inode->i_ctime = curtime; + drop_nlink(dentry->d_inode); + if (sd->s_type & SYSFS_DIR) { + drop_nlink(dentry->d_inode); + drop_nlink(dir); + /* XXX: unpin if directory, this will go away soon */ + dput(dentry); } + } + mutex_unlock(&dir->i_mutex); + + /* bye bye */ + if (dentry) dput(dentry); - } + else + dput(parent); } int sysfs_hash_and_remove(struct dentry * dir, const char * name) @@ -251,7 +290,6 @@ int sysfs_hash_and_remove(struct dentry continue; if (!strcmp(sd->s_name, name)) { list_del_init(&sd->s_sibling); - sysfs_drop_dentry(sd, dir); found = 1; break; } @@ -261,7 +299,9 @@ int sysfs_hash_and_remove(struct dentry if (!found) return -ENOENT; + sysfs_drop_dentry(sd); sysfs_deactivate(sd); sysfs_put(sd); + return 0; } --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -76,7 +76,7 @@ extern struct sysfs_dirent *sysfs_find(s extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **); extern void sysfs_remove_subdir(struct dentry *); -extern void sysfs_drop_dentry(struct sysfs_dirent *sd, struct dentry *parent); +extern void sysfs_drop_dentry(struct sysfs_dirent *sd); extern int sysfs_setattr(struct dentry *dentry, struct iattr *iattr); extern spinlock_t sysfs_lock;