2022년 8월 11일 (목) 오후 2:46, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>
> 2022-08-11 14:32 GMT+09:00, Hyunchul Lee <hyc.lee@xxxxxxxxx>:
> > 2022년 8월 11일 (목) 오전 8:18, Hyunchul Lee <hyc.lee@xxxxxxxxx>님이 작성:
> >>
> >> 2022년 8월 10일 (수) 오후 5:00, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
> >> >
> >> > 2022-08-10 16:58 GMT+09:00, Namjae Jeon <linkinjeon@xxxxxxxxxx>:
> >> > > 2022-08-10 10:04 GMT+09:00, Hyunchul Lee <hyc.lee@xxxxxxxxx>:
> >> > >> Move the call of ksmbd_vfs_getattr above the place
> >> > >> where stat is needed, and remove unnecessary
> >> > >> the call of generic_fillattr.
> >> > >>
> >> > >> This patch fixes wrong AllocationSize of SMB2_CREATE
> >> > >> response. Because ext4 updates inode->i_blocks only
> >> > >> when disk space is allocated, generic_fillattr does
> >> > >> not set stat.blocks properly for delayed allocation.
> >> > And how can I reproduce problem and make sure that it is improved?
> >>
> >> This issue can be reproduce with commands below:
> >>
> >> touch ${FILENAME}
> >> xfs_io -c "pwrite -S 0xAB 0 40k" ${FILENAME}
> >> xfs_io -c "stat" ${FILENAME}
> >>
> >> 40KB are written, but the number of blocks are 8
> >> stat.size = 40960
> >> stat.blocks = 8
> >>
> >> > > So what causes delay allocation between the lines you moved this
> >> > > code?
> >>
> >> For the example above, the second command writes 40KB but
> >> inode->i_blocks is not updated because of delayed allocation.
> >> And when the third command is executed, in smb2_open
> >> ksmbd_vfs_getattr returns stat.blocks which is 80, but
> >> the following generic_fillattr returns stat.blocks which
> >> 8.
> >>
> >> With this patch applied, the third command returns stat.blocks
> >> which is 88, not 80.
> >> I will look into this.
> >>
> >
> > This is not a problem and happens only when acl_xattr config is enabled.
> > If the config is enabled. The security descriptor is written to xattr
> > as not inline,
> > and the blocks for xattr are counted.
> Okay, The The patch description seems to be wrong.
> You describe that unneeded generic_fillattr call cause this problem,
> not moving the call of ksmbd_vfs_getattr() in patch description. Right
> ?
Right, I will update the description.
> >
> >>
> >>
> >> > >
> >> > >> But ext4 returns the blocks that include the delayed
> >> > >> allocation blocks when getattr is called.
> >> > >>
> >> > >> Signed-off-by: Hyunchul Lee <hyc.lee@xxxxxxxxx>
> >> > >> ---
> >> > >> Changes from v1:
> >> > >> - Update the commit description.
> >> > >>
> >> > >> fs/ksmbd/smb2pdu.c | 15 ++++++---------
> >> > >> 1 file changed, 6 insertions(+), 9 deletions(-)
> >> > >>
> >> > >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> >> > >> index e6f4ccc12f49..7b4bd0d81133 100644
> >> > >> --- a/fs/ksmbd/smb2pdu.c
> >> > >> +++ b/fs/ksmbd/smb2pdu.c
> >> > >> @@ -3022,12 +3022,6 @@ int smb2_open(struct ksmbd_work *work)
> >> > >> list_add(&fp->node, &fp->f_ci->m_fp_list);
> >> > >> write_unlock(&fp->f_ci->m_lock);
> >> > >>
> >> > >> - rc = ksmbd_vfs_getattr(&path, &stat);
> >> > >> - if (rc) {
> >> > >> - generic_fillattr(user_ns, d_inode(path.dentry), &stat);
> >> > >> - rc = 0;
> >> > >> - }
> >> > >> -
> >> > >> /* Check delete pending among previous fp before oplock break
> >> > >> */
> >> > >> if (ksmbd_inode_pending_delete(fp)) {
> >> > >> rc = -EBUSY;
> >> > >> @@ -3114,6 +3108,12 @@ int smb2_open(struct ksmbd_work *work)
> >> > >> }
> >> > >> }
> >> > >>
> >> > >> + rc = ksmbd_vfs_getattr(&path, &stat);
> >> > >> + if (rc) {
> >> > >> + generic_fillattr(user_ns, d_inode(path.dentry), &stat);
> >> > >> + rc = 0;
> >> > >> + }
> >> > >> +
> >> > >> if (stat.result_mask & STATX_BTIME)
> >> > >> fp->create_time = ksmbd_UnixTimeToNT(stat.btime);
> >> > >> else
> >> > >> @@ -3129,9 +3129,6 @@ int smb2_open(struct ksmbd_work *work)
> >> > >>
> >> > >> memcpy(fp->client_guid, conn->ClientGUID,
> >> > >> SMB2_CLIENT_GUID_SIZE);
> >> > >>
> >> > >> - generic_fillattr(user_ns, file_inode(fp->filp),
> >> > >> - &stat);
> >> > >> -
> >> > >> rsp->StructureSize = cpu_to_le16(89);
> >> > >> rcu_read_lock();
> >> > >> opinfo = rcu_dereference(fp->f_opinfo);
> >> > >> --
> >> > >> 2.17.1
> >> > >>
> >> > >>
> >> > >
> >>
> >>
> >>
> >> --
> >> Thanks,
> >> Hyunchul
> >
> >
> >
> > --
> > Thanks,
> > Hyunchul
> >
--
Thanks,
Hyunchul
