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.
> >
> >> 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
