On Wed, Sep 22, 2021 at 07:17:13PM +0300, Konstantin Komarov wrote:
> Now ntfs3 locks mutex for smaller time.
Really like this change. It was my todo list also. Thanks. Still some
comments below.
>
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
> ---
> fs/ntfs3/inode.c | 17 ++++++++++++++---
> fs/ntfs3/namei.c | 20 --------------------
> 2 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index d583b71bec50..6fc99eebd1c1 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -1198,9 +1198,13 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
> struct REPARSE_DATA_BUFFER *rp = NULL;
> bool rp_inserted = false;
>
> + ni_lock_dir(dir_ni);
> +
> dir_root = indx_get_root(&dir_ni->dir, dir_ni, NULL, NULL);
> - if (!dir_root)
> - return ERR_PTR(-EINVAL);
> + if (!dir_root) {
> + err = -EINVAL;
> + goto out1;
> + }
>
> if (S_ISDIR(mode)) {
> /* Use parent's directory attributes. */
> @@ -1549,6 +1553,9 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
> if (err)
> goto out6;
>
> + /* Unlock parent directory before ntfs_init_acl. */
> + ni_unlock(dir_ni);
There is err path which can go to err6 (line 1585). Then we get double
unlock situation.
> +
> inode->i_generation = le16_to_cpu(rec->seq);
>
> dir->i_mtime = dir->i_ctime = inode->i_atime;
> @@ -1605,8 +1612,10 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
> out7:
>
> /* Undo 'indx_insert_entry'. */
> + ni_lock_dir(dir_ni);
> indx_delete_entry(&dir_ni->dir, dir_ni, new_de + 1,
> le16_to_cpu(new_de->key_size), sbi);
> + /* ni_unlock(dir_ni); will be called later. */
> out6:
> if (rp_inserted)
> ntfs_remove_reparse(sbi, IO_REPARSE_TAG_SYMLINK, &new_de->ref);
> @@ -1630,8 +1639,10 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
> kfree(rp);
>
> out1:
> - if (err)
> + if (err) {
> + ni_unlock(dir_ni);
This will be double unlock if we exit with err path out6.
Argillander
> return ERR_PTR(err);
> + }
>
> unlock_new_inode(inode);
>
> diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
> index 1c475da4e19d..bc741213ad84 100644
> --- a/fs/ntfs3/namei.c
> +++ b/fs/ntfs3/namei.c
> @@ -95,16 +95,11 @@ static struct dentry *ntfs_lookup(struct inode *dir, struct dentry *dentry,
> static int ntfs_create(struct user_namespace *mnt_userns, struct inode *dir,
> struct dentry *dentry, umode_t mode, bool excl)
> {
> - struct ntfs_inode *ni = ntfs_i(dir);
> struct inode *inode;
>
> - ni_lock_dir(ni);
> -
> inode = ntfs_create_inode(mnt_userns, dir, dentry, NULL, S_IFREG | mode,
> 0, NULL, 0, NULL);
>
> - ni_unlock(ni);
> -
> return IS_ERR(inode) ? PTR_ERR(inode) : 0;
> }
>
> @@ -116,16 +111,11 @@ static int ntfs_create(struct user_namespace *mnt_userns, struct inode *dir,
> static int ntfs_mknod(struct user_namespace *mnt_userns, struct inode *dir,
> struct dentry *dentry, umode_t mode, dev_t rdev)
> {
> - struct ntfs_inode *ni = ntfs_i(dir);
> struct inode *inode;
>
> - ni_lock_dir(ni);
> -
> inode = ntfs_create_inode(mnt_userns, dir, dentry, NULL, mode, rdev,
> NULL, 0, NULL);
>
> - ni_unlock(ni);
> -
> return IS_ERR(inode) ? PTR_ERR(inode) : 0;
> }
>
> @@ -196,15 +186,10 @@ static int ntfs_symlink(struct user_namespace *mnt_userns, struct inode *dir,
> {
> u32 size = strlen(symname);
> struct inode *inode;
> - struct ntfs_inode *ni = ntfs_i(dir);
> -
> - ni_lock_dir(ni);
>
> inode = ntfs_create_inode(mnt_userns, dir, dentry, NULL, S_IFLNK | 0777,
> 0, symname, size, NULL);
>
> - ni_unlock(ni);
> -
> return IS_ERR(inode) ? PTR_ERR(inode) : 0;
> }
>
> @@ -215,15 +200,10 @@ static int ntfs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
> struct dentry *dentry, umode_t mode)
> {
> struct inode *inode;
> - struct ntfs_inode *ni = ntfs_i(dir);
> -
> - ni_lock_dir(ni);
>
> inode = ntfs_create_inode(mnt_userns, dir, dentry, NULL, S_IFDIR | mode,
> 0, NULL, 0, NULL);
>
> - ni_unlock(ni);
> -
> return IS_ERR(inode) ? PTR_ERR(inode) : 0;
> }
>
> --
> 2.33.0
>
>
