This was the patch that was causing test generic/074 to regress so I took it out of for-next temporarily till we can fix it. The version of the patch I was using is attached On Tue, Jan 23, 2024 at 1:48 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: > > On Tue, Jan 23, 2024 at 12:52 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: > > > > On Sun, Jan 21, 2024 at 9:03 AM <nspmangalore@xxxxxxxxx> wrote: > > > > > > From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx> > > > > > > SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB > > > flag set is already used by the code for SMB3+. > > > smb2_close_getattr is the function that uses this to > > > update the inode attributes. > > > > > > However, we were skipping the EndOfFile info that's returned > > > by the server. There is a small chance that the file size > > > may have been changed in the small window between the client > > > sending the close request (thereby giving up lease if it had) > > > to the point that the server returns the response. > > > > > > This change uses the field to update the inode size. > > > Also, it is a valid case for a zero AllocationSize to be returned > > > by the server for the file. We were discarding such values, thereby > > > resulting in stale i_blocks value. Fixed that here too. > > > > > > Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx> > > > --- > > > fs/smb/client/smb2ops.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c > > > index d9553c2556a2..e23577584ed6 100644 > > > --- a/fs/smb/client/smb2ops.c > > > +++ b/fs/smb/client/smb2ops.c > > > @@ -1433,9 +1433,9 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon, > > > * but instead 512 byte (2**9) size is required for > > > * calculating num blocks. > > > */ > > > - if (le64_to_cpu(file_inf.AllocationSize) > 4096) > > > - inode->i_blocks = > > > - (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9; > > > + inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9; > > > + > > > + inode->i_size = le64_to_cpu(file_inf.EndOfFile); > > > > > > /* End of file and Attributes should not have to be updated on close */ > > > spin_unlock(&inode->i_lock); > > > -- > > > 2.34.1 > > > > > Noticed a couple of things in other places in the code: > > 1. i_size_write() should be called instead of updating i_size directly. > > 2. There's a server_eof field that is generally maintained in sync > > with i_size. Need to check how that needs to be done here. > > > > I'll submit a v2 of this patch soon. > > > > -- > > Regards, > > Shyam > > Attached updated patch. > > -- > Regards, > Shyam -- Thanks, Steve
From 3665f29562a6fefefd36a601d478854a6b672255 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx> Date: Sun, 21 Jan 2024 03:32:44 +0000 Subject: [PATCH 1/6] cifs: smb2_close_getattr should also update i_size SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB flag set is already used by the code for SMB3+. smb2_close_getattr is the function that uses this to update the inode attributes. However, we were skipping the EndOfFile info that's returned by the server. There is a small chance that the file size may have been changed in the small window between the client sending the close request (thereby giving up lease if it had) to the point that the server returns the response. This change uses the field to update the inode size. Also, it is a valid case for a zero AllocationSize to be returned by the server for the file. We were discarding such values, thereby resulting in stale i_blocks value. Fixed that here too. Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx> Cc: stable@xxxxxxxxxxxxxxx Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> --- fs/smb/client/smb2ops.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 27f8caccff7f..eebd2b720945 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -1433,9 +1433,10 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon, * but instead 512 byte (2**9) size is required for * calculating num blocks. */ - if (le64_to_cpu(file_inf.AllocationSize) > 4096) - inode->i_blocks = - (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9; + inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9; + + CIFS_I(inode)->netfs.remote_i_size = le64_to_cpu(file_inf.EndOfFile); + i_size_write(inode, CIFS_I(inode)->netfs.remote_i_size); /* End of file and Attributes should not have to be updated on close */ spin_unlock(&inode->i_lock); -- 2.40.1
