Messages in this thread Patch in this message | | | From | (Eric W. Biederman) | Date | Tue, 07 Oct 2008 03:51:17 -0700 | Subject | [PATCH 2/3] sysfs: Fix and sysfs_mv_dir by using lock_rename. |
| |
sysfs_mv_dir picks an arbitrary mutex lock ordering and busy waits until it can grab the inode mutexes in that order. This can dead lock on UP systems.
Instead use the generic lock_rename that guarantees we grab parent directories before child directories, so there is not need to spin and no chance of deadlock.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> --- fs/sysfs/dir.c | 43 +++++++++++++++++++------------------------ 1 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index a76fb54..94f2ed1 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -955,12 +955,22 @@ err_out: return error; } +static void sysfs_lock_rename(struct sysfs_rename_struct *srs) +{ + lock_rename(srs->old_parent, srs->new_parent); +} + +static void sysfs_unlock_rename(struct sysfs_rename_struct *srs) +{ + unlock_rename(srs->old_parent, srs->new_parent); +} + static int sysfs_mv_dir(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent_sd, const char *new_name) { struct list_head todo; struct sysfs_rename_struct *srs; - struct inode *old_parent_inode = NULL, *new_parent_inode = NULL; + struct sysfs_dirent *old_parent_sd; const char *dup_name = NULL; const void *old_tag, *tag; int error; @@ -971,11 +981,12 @@ static int sysfs_mv_dir(struct sysfs_dirent *sd, if (!new_parent_sd) new_parent_sd = &sysfs_root; + old_parent_sd = sysfs_get(sd->s_parent); old_tag = sd->s_tag; - tag = sysfs_creation_tag(sd->s_parent, sd); + tag = sysfs_creation_tag(old_parent_sd, sd); error = 0; - if ((sd->s_parent == new_parent_sd) && (old_tag == tag) && + if ((old_parent_sd == new_parent_sd) && (old_tag == tag) && (strcmp(sd->s_name, new_name) == 0)) goto out; /* nothing to do */ @@ -986,22 +997,8 @@ static int sysfs_mv_dir(struct sysfs_dirent *sd, goto out_release; } - error = -ENOMEM; - mutex_lock(&sysfs_mutex); - old_parent_inode = sysfs_get_inode(sd->s_parent); - new_parent_inode = sysfs_get_inode(new_parent_sd); - mutex_unlock(&sysfs_mutex); - if (!old_parent_inode || !new_parent_inode) - goto out_release; - -again: - mutex_lock(&old_parent_inode->i_mutex); - if (old_parent_inode != new_parent_inode) { - if (!mutex_trylock(&new_parent_inode->i_mutex)) { - mutex_unlock(&old_parent_inode->i_mutex); - goto again; - } - } + if (!list_empty(&todo)) + sysfs_lock_rename(list_first_entry(&todo, struct sysfs_rename_struct, list)); mutex_lock(&sysfs_mutex); error = -EEXIST; @@ -1048,17 +1045,15 @@ again: error = 0; out_unlock: mutex_unlock(&sysfs_mutex); - if (new_parent_inode != old_parent_inode) - mutex_unlock(&new_parent_inode->i_mutex); - mutex_unlock(&old_parent_inode->i_mutex); + if (!list_empty(&todo)) + sysfs_unlock_rename(list_first_entry(&todo, struct sysfs_rename_struct, list)); kfree(dup_name); out_release: - iput(new_parent_inode); - iput(old_parent_inode); post_rename(&todo); sysfs_release_supers(); out: + sysfs_put(old_parent_sd); mutex_unlock(&sysfs_rename_mutex); return error; } -- 1.5.3.rc6.17.g1911
| |