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