| Message ID | a269be2f-4ab6-b1c6-790c-9d3052bf22cc@paragon-software.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor locking in inode_operations | expand |
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@paragon-software.com> > --- > 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 > >
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); + 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); 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; }
Now ntfs3 locks mutex for smaller time. Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> --- fs/ntfs3/inode.c | 17 ++++++++++++++--- fs/ntfs3/namei.c | 20 -------------------- 2 files changed, 14 insertions(+), 23 deletions(-)