Re: [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem

LGTM

Huiwen

在 2026/7/31 23:34, Frank Sorenson 写道:
> cifs_setsize() calls fscache_resize_cookie() without holding i_rwsem.
> When the fscache cookie is active (FSCACHE_COOKIE_IS_CACHING is set),
> fscache_resize_cookie() performs a real resize that requires i_rwsem
> held exclusively.  If another file descriptor has the same inode open,
> fscache_use_cookie() was already called from that cifs_open(), making
> the cookie active.  In that case, calling cifs_setsize() from
> cifs_do_truncate() (invoked from cifs_open() without i_rwsem) races
> against concurrent fscache I/O.
> 
> Strip fscache_resize_cookie() from cifs_setsize(), making it a pure
> size/page-cache helper.  Add cifs_resize_file_locked() for callers
> that already hold i_rwsem: it calls netfs_resize_file() and
> cifs_setsize(), then temporarily activates the cookie with
> fscache_use_cookie() to perform the resize under the lock, then
> deactivates it with cifs_fscache_unuse_inode_cookie().  Using
> fscache_use_cookie() before the resize ensures correctness whether or
> not another fd already holds the cookie active.
> 
> Switch cifs_file_set_size(), smb2_duplicate_extents(), and both size-
> extension branches of smb3_simple_falloc() to the new wrapper; those
> paths already hold i_rwsem via VFS setattr, lock_two_nondirectories(),
> or cifs_fallocate() respectively.  cifs_do_truncate() continues to
> call cifs_setsize() followed by cifs_invalidate_cache(), since it runs
> without i_rwsem.
> 
> Fixes: fa724e235cfd ("cifs: add fscache_resize_cookie() to cifs_setsize()")
> Cc: [email protected]
> Cc: David Howells <[email protected]>
> Cc: Paulo Alcantara <[email protected]>
> Signed-off-by: Frank Sorenson <[email protected]>
> Reviewed-by: Paulo Alcantara (Red Hat) <[email protected]>
> ---
>   fs/smb/client/cifsfs.h  |  1 +
>   fs/smb/client/inode.c   | 24 +++++++++++++++++++++---
>   fs/smb/client/smb2ops.c |  9 +++------
>   3 files changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
> index 854e672a4e37..651670c19c2b 100644
> --- a/fs/smb/client/cifsfs.h
> +++ b/fs/smb/client/cifsfs.h
> @@ -147,6 +147,7 @@ ssize_t cifs_file_copychunk_range(unsigned int xid, struct file *src_file,
>   
>   long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg);
>   void cifs_setsize(struct inode *inode, loff_t offset);
> +void cifs_resize_file_locked(struct inode *inode, loff_t offset);
>   
>   struct fs_context;
>   struct smb3_fs_context;
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 0afff761aab9..eaf27a9cf4f5 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -3059,7 +3059,26 @@ void cifs_setsize(struct inode *inode, loff_t offset)
>   	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
>   	truncate_pagecache(inode, offset);
>   	netfs_wait_for_outstanding_io(inode);
> -	fscache_resize_cookie(cifs_inode_cookie(inode), offset);
> +}
> +
> +void cifs_resize_file_locked(struct inode *inode, loff_t offset)
> +{
> +	struct fscache_cookie *cookie = cifs_inode_cookie(inode);
> +
> +	lockdep_assert_held_write(&inode->i_rwsem);
> +
> +	netfs_resize_file(netfs_inode(inode), offset, true);
> +	cifs_setsize(inode, offset);
> +
> +	if (!cookie)
> +		return;
> +
> +	fscache_use_cookie(cookie, true);
> +	fscache_resize_cookie(cookie, offset);
> +	cifs_fscache_unuse_inode_cookie(inode, true);
>   }
>   
>   int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
> @@ -3125,10 +3144,8 @@ int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
>   	cifs_put_tlink(tlink);
>   
>   set_size_out:
> -	if (rc == 0) {
> -		netfs_resize_file(&cifsInode->netfs, size, true);
> -		cifs_setsize(inode, size);
> -	}
> +	if (rc == 0)
> +		cifs_resize_file_locked(inode, size);
>   
>   	return rc;
>   }
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 192649fec25d..0e872d58fae7 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -2222,8 +2222,7 @@ smb2_duplicate_extents(const unsigned int xid,
>   		rc = smb2_set_file_size(xid, tcon, trgtfile, dest_off + len, false);
>   		if (rc)
>   			goto duplicate_extents_out;
> -		netfs_resize_file(netfs_inode(inode), dest_off + len, true);
> -		cifs_setsize(inode, dest_off + len);
> +		cifs_resize_file_locked(inode, dest_off + len);
>   	}
>   	rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>   			trgtfile->fid.volatile_fid,
> @@ -3776,8 +3775,7 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
>   			}
>   
>   			new_eof = off + len;
> -			netfs_resize_file(&cifsi->netfs, new_eof, true);
> -			cifs_setsize(inode, new_eof);
> +			cifs_resize_file_locked(inode, new_eof);
>   
>   			qrc = SMB2_query_info(xid, tcon,
>   					      cfile->fid.persistent_fid,
> @@ -3825,8 +3823,7 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
>   		if (rc)
>   			goto out;
>   
> -		netfs_resize_file(&cifsi->netfs, new_eof, true);
> -		cifs_setsize(inode, new_eof);
> +		cifs_resize_file_locked(inode, new_eof);
>   
>   		qrc = SMB2_query_info(xid, tcon,
>   				      cfile->fid.persistent_fid,