| Message ID | 20220811063659.67248-1-hyc.lee@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] ksmbd: remove unnecessary generic_fillattr in smb2_open | expand |
On Thu, Aug 11, 2022 at 03:36:59PM +0900, Hyunchul Lee wrote: > + rc = ksmbd_vfs_getattr(&path, &stat); > + if (rc) { > + generic_fillattr(user_ns, d_inode(path.dentry), &stat); > + rc = 0; > + } Why is this calling generic_fillattr at all? generic_fillattr is a helper for file systems. Consumers of the file system API like ksmbd must never use it directly.
2022년 8월 11일 (목) 오후 3:51, Christoph Hellwig <hch@infradead.org>님이 작성: > > On Thu, Aug 11, 2022 at 03:36:59PM +0900, Hyunchul Lee wrote: > > + rc = ksmbd_vfs_getattr(&path, &stat); > > + if (rc) { > > + generic_fillattr(user_ns, d_inode(path.dentry), &stat); > > + rc = 0; > > + } > > Why is this calling generic_fillattr at all? generic_fillattr is a > helper for file systems. Consumers of the file system API like ksmbd > must never use it directly. I will remove generic_fillattr. Thank you for your comment.
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 9751cc92c111..d0a0256334ea 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -3042,12 +3042,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; @@ -3134,6 +3128,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 @@ -3149,9 +3149,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);
Remove unnecessary generic_fillattr to fix wrong AllocationSize of SMB2_CREATE response, And Move the call of ksmbd_vfs_getattr above the place where stat is needed because of truncate. 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. But ext4 returns the blocks that include the delayed allocation blocks when getattr is called. The issue can be reproduced with commands below: touch ${FILENAME} xfs_io -c "pwrite -S 0xAB 0 40k" ${FILENAME} xfs_io -c "stat" ${FILENAME} 40KB are written, but the count of blocks is 8. Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> --- Changes from v1: - Update the commit description. Changes from v2: - Fix the commit description and add the way to reproduce the issue. fs/ksmbd/smb2pdu.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)