On 07/11, Chao Yu wrote:
> On 2023/7/11 1:18, Jaegeuk Kim wrote:
> > On 07/10, Chao Yu wrote:
> > > syzbot reports deadlock bug as below:
> > > 
> > > -> #1 (&fi->i_sem){+.+.}-{3:3}:
> > >         down_write+0x3a/0x50 kernel/locking/rwsem.c:1573
> > >         f2fs_down_write fs/f2fs/f2fs.h:2133 [inline]
> > >         f2fs_add_inline_entry+0x3a8/0x760 fs/f2fs/inline.c:644
> > >         f2fs_add_dentry+0xba/0x1e0 fs/f2fs/dir.c:784
> > >         f2fs_do_add_link+0x21e/0x340 fs/f2fs/dir.c:827
> > >         f2fs_add_link fs/f2fs/f2fs.h:3554 [inline]
> > >         f2fs_create+0x32c/0x530 fs/f2fs/namei.c:377
> > >         lookup_open fs/namei.c:3492 [inline]
> > >         open_last_lookups fs/namei.c:3560 [inline]
> > >         path_openat+0x13e7/0x3180 fs/namei.c:3790
> > >         do_filp_open+0x234/0x490 fs/namei.c:3820
> > >         do_sys_openat2+0x13e/0x1d0 fs/open.c:1407
> > >         do_sys_open fs/open.c:1422 [inline]
> > >         __do_sys_open fs/open.c:1430 [inline]
> > >         __se_sys_open fs/open.c:1426 [inline]
> > >         __x64_sys_open+0x225/0x270 fs/open.c:1426
> > >         do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > >         do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> > >         entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > 
> > > -> #0 (&fi->i_xattr_sem){.+.+}-{3:3}:
> > >         check_prev_add kernel/locking/lockdep.c:3142 [inline]
> > >         check_prevs_add kernel/locking/lockdep.c:3261 [inline]
> > >         validate_chain kernel/locking/lockdep.c:3876 [inline]
> > >         __lock_acquire+0x39ff/0x7f70 kernel/locking/lockdep.c:5144
> > >         lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5761
> > >         down_read+0x47/0x2f0 kernel/locking/rwsem.c:1520
> > >         f2fs_down_read fs/f2fs/f2fs.h:2108 [inline]
> > >         f2fs_getxattr+0xb8/0x1460 fs/f2fs/xattr.c:532
> > >         __f2fs_get_acl+0x52/0x8e0 fs/f2fs/acl.c:179
> > >         f2fs_acl_create fs/f2fs/acl.c:377 [inline]
> > >         f2fs_init_acl+0xd7/0x9a0 fs/f2fs/acl.c:420
> > >         f2fs_init_inode_metadata+0x824/0x1190 fs/f2fs/dir.c:558
> > >         f2fs_do_tmpfile+0x34/0x170 fs/f2fs/dir.c:839
> > >         __f2fs_tmpfile+0x1f9/0x380 fs/f2fs/namei.c:884
> > >         f2fs_ioc_start_atomic_write+0x4a3/0x9e0 fs/f2fs/file.c:2099
> > >         __f2fs_ioctl+0x1b5c/0xb770
> > >         vfs_ioctl fs/ioctl.c:51 [inline]
> > >         __do_sys_ioctl fs/ioctl.c:870 [inline]
> > >         __se_sys_ioctl+0xf8/0x170 fs/ioctl.c:856
> > >         do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > >         do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> > >         entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > 
> > > The root cause is as below reversed lock order:
> > > - f2fs_create
> > >   - f2fs_add_dentry
> > >    - f2fs_down_write(&F2FS_I(dir)->i_xattr_sem)
> > >                                           - f2fs_ioc_start_atomic_write
> > >                                            - __f2fs_tmpfile
> > >                                             - f2fs_do_tmpfile
> > >                                              - 
> > > f2fs_down_write(&F2FS_I(inode)->i_sem)
> > >                                              - f2fs_init_inode_metadata
> > >                                               - f2fs_init_acl
> > >                                                - __f2fs_get_acl
> > >                                                 - f2fs_getxattr
> > >                                                  - 
> > > f2fs_down_read(&F2FS_I(dir)->i_xattr_sem)
> > >    - f2fs_add_inline_entry
> > >     - f2fs_down_write(&F2FS_I(inode)->i_sem)
> > 
> > How is it possible to get two inode are identical?
> 
> Oh, so, it looks like a false positive?
> 
> BTW, except commit message, what do you think of the change itself?
> We can using i_sem to instead i_xattr_sem, so that 1) it can shrink
> inode's size, 2) it's more clear that use common i_sem lock to protect
> inode's {,x}attr access.

If then, xattr can be blocked by unnecessary i_sem lock?

> 
> Thanks,
> 
> > 
> > > 
> > > We can break the dependency of deadlock by below change:
> > > - use i_sem to keep order of {get,set}xattr instead of i_xattr_sem
> > > - keep below lock order in inode operation to avoid deadlock:
> > >   dir->i_sem -> inode->i_sem
> > >   dir->i_sem -> dpage_lock
> > > 
> > > Fixes: 5eda1ad1aaff ("f2fs: fix deadlock in i_xattr_sem and inode page 
> > > lock")
> > > Reported-by: [email protected]
> > > Closes: 
> > > https://lore.kernel.org/linux-f2fs-devel/[email protected]
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > >   fs/f2fs/acl.c    | 36 +++++++++++++++++++-----------------
> > >   fs/f2fs/acl.h    |  4 ++--
> > >   fs/f2fs/dir.c    | 22 ++++++++++++++++------
> > >   fs/f2fs/f2fs.h   |  1 -
> > >   fs/f2fs/super.c  |  5 ++---
> > >   fs/f2fs/verity.c |  5 +++--
> > >   fs/f2fs/xattr.c  | 37 +++++++++++++++++++++----------------
> > >   fs/f2fs/xattr.h  | 19 +++++++++++++------
> > >   8 files changed, 76 insertions(+), 53 deletions(-)
> > > 
> > > diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> > > index ec2aeccb69a3..af58b38e953c 100644
> > > --- a/fs/f2fs/acl.c
> > > +++ b/fs/f2fs/acl.c
> > > @@ -166,7 +166,7 @@ static void *f2fs_acl_to_disk(struct f2fs_sb_info 
> > > *sbi,
> > >   }
> > >   static struct posix_acl *__f2fs_get_acl(struct inode *inode, int type,
> > > -                                         struct page *dpage)
> > > +                                         struct page *dpage, bool locked)
> > >   {
> > >           int name_index = F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT;
> > >           void *value = NULL;
> > > @@ -176,13 +176,13 @@ static struct posix_acl *__f2fs_get_acl(struct 
> > > inode *inode, int type,
> > >           if (type == ACL_TYPE_ACCESS)
> > >                   name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
> > > - retval = f2fs_getxattr(inode, name_index, "", NULL, 0, dpage);
> > > + retval = f2fs_getxattr(inode, name_index, "", NULL, 0, dpage, locked);
> > >           if (retval > 0) {
> > >                   value = f2fs_kmalloc(F2FS_I_SB(inode), retval, 
> > > GFP_F2FS_ZERO);
> > >                   if (!value)
> > >                           return ERR_PTR(-ENOMEM);
> > >                   retval = f2fs_getxattr(inode, name_index, "", value,
> > > -                                                 retval, dpage);
> > > +                                                 retval, dpage, locked);
> > >           }
> > >           if (retval > 0)
> > > @@ -201,7 +201,7 @@ struct posix_acl *f2fs_get_acl(struct inode *inode, 
> > > int type, bool rcu)
> > >           if (rcu)
> > >                   return ERR_PTR(-ECHILD);
> > > - return __f2fs_get_acl(inode, type, NULL);
> > > + return __f2fs_get_acl(inode, type, NULL, false);
> > >   }
> > >   static int f2fs_acl_update_mode(struct mnt_idmap *idmap,
> > > @@ -226,9 +226,9 @@ static int f2fs_acl_update_mode(struct mnt_idmap 
> > > *idmap,
> > >           return 0;
> > >   }
> > > -static int __f2fs_set_acl(struct mnt_idmap *idmap,
> > > -                 struct inode *inode, int type,
> > > -                 struct posix_acl *acl, struct page *ipage)
> > > +static int __f2fs_set_acl(struct mnt_idmap *idmap, struct inode *inode,
> > > +                                 int type, struct posix_acl *acl,
> > > +                                 struct page *ipage, bool locked)
> > >   {
> > >           int name_index;
> > >           void *value = NULL;
> > > @@ -266,7 +266,8 @@ static int __f2fs_set_acl(struct mnt_idmap *idmap,
> > >                   }
> > >           }
> > > - error = f2fs_setxattr(inode, name_index, "", value, size, ipage, 0);
> > > + error = f2fs_setxattr(inode, name_index, "", value, size,
> > > +                                         ipage, 0, locked);
> > >           kfree(value);
> > >           if (!error)
> > > @@ -284,7 +285,7 @@ int f2fs_set_acl(struct mnt_idmap *idmap, struct 
> > > dentry *dentry,
> > >           if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
> > >                   return -EIO;
> > > - return __f2fs_set_acl(idmap, inode, type, acl, NULL);
> > > + return __f2fs_set_acl(idmap, inode, type, acl, NULL, false);
> > >   }
> > >   /*
> > > @@ -362,7 +363,7 @@ static int f2fs_acl_create_masq(struct posix_acl 
> > > *acl, umode_t *mode_p)
> > >   static int f2fs_acl_create(struct inode *dir, umode_t *mode,
> > >                   struct posix_acl **default_acl, struct posix_acl **acl,
> > > -         struct page *dpage)
> > > +         struct page *dpage, bool locked)
> > >   {
> > >           struct posix_acl *p;
> > >           struct posix_acl *clone;
> > > @@ -374,7 +375,7 @@ static int f2fs_acl_create(struct inode *dir, umode_t 
> > > *mode,
> > >           if (S_ISLNK(*mode) || !IS_POSIXACL(dir))
> > >                   return 0;
> > > - p = __f2fs_get_acl(dir, ACL_TYPE_DEFAULT, dpage);
> > > + p = __f2fs_get_acl(dir, ACL_TYPE_DEFAULT, dpage, locked);
> > >           if (!p || p == ERR_PTR(-EOPNOTSUPP)) {
> > >                   *mode &= ~current_umask();
> > >                   return 0;
> > > @@ -412,28 +413,29 @@ static int f2fs_acl_create(struct inode *dir, 
> > > umode_t *mode,
> > >   }
> > >   int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page 
> > > *ipage,
> > > -                                                 struct page *dpage)
> > > +                                         struct page *dpage, bool locked)
> > >   {
> > >           struct posix_acl *default_acl = NULL, *acl = NULL;
> > >           int error;
> > > - error = f2fs_acl_create(dir, &inode->i_mode, &default_acl, &acl, dpage);
> > > + error = f2fs_acl_create(dir, &inode->i_mode, &default_acl, &acl,
> > > +                                                 dpage, locked);
> > >           if (error)
> > >                   return error;
> > >           f2fs_mark_inode_dirty_sync(inode, true);
> > >           if (default_acl) {
> > > -         error = __f2fs_set_acl(NULL, inode, ACL_TYPE_DEFAULT, 
> > > default_acl,
> > > -                                ipage);
> > > +         error = __f2fs_set_acl(NULL, inode, ACL_TYPE_DEFAULT,
> > > +                                 default_acl, ipage, locked);
> > >                   posix_acl_release(default_acl);
> > >           } else {
> > >                   inode->i_default_acl = NULL;
> > >           }
> > >           if (acl) {
> > >                   if (!error)
> > > -                 error = __f2fs_set_acl(NULL, inode, ACL_TYPE_ACCESS, 
> > > acl,
> > > -                                        ipage);
> > > +                 error = __f2fs_set_acl(NULL, inode, ACL_TYPE_ACCESS,
> > > +                                                 acl, ipage, locked);
> > >                   posix_acl_release(acl);
> > >           } else {
> > >                   inode->i_acl = NULL;
> > > diff --git a/fs/f2fs/acl.h b/fs/f2fs/acl.h
> > > index 94ebfbfbdc6f..9c14b6f549c6 100644
> > > --- a/fs/f2fs/acl.h
> > > +++ b/fs/f2fs/acl.h
> > > @@ -37,13 +37,13 @@ extern struct posix_acl *f2fs_get_acl(struct inode *, 
> > > int, bool);
> > >   extern int f2fs_set_acl(struct mnt_idmap *, struct dentry *,
> > >                           struct posix_acl *, int);
> > >   extern int f2fs_init_acl(struct inode *, struct inode *, struct page *,
> > > -                                                 struct page *);
> > > +                                                 struct page *, bool);
> > >   #else
> > >   #define f2fs_get_acl    NULL
> > >   #define f2fs_set_acl    NULL
> > >   static inline int f2fs_init_acl(struct inode *inode, struct inode *dir,
> > > -                         struct page *ipage, struct page *dpage)
> > > +                 struct page *ipage, struct page *dpage, bool locked)
> > >   {
> > >           return 0;
> > >   }
> > > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> > > index d635c58cf5a3..4b5c62e18d67 100644
> > > --- a/fs/f2fs/dir.c
> > > +++ b/fs/f2fs/dir.c
> > > @@ -540,6 +540,8 @@ struct page *f2fs_init_inode_metadata(struct inode 
> > > *inode, struct inode *dir,
> > >           int err;
> > >           if (is_inode_flag_set(inode, FI_NEW_INODE)) {
> > > +         struct f2fs_xattr_arg xarg;
> > > +
> > >                   page = f2fs_new_inode_page(inode);
> > >                   if (IS_ERR(page))
> > >                           return page;
> > > @@ -555,12 +557,16 @@ struct page *f2fs_init_inode_metadata(struct inode 
> > > *inode, struct inode *dir,
> > >                           put_page(page);
> > >                   }
> > > -         err = f2fs_init_acl(inode, dir, page, dpage);
> > > +         err = f2fs_init_acl(inode, dir, page, dpage, true);
> > >                   if (err)
> > >                           goto put_error;
> > > +         xarg.page = page;
> > > +         xarg.locked = true;
> > > +
> > >                   err = f2fs_init_security(inode, dir,
> > > -                                  fname ? fname->usr_fname : NULL, page);
> > > +                                  fname ? fname->usr_fname : NULL,
> > > +                                  &xarg);
> > >                   if (err)
> > >                           goto put_error;
> > > @@ -775,18 +781,20 @@ int f2fs_add_dentry(struct inode *dir, const struct 
> > > f2fs_filename *fname,
> > >   {
> > >           int err = -EAGAIN;
> > > + f2fs_down_read(&F2FS_I(dir)->i_sem);
> > > +
> > >           if (f2fs_has_inline_dentry(dir)) {
> > >                   /*
> > > -          * Should get i_xattr_sem to keep the lock order:
> > > -          * i_xattr_sem -> inode_page lock used by f2fs_setxattr.
> > > +          * Should get i_sem to keep the lock order:
> > > +          * i_sem -> inode_page lock used by f2fs_setxattr.
> > >                    */
> > > -         f2fs_down_read(&F2FS_I(dir)->i_xattr_sem);
> > >                   err = f2fs_add_inline_entry(dir, fname, inode, ino, 
> > > mode);
> > > -         f2fs_up_read(&F2FS_I(dir)->i_xattr_sem);
> > >           }
> > >           if (err == -EAGAIN)
> > >                   err = f2fs_add_regular_entry(dir, fname, inode, ino, 
> > > mode);
> > > + f2fs_up_read(&F2FS_I(dir)->i_sem);
> > > +
> > >           f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
> > >           return err;
> > >   }
> > > @@ -835,6 +843,7 @@ int f2fs_do_tmpfile(struct inode *inode, struct inode 
> > > *dir)
> > >           struct page *page;
> > >           int err = 0;
> > > + f2fs_down_write(&F2FS_I(dir)->i_sem);
> > >           f2fs_down_write(&F2FS_I(inode)->i_sem);
> > >           page = f2fs_init_inode_metadata(inode, dir, NULL, NULL);
> > >           if (IS_ERR(page)) {
> > > @@ -847,6 +856,7 @@ int f2fs_do_tmpfile(struct inode *inode, struct inode 
> > > *dir)
> > >           f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
> > >   fail:
> > >           f2fs_up_write(&F2FS_I(inode)->i_sem);
> > > + f2fs_up_write(&F2FS_I(dir)->i_sem);
> > >           return err;
> > >   }
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index c7cb2177b252..60ec032be48d 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -838,7 +838,6 @@ struct f2fs_inode_info {
> > >           /* avoid racing between foreground op and gc */
> > >           struct f2fs_rwsem i_gc_rwsem[2];
> > > - struct f2fs_rwsem i_xattr_sem; /* avoid racing between reading and 
> > > changing EAs */
> > >           int i_extra_isize;              /* size of extra space located 
> > > in i_addr */
> > >           kprojid_t i_projid;             /* id for project quota */
> > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > index ca31163da00a..c72fda24cffd 100644
> > > --- a/fs/f2fs/super.c
> > > +++ b/fs/f2fs/super.c
> > > @@ -1418,7 +1418,6 @@ static struct inode *f2fs_alloc_inode(struct 
> > > super_block *sb)
> > >           INIT_LIST_HEAD(&fi->gdirty_list);
> > >           init_f2fs_rwsem(&fi->i_gc_rwsem[READ]);
> > >           init_f2fs_rwsem(&fi->i_gc_rwsem[WRITE]);
> > > - init_f2fs_rwsem(&fi->i_xattr_sem);
> > >           /* Will be used by directory only */
> > >           fi->i_dir_level = F2FS_SB(sb)->dir_level;
> > > @@ -3163,7 +3162,7 @@ static int f2fs_get_context(struct inode *inode, 
> > > void *ctx, size_t len)
> > >   {
> > >           return f2fs_getxattr(inode, F2FS_XATTR_INDEX_ENCRYPTION,
> > >                                   F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
> > > -                         ctx, len, NULL);
> > > +                         ctx, len, NULL, false);
> > >   }
> > >   static int f2fs_set_context(struct inode *inode, const void *ctx, 
> > > size_t len,
> > > @@ -3183,7 +3182,7 @@ static int f2fs_set_context(struct inode *inode, 
> > > const void *ctx, size_t len,
> > >           return f2fs_setxattr(inode, F2FS_XATTR_INDEX_ENCRYPTION,
> > >                                   F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
> > > -                         ctx, len, fs_data, XATTR_CREATE);
> > > +                         ctx, len, fs_data, XATTR_CREATE, true);
> > >   }
> > >   static const union fscrypt_policy *f2fs_get_dummy_policy(struct 
> > > super_block *sb)
> > > diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
> > > index 4fc95f353a7a..e181528b1f26 100644
> > > --- a/fs/f2fs/verity.c
> > > +++ b/fs/f2fs/verity.c
> > > @@ -181,7 +181,7 @@ static int f2fs_end_enable_verity(struct file *filp, 
> > > const void *desc,
> > >           /* Set the verity xattr. */
> > >           err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_VERITY,
> > >                               F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc),
> > > -                     NULL, XATTR_CREATE);
> > > +                     NULL, XATTR_CREATE, false);
> > >           if (err)
> > >                   goto cleanup;
> > > @@ -226,7 +226,8 @@ static int f2fs_get_verity_descriptor(struct inode 
> > > *inode, void *buf,
> > >           /* Get the descriptor location */
> > >           res = f2fs_getxattr(inode, F2FS_XATTR_INDEX_VERITY,
> > > -                     F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc), NULL);
> > > +                     F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc),
> > > +                     NULL, false);
> > >           if (res < 0 && res != -ERANGE)
> > >                   return res;
> > >           if (res != sizeof(dloc) || dloc.version != 
> > > cpu_to_le32(F2FS_VERIFY_VER)) {
> > > diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> > > index 476b186b90a6..a6a611f8a771 100644
> > > --- a/fs/f2fs/xattr.c
> > > +++ b/fs/f2fs/xattr.c
> > > @@ -61,7 +61,7 @@ static int f2fs_xattr_generic_get(const struct 
> > > xattr_handler *handler,
> > >                   return -EINVAL;
> > >           }
> > >           return f2fs_getxattr(inode, handler->flags, name,
> > > -                      buffer, size, NULL);
> > > +                      buffer, size, NULL, false);
> > >   }
> > >   static int f2fs_xattr_generic_set(const struct xattr_handler *handler,
> > > @@ -84,7 +84,7 @@ static int f2fs_xattr_generic_set(const struct 
> > > xattr_handler *handler,
> > >                   return -EINVAL;
> > >           }
> > >           return f2fs_setxattr(inode, handler->flags, name,
> > > -                                 value, size, NULL, flags);
> > > +                                 value, size, NULL, flags, false);
> > >   }
> > >   static bool f2fs_xattr_user_list(struct dentry *dentry)
> > > @@ -136,15 +136,16 @@ static int f2fs_xattr_advise_set(const struct 
> > > xattr_handler *handler,
> > >   #ifdef CONFIG_F2FS_FS_SECURITY
> > >   static int f2fs_initxattrs(struct inode *inode, const struct xattr 
> > > *xattr_array,
> > > -         void *page)
> > > +                                                         void *fs_data)
> > >   {
> > > + struct f2fs_xattr_arg *xarg = (struct f2fs_xattr_arg *)fs_data;
> > >           const struct xattr *xattr;
> > >           int err = 0;
> > >           for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> > >                   err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY,
> > >                                   xattr->name, xattr->value,
> > > -                         xattr->value_len, (struct page *)page, 0);
> > > +                         xattr->value_len, xarg->page, 0, xarg->locked);
> > >                   if (err < 0)
> > >                           break;
> > >           }
> > > @@ -152,10 +153,11 @@ static int f2fs_initxattrs(struct inode *inode, 
> > > const struct xattr *xattr_array,
> > >   }
> > >   int f2fs_init_security(struct inode *inode, struct inode *dir,
> > > -                         const struct qstr *qstr, struct page *ipage)
> > > +                         const struct qstr *qstr,
> > > +                         struct f2fs_xattr_arg *xarg)
> > >   {
> > >           return security_inode_init_security(inode, dir, qstr,
> > > -                         &f2fs_initxattrs, ipage);
> > > +                         &f2fs_initxattrs, xarg);
> > >   }
> > >   #endif
> > > @@ -512,7 +514,8 @@ static inline int write_all_xattrs(struct inode 
> > > *inode, __u32 hsize,
> > >   }
> > >   int f2fs_getxattr(struct inode *inode, int index, const char *name,
> > > -         void *buffer, size_t buffer_size, struct page *ipage)
> > > +                         void *buffer, size_t buffer_size,
> > > +                         struct page *ipage, bool locked)
> > >   {
> > >           struct f2fs_xattr_entry *entry = NULL;
> > >           int error;
> > > @@ -528,12 +531,12 @@ int f2fs_getxattr(struct inode *inode, int index, 
> > > const char *name,
> > >           if (len > F2FS_NAME_LEN)
> > >                   return -ERANGE;
> > > - if (!ipage)
> > > -         f2fs_down_read(&F2FS_I(inode)->i_xattr_sem);
> > > + if (!locked)
> > > +         f2fs_down_read(&F2FS_I(inode)->i_sem);
> > >           error = lookup_all_xattrs(inode, ipage, index, len, name,
> > >                                   &entry, &base_addr, &base_size, 
> > > &is_inline);
> > > - if (!ipage)
> > > -         f2fs_up_read(&F2FS_I(inode)->i_xattr_sem);
> > > + if (!locked)
> > > +         f2fs_up_read(&F2FS_I(inode)->i_sem);
> > >           if (error)
> > >                   return error;
> > > @@ -567,9 +570,9 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char 
> > > *buffer, size_t buffer_size)
> > >           int error;
> > >           size_t rest = buffer_size;
> > > - f2fs_down_read(&F2FS_I(inode)->i_xattr_sem);
> > > + f2fs_down_read(&F2FS_I(inode)->i_sem);
> > >           error = read_all_xattrs(inode, NULL, &base_addr);
> > > - f2fs_up_read(&F2FS_I(inode)->i_xattr_sem);
> > > + f2fs_up_read(&F2FS_I(inode)->i_sem);
> > >           if (error)
> > >                   return error;
> > > @@ -775,7 +778,7 @@ static int __f2fs_setxattr(struct inode *inode, int 
> > > index,
> > >   int f2fs_setxattr(struct inode *inode, int index, const char *name,
> > >                                   const void *value, size_t size,
> > > -                         struct page *ipage, int flags)
> > > +                         struct page *ipage, int flags, bool locked)
> > >   {
> > >           struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > >           int err;
> > > @@ -796,9 +799,11 @@ int f2fs_setxattr(struct inode *inode, int index, 
> > > const char *name,
> > >           f2fs_balance_fs(sbi, true);
> > >           f2fs_lock_op(sbi);
> > > - f2fs_down_write(&F2FS_I(inode)->i_xattr_sem);
> > > + if (!locked)
> > > +         f2fs_down_write(&F2FS_I(inode)->i_sem);
> > >           err = __f2fs_setxattr(inode, index, name, value, size, ipage, 
> > > flags);
> > > - f2fs_up_write(&F2FS_I(inode)->i_xattr_sem);
> > > + if (!locked)
> > > +         f2fs_up_write(&F2FS_I(inode)->i_sem);
> > >           f2fs_unlock_op(sbi);
> > >           f2fs_update_time(sbi, REQ_TIME);
> > > diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> > > index b1811c392e6f..170e4a49af31 100644
> > > --- a/fs/f2fs/xattr.h
> > > +++ b/fs/f2fs/xattr.h
> > > @@ -52,6 +52,11 @@ struct f2fs_xattr_entry {
> > >           char    e_name[];      /* attribute name */
> > >   };
> > > +struct f2fs_xattr_arg {
> > > + struct page *page;      /* inode page */
> > > + bool locked;            /* indicate i_sem is locked */
> > > +};
> > > +
> > >   #define XATTR_HDR(ptr)          ((struct f2fs_xattr_header *)(ptr))
> > >   #define XATTR_ENTRY(ptr)        ((struct f2fs_xattr_entry *)(ptr))
> > >   #define XATTR_FIRST_ENTRY(ptr)  (XATTR_ENTRY(XATTR_HDR(ptr) + 1))
> > > @@ -128,9 +133,9 @@ extern const struct xattr_handler 
> > > f2fs_xattr_security_handler;
> > >   extern const struct xattr_handler *f2fs_xattr_handlers[];
> > >   extern int f2fs_setxattr(struct inode *, int, const char *,
> > > -                         const void *, size_t, struct page *, int);
> > > +                         const void *, size_t, struct page *, int, bool);
> > >   extern int f2fs_getxattr(struct inode *, int, const char *, void *,
> > > -                                         size_t, struct page *);
> > > +                                 size_t, struct page *, bool locked);
> > >   extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
> > >   extern int f2fs_init_xattr_caches(struct f2fs_sb_info *);
> > >   extern void f2fs_destroy_xattr_caches(struct f2fs_sb_info *);
> > > @@ -140,13 +145,13 @@ extern void f2fs_destroy_xattr_caches(struct 
> > > f2fs_sb_info *);
> > >   #define f2fs_listxattr          NULL
> > >   static inline int f2fs_setxattr(struct inode *inode, int index,
> > >                   const char *name, const void *value, size_t size,
> > > -         struct page *page, int flags)
> > > +         struct page *page, int flags, bool locked)
> > >   {
> > >           return -EOPNOTSUPP;
> > >   }
> > >   static inline int f2fs_getxattr(struct inode *inode, int index,
> > >                           const char *name, void *buffer,
> > > -                 size_t buffer_size, struct page *dpage)
> > > +                 size_t buffer_size, struct page *dpage, bool locked)
> > >   {
> > >           return -EOPNOTSUPP;
> > >   }
> > > @@ -156,10 +161,12 @@ static inline void f2fs_destroy_xattr_caches(struct 
> > > f2fs_sb_info *sbi) { }
> > >   #ifdef CONFIG_F2FS_FS_SECURITY
> > >   extern int f2fs_init_security(struct inode *, struct inode *,
> > > -                         const struct qstr *, struct page *);
> > > +                                 const struct qstr *,
> > > +                                 struct f2fs_xattr_arg *xarg);
> > >   #else
> > >   static inline int f2fs_init_security(struct inode *inode, struct inode 
> > > *dir,
> > > -                         const struct qstr *qstr, struct page *ipage)
> > > +                                 const struct qstr *qstr,
> > > +                                 struct f2fs_xattr_arg *xarg)
> > >   {
> > >           return 0;
> > >   }
> > > -- 
> > > 2.40.1


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to