btrfs/ioctl.c uses a "killable" lock on the directory when creating an destroying subvols. This patch adds lookup_and_lock_killable() and uses it in btrfs. Possibly all look_and_lock should be killable as there is no down-side, but that can come in a later patch. Signed-off-by: NeilBrown <neil@xxxxxxxxxx> --- fs/btrfs/ioctl.c | 49 ++++++++++++++----------------------------- fs/namei.c | 36 +++++++++++++++++++++++++++++++ include/linux/namei.h | 3 +++ 3 files changed, 55 insertions(+), 33 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 913acef3f0a9..9a3af4049c60 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -905,18 +905,15 @@ static noinline int btrfs_mksubvol(const struct path *parent, struct fscrypt_str name_str = FSTR_INIT((char *)name, namelen); int error; - error = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT); - if (error == -EINTR) - return error; - - dentry = lookup_one(idmap, &QSTR_LEN(name, namelen), parent->dentry); - error = PTR_ERR(dentry); + dentry = lookup_and_lock_killable(idmap, &QSTR_LEN(name, namelen), + parent->dentry, + LOOKUP_CREATE|LOOKUP_EXCL); if (IS_ERR(dentry)) - goto out_unlock; + return PTR_ERR(dentry); error = btrfs_may_create(idmap, dir, dentry); if (error) - goto out_dput; + goto out_unlock; /* * even if this name doesn't exist, we may get hash collisions. @@ -925,7 +922,7 @@ static noinline int btrfs_mksubvol(const struct path *parent, error = btrfs_check_dir_item_collision(BTRFS_I(dir)->root, dir->i_ino, &name_str); if (error) - goto out_dput; + goto out_unlock; down_read(&fs_info->subvol_sem); @@ -941,10 +938,8 @@ static noinline int btrfs_mksubvol(const struct path *parent, fsnotify_mkdir(dir, dentry); out_up_read: up_read(&fs_info->subvol_sem); -out_dput: - dput(dentry); out_unlock: - btrfs_inode_unlock(BTRFS_I(dir), 0); + dentry_unlock(dentry); return error; } @@ -2421,19 +2416,9 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file, goto free_subvol_name; } - ret = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT); - if (ret == -EINTR) + dentry = lookup_and_lock_killable(idmap, &QSTR(subvol_name), parent, 0); + if (IS_ERR(dentry)) goto free_subvol_name; - dentry = lookup_one(idmap, &QSTR(subvol_name), parent); - if (IS_ERR(dentry)) { - ret = PTR_ERR(dentry); - goto out_unlock_dir; - } - - if (d_really_is_negative(dentry)) { - ret = -ENOENT; - goto out_dput; - } inode = d_inode(dentry); dest = BTRFS_I(inode)->root; @@ -2453,7 +2438,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file, */ ret = -EPERM; if (!btrfs_test_opt(fs_info, USER_SUBVOL_RM_ALLOWED)) - goto out_dput; + goto out_unlock; /* * Do not allow deletion if the parent dir is the same @@ -2464,21 +2449,21 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file, */ ret = -EINVAL; if (root == dest) - goto out_dput; + goto out_unlock; ret = inode_permission(idmap, inode, MAY_WRITE | MAY_EXEC); if (ret) - goto out_dput; + goto out_unlock; } /* check if subvolume may be deleted by a user */ ret = btrfs_may_delete(idmap, dir, dentry, 1); if (ret) - goto out_dput; + goto out_unlock; if (btrfs_ino(BTRFS_I(inode)) != BTRFS_FIRST_FREE_OBJECTID) { ret = -EINVAL; - goto out_dput; + goto out_unlock; } btrfs_inode_lock(BTRFS_I(inode), 0); @@ -2487,10 +2472,8 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file, if (!ret) d_delete_notify(dir, dentry); -out_dput: - dput(dentry); -out_unlock_dir: - btrfs_inode_unlock(BTRFS_I(dir), 0); +out_unlock: + dentry_unlock(dentry); free_subvol_name: kfree(subvol_name_ptr); free_parent: diff --git a/fs/namei.c b/fs/namei.c index 5e8fe2d78486..55ce5700ba0e 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1814,6 +1814,42 @@ struct dentry *lookup_and_lock(struct mnt_idmap *idmap, } EXPORT_SYMBOL(lookup_and_lock); +/** + * lookup_and_lock_killable - lookup and lock a name prior to dir ops + * @last: the name in the given directory + * @base: the directory in which the name is to be found + * @lookup_flags: %LOOKUP_xxx flags + * + * The name is looked up and necessary locks are taken so that + * the name can be created or removed. + * The "necessary locks" are currently the inode node lock on @base. + * If a fatal signal arrives or is already pending the operation is aborted. + * The name @last is NOT expected to already have the hash calculated. + * Permission checks are performed to ensure %MAY_EXEC access to @base. + * Returns: the dentry, suitably locked, or an ERR_PTR(). + */ +struct dentry *lookup_and_lock_killable(struct mnt_idmap *idmap, + struct qstr *last, + struct dentry *base, + unsigned int lookup_flags) +{ + struct dentry *dentry; + int err; + + err = down_write_killable_nested(&base->d_inode->i_rwsem, I_MUTEX_PARENT); + if (err) + return ERR_PTR(err); + err = lookup_one_common(idmap, last, base); + if (err < 0) + return ERR_PTR(err); + + dentry = lookup_one_qstr_excl(last, base, lookup_flags); + if (IS_ERR(dentry)) + inode_unlock(base->d_inode); + return dentry; +} +EXPORT_SYMBOL(lookup_and_lock_killable); + void dentry_unlock_dir_locked(struct dentry *dentry) { d_lookup_done(dentry); diff --git a/include/linux/namei.h b/include/linux/namei.h index 378ee72b57f4..5177499a2f6b 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -83,6 +83,9 @@ struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap, struct dentry *lookup_and_lock(struct mnt_idmap *idmap, struct qstr *last, struct dentry *base, unsigned int lookup_flags); +struct dentry *lookup_and_lock_killable(struct mnt_idmap *idmap, + struct qstr *last, struct dentry *base, + unsigned int lookup_flags); struct dentry *lookup_and_lock_noperm(struct qstr *name, struct dentry *base, unsigned int lookup_flags); struct dentry *lookup_and_lock_noperm_locked(struct qstr *name, struct dentry *base, -- 2.49.0