On Tue, Oct 26, 2021 at 07:42:15PM +0300, Konstantin Komarov wrote:
> Right now in ntfs_save_wsl_perm we lock/unlock 4 times.
> This commit fixes this situation.
> We add "locked" argument to ntfs_set_ea.
>
> Suggested-by: Kari Argillander <kari.argillander@xxxxxxxxx>
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
Add tag if you fix Joes nit.
Reviewed-by: Kari Argillander <kari.argillander@xxxxxxxxx>
> ---
> fs/ntfs3/xattr.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 157b70aecb4f..6d8b1cd7681d 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -259,7 +259,7 @@ static int ntfs_get_ea(struct inode *inode, const char *name, size_t name_len,
>
> static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> size_t name_len, const void *value,
> - size_t val_size, int flags)
> + size_t val_size, int flags, bool locked)
> {
> struct ntfs_inode *ni = ntfs_i(inode);
> struct ntfs_sb_info *sbi = ni->mi.sbi;
> @@ -278,7 +278,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> u64 new_sz;
> void *p;
>
> - ni_lock(ni);
> + if (!locked)
> + ni_lock(ni);
>
> run_init(&ea_run);
>
> @@ -467,7 +468,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> mark_inode_dirty(&ni->vfs_inode);
>
> out:
> - ni_unlock(ni);
> + if (!locked)
> + ni_unlock(ni);
>
> run_close(&ea_run);
> kfree(ea_all);
> @@ -595,7 +597,7 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
> flags = 0;
> }
>
> - err = ntfs_set_ea(inode, name, name_len, value, size, flags);
> + err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
> if (err == -ENODATA && !size)
> err = 0; /* Removing non existed xattr. */
> if (!err)
> @@ -989,7 +991,7 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
> }
> #endif
> /* Deal with NTFS extended attribute. */
> - err = ntfs_set_ea(inode, name, name_len, value, size, flags);
> + err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
Did you miss Joes comment or is there another reason why there is still
is (true|0) arguments?
https://lore.kernel.org/ntfs3/adcb168fc78f62583f8d925bcadbbcda9ba7da20.camel@xxxxxxxxxxx/
argillander
>
> out:
> inode->i_ctime = current_time(inode);
> @@ -1007,35 +1009,37 @@ int ntfs_save_wsl_perm(struct inode *inode)
> {
> int err;
> __le32 value;
> + struct ntfs_inode *ni = ntfs_i(inode);
>
> - /* TODO: refactor this, so we don't lock 4 times in ntfs_set_ea */
> + ni_lock(ni);
> value = cpu_to_le32(i_uid_read(inode));
> err = ntfs_set_ea(inode, "$LXUID", sizeof("$LXUID") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true); /* true == already locked. */
> if (err)
> goto out;
>
> value = cpu_to_le32(i_gid_read(inode));
> err = ntfs_set_ea(inode, "$LXGID", sizeof("$LXGID") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true);
> if (err)
> goto out;
>
> value = cpu_to_le32(inode->i_mode);
> err = ntfs_set_ea(inode, "$LXMOD", sizeof("$LXMOD") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true);
> if (err)
> goto out;
>
> if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
> value = cpu_to_le32(inode->i_rdev);
> err = ntfs_set_ea(inode, "$LXDEV", sizeof("$LXDEV") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true);
> if (err)
> goto out;
> }
>
> out:
> + ni_unlock(ni);
> /* In case of error should we delete all WSL xattr? */
> return err;
> }
> --
> 2.33.0
>
>
>
