Linux CIFS filesystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation
@ 2026-07-31 15:34 Frank Sorenson
  2026-07-31 15:34 ` [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC Frank Sorenson
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Frank Sorenson @ 2026-07-31 15:34 UTC (permalink / raw)
  To: linux-cifs, pc, stfrench, hehuiwen

After commit fa724e235cfd ("cifs: add fscache_resize_cookie() to
cifs_setsize()") consolidated fscache cookie resizing into cifs_setsize(),
two further fixes are needed and two dead-code blocks became removable.

Patch 1 fixes cifs_do_truncate() (O_TRUNC via cifs_open()).  That path
runs without i_rwsem, so it cannot use the locked resize path.
cifs_invalidate_cache() is the correct alternative: it works without the
lock and ensures stale cached data is not served once the cookie is later
activated.

Patch 2 addresses a race identified during review: cifs_setsize() calls
fscache_resize_cookie() without i_rwsem, but another concurrent open may
already have the cookie active (IS_CACHING set), making fscache_resize_cookie()
a real operation requiring the lock.  The fix strips fscache_resize_cookie()
from cifs_setsize() and introduces cifs_resize_file_locked(), which
temporarily activates the cookie with fscache_use_cookie(), performs the
resize under i_rwsem, then deactivates it.  Callers that already hold
i_rwsem (cifs_file_set_size, smb2_duplicate_extents, smb3_simple_falloc)
switch to the wrapper; cifs_do_truncate() continues to use cifs_setsize()
followed by cifs_invalidate_cache() as established in patch 1.

Patches 3 and 4 remove dead code.  Patch 3 removes a caller-side
truncate_setsize() + fscache_resize_cookie() block from
cifs_remap_file_range() that became redundant once smb2_duplicate_extents()
started performing the full size update via cifs_setsize() under the
i_rwsem held by lock_two_nondirectories().  Patch 4 removes equivalent
dead blocks from cifs_setattr_unix() and cifs_setattr_nounix(): since
cifs_file_set_size() calls cifs_setsize() on success, i_size always equals
attrs->ia_size on the success path, making the subsequent size-inequality
blocks unreachable.

v2:
  - Added patch 2 (cifs_resize_file_locked): based on review feedback
    (Huiwen He) that the fscache cookie is not guaranteed to be quiescent
    in cifs_do_truncate() - another concurrent open may already have it
    active.  The fix strips fscache_resize_cookie() from cifs_setsize() and
    adds a locked wrapper for callers that hold i_rwsem.
  - Patch 1 commit message updated: the original rationale (cookie is
    always quiescent in cifs_do_truncate) was incorrect; the correct reason
    is that i_rwsem cannot be held in that path.
  - Patches 2-3 from v1 renumbered to 3-4 to place fixes before cleanups.
  - Patches 3-4: add Reviewed-by from Huiwen He; otherwise unchanged from v1.
  - All patches: add Reviewed-by from Paulo Alcantara.

Frank Sorenson (4):
  cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC
  cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie()
    under i_rwsem
  cifs: remove redundant size-update block in cifs_remap_file_range()
  cifs: remove dead size-update blocks in cifs_setattr_unix/nounix

 fs/smb/client/cifsfs.c  |  6 +-----
 fs/smb/client/cifsfs.h  |  1 +
 fs/smb/client/file.c    |  1 +
 fs/smb/client/inode.c   | 41 ++++++++++++++++++++++-------------------
 fs/smb/client/smb2ops.c |  9 +++------
 5 files changed, 28 insertions(+), 30 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC
  2026-07-31 15:34 [PATCH v2 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation Frank Sorenson
@ 2026-07-31 15:34 ` Frank Sorenson
  2026-07-31 22:37   ` David Howells
  2026-07-31 15:34 ` [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem Frank Sorenson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Frank Sorenson @ 2026-07-31 15:34 UTC (permalink / raw)
  To: linux-cifs, pc, stfrench, hehuiwen; +Cc: stable, David Howells, Paulo Alcantara

cifs_do_truncate() is invoked from cifs_open() without i_rwsem, so it
cannot use cifs_resize_file_locked() to perform a proper fscache cookie
resize.  Instead, add cifs_invalidate_cache() after cifs_setsize().

cifs_invalidate_cache() calls fscache_invalidate(), which works without
holding i_rwsem: it unconditionally increments inval_counter and sets
FSCACHE_COOKIE_NO_DATA_TO_READ, ensuring that stale cached data is not
served once the cookie is later activated by fscache_use_cookie().
Truncation to zero leaves no valid cached data, making invalidation the
correct semantic here.

Fixes: fa724e235cfd ("cifs: add fscache_resize_cookie() to cifs_setsize()")
Cc: stable@vger.kernel.org
Cc: David Howells <dhowells@redhat.com>
Cc: Paulo Alcantara <pc@manguebit.com>
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
---
 fs/smb/client/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index ac89c1ba56b1..389083f9ce00 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -1016,6 +1016,7 @@ static int cifs_do_truncate(const unsigned int xid, struct dentry *dentry)
 		if (!rc) {
 			netfs_resize_file(&cinode->netfs, 0, true);
 			cifs_setsize(inode, 0);
+			cifs_invalidate_cache(inode, 0);
 		}
 	}
 	if (cfile)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem
  2026-07-31 15:34 [PATCH v2 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation Frank Sorenson
  2026-07-31 15:34 ` [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC Frank Sorenson
@ 2026-07-31 15:34 ` Frank Sorenson
  2026-08-02 14:53   ` Huiwen He
  2026-07-31 15:34 ` [PATCH v2 3/4] cifs: remove redundant size-update block in cifs_remap_file_range() Frank Sorenson
  2026-07-31 15:35 ` [PATCH v2 4/4] cifs: remove dead size-update blocks in cifs_setattr_unix/nounix Frank Sorenson
  3 siblings, 1 reply; 9+ messages in thread
From: Frank Sorenson @ 2026-07-31 15:34 UTC (permalink / raw)
  To: linux-cifs, pc, stfrench, hehuiwen; +Cc: stable, David Howells, Paulo Alcantara

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: stable@vger.kernel.org
Cc: David Howells <dhowells@redhat.com>
Cc: Paulo Alcantara <pc@manguebit.com>
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
---
 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,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] cifs: remove redundant size-update block in cifs_remap_file_range()
  2026-07-31 15:34 [PATCH v2 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation Frank Sorenson
  2026-07-31 15:34 ` [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC Frank Sorenson
  2026-07-31 15:34 ` [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem Frank Sorenson
@ 2026-07-31 15:34 ` Frank Sorenson
  2026-07-31 15:35 ` [PATCH v2 4/4] cifs: remove dead size-update blocks in cifs_setattr_unix/nounix Frank Sorenson
  3 siblings, 0 replies; 9+ messages in thread
From: Frank Sorenson @ 2026-07-31 15:34 UTC (permalink / raw)
  To: linux-cifs, pc, stfrench, hehuiwen

cifs_remap_file_range() acquires i_rwsem on both inodes via
lock_two_nondirectories() before calling smb2_duplicate_extents().
cifs_setsize() (called inside smb2_duplicate_extents() when the clone
extends the file) therefore already runs under the lock, meaning the
fscache_resize_cookie() added to cifs_setsize() by commit fa724e235cfd
("cifs: add fscache_resize_cookie() to cifs_setsize()") is correctly
serialised for this path without further changes.

That same commit made the caller-side block:

    if (rc == 0 && new_size > i_size) {
        truncate_setsize(target_inode, new_size);
        fscache_resize_cookie(cifs_inode_cookie(target_inode), new_size);
    }

redundant: smb2_duplicate_extents() already performs the full size update
via cifs_setsize() when the operation extends the file.  Remove the
now-dead block.

Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Reviewed-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
---
 fs/smb/client/cifsfs.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 1788d93a2522..1060ac5f9fff 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1464,11 +1464,7 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,
 	if (target_tcon->ses->server->ops->duplicate_extents) {
 		rc = target_tcon->ses->server->ops->duplicate_extents(xid,
 			smb_file_src, smb_file_target, off, len, destoff);
-		if (rc == 0 && new_size > i_size) {
-			truncate_setsize(target_inode, new_size);
-			fscache_resize_cookie(cifs_inode_cookie(target_inode),
-					      new_size);
-		} else if (rc == -EOPNOTSUPP) {
+		if (rc == -EOPNOTSUPP) {
 			/*
 			 * copy_file_range syscall man page indicates EINVAL
 			 * is returned e.g when "fd_in and fd_out refer to the
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] cifs: remove dead size-update blocks in cifs_setattr_unix/nounix
  2026-07-31 15:34 [PATCH v2 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation Frank Sorenson
                   ` (2 preceding siblings ...)
  2026-07-31 15:34 ` [PATCH v2 3/4] cifs: remove redundant size-update block in cifs_remap_file_range() Frank Sorenson
@ 2026-07-31 15:35 ` Frank Sorenson
  3 siblings, 0 replies; 9+ messages in thread
From: Frank Sorenson @ 2026-07-31 15:35 UTC (permalink / raw)
  To: linux-cifs, pc, stfrench, hehuiwen

Commit 110fee6b9bb5 ("smb: client: fix missing timestamp updates with
O_TRUNC") introduced cifs_file_set_size(), which calls netfs_resize_file()
and cifs_setsize() on success.  cifs_setsize() calls i_size_write(),
updating i_size to the new value.  The subsequent blocks in both
cifs_setattr_unix() and cifs_setattr_nounix():

    if ((attrs->ia_valid & ATTR_SIZE) &&
        attrs->ia_size != i_size_read(inode)) {
        truncate_setsize(inode, attrs->ia_size);
        netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
        fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
    }

are therefore unreachable on the success path: attrs->ia_size ==
i_size_read(inode) always holds after cifs_file_set_size() succeeds.
On the failure path, execution jumps to out/cifs_setattr_exit before
reaching these blocks.

truncate_setsize() and netfs_resize_file() are redundant with what
cifs_file_set_size() already did; fscache_resize_cookie() was moved there
by commit fa724e235cfd ("cifs: add fscache_resize_cookie() to
cifs_setsize()").  Remove both dead blocks.

Fixes: 110fee6b9bb5 ("smb: client: fix missing timestamp updates with O_TRUNC")
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Reviewed-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
---
 fs/smb/client/inode.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index eaf27a9cf4f5..10a3322e89aa 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3306,13 +3306,6 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 	if (rc)
 		goto out;
 
-	if ((attrs->ia_valid & ATTR_SIZE) &&
-	    attrs->ia_size != i_size_read(inode)) {
-		truncate_setsize(inode, attrs->ia_size);
-		netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
-		fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
-	}
-
 	setattr_copy(&nop_mnt_idmap, inode, attrs);
 	mark_inode_dirty(inode);
 
@@ -3518,13 +3511,6 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 	if (rc)
 		goto cifs_setattr_exit;
 
-	if ((attrs->ia_valid & ATTR_SIZE) &&
-	    attrs->ia_size != i_size_read(inode)) {
-		truncate_setsize(inode, attrs->ia_size);
-		netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
-		fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
-	}
-
 	setattr_copy(&nop_mnt_idmap, inode, attrs);
 	mark_inode_dirty(inode);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC
  2026-07-31 15:34 ` [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC Frank Sorenson
@ 2026-07-31 22:37   ` David Howells
  2026-08-02 11:56     ` Frank Sorenson
  0 siblings, 1 reply; 9+ messages in thread
From: David Howells @ 2026-07-31 22:37 UTC (permalink / raw)
  To: Frank Sorenson
  Cc: dhowells, linux-cifs, pc, stfrench, hehuiwen, stable,
	Paulo Alcantara

Frank Sorenson <sorenson@redhat.com> wrote:

> cifs_do_truncate() is invoked from cifs_open() without i_rwsem, so it
> cannot use cifs_resize_file_locked() to perform a proper fscache cookie
> resize.  Instead, add cifs_invalidate_cache() after cifs_setsize().
> 
> cifs_invalidate_cache() calls fscache_invalidate(), which works without
> holding i_rwsem: it unconditionally increments inval_counter and sets
> FSCACHE_COOKIE_NO_DATA_TO_READ, ensuring that stale cached data is not
> served once the cookie is later activated by fscache_use_cookie().
> Truncation to zero leaves no valid cached data, making invalidation the
> correct semantic here.

What happens if there's a concurrent read or write in another thread?
truncate(), buffered read/write and direct read/write() will play reasonably
with each other through a combination of i_rwsem and the stuff in
fs/netfs/locking.c.

But apart from that, I think that invalidating the cache should work.  It may
be slower, but since you're getting rid of all the data anyway...

David


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC
  2026-07-31 22:37   ` David Howells
@ 2026-08-02 11:56     ` Frank Sorenson
  2026-08-02 14:44       ` Huiwen He
  0 siblings, 1 reply; 9+ messages in thread
From: Frank Sorenson @ 2026-08-02 11:56 UTC (permalink / raw)
  To: David Howells; +Cc: linux-cifs, pc, stfrench, hehuiwen, stable, Paulo Alcantara


On 7/31/26 5:37 PM, David Howells wrote:
> Frank Sorenson <sorenson@redhat.com> wrote:
>
>> cifs_do_truncate() is invoked from cifs_open() without i_rwsem, so it
>> cannot use cifs_resize_file_locked() to perform a proper fscache cookie
>> resize.  Instead, add cifs_invalidate_cache() after cifs_setsize().
>>
>> cifs_invalidate_cache() calls fscache_invalidate(), which works without
>> holding i_rwsem: it unconditionally increments inval_counter and sets
>> FSCACHE_COOKIE_NO_DATA_TO_READ, ensuring that stale cached data is not
>> served once the cookie is later activated by fscache_use_cookie().
>> Truncation to zero leaves no valid cached data, making invalidation the
>> correct semantic here.
> What happens if there's a concurrent read or write in another thread?
> truncate(), buffered read/write and direct read/write() will play reasonably
> with each other through a combination of i_rwsem and the stuff in
> fs/netfs/locking.c.
>
> But apart from that, I think that invalidating the cache should work.  It may
> be slower, but since you're getting rid of all the data anyway...
>
> David

The lockless call is pre-existing; this patch just adds the 
fscache_invalidate on top of it and doesn't worsen it, since it's safe 
to call without i_rwsem.  Fixing the concurrent I/O issue properly is 
out of scope for this series.


Frank

-- 
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer, filesystems
Red Hat


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC
  2026-08-02 11:56     ` Frank Sorenson
@ 2026-08-02 14:44       ` Huiwen He
  0 siblings, 0 replies; 9+ messages in thread
From: Huiwen He @ 2026-08-02 14:44 UTC (permalink / raw)
  To: sorenson, David Howells; +Cc: linux-cifs, pc, stfrench, stable, Paulo Alcantara

LGTM. This is safe with or without the inode lock. The pre-existing 
O_TRUNC locking issue can be addressed separately.

Huiwen
在 2026/8/2 19:56, Frank Sorenson 写道:
> 
> On 7/31/26 5:37 PM, David Howells wrote:
>> Frank Sorenson <sorenson@redhat.com> wrote:
>>
>>> cifs_do_truncate() is invoked from cifs_open() without i_rwsem, so it
>>> cannot use cifs_resize_file_locked() to perform a proper fscache cookie
>>> resize.  Instead, add cifs_invalidate_cache() after cifs_setsize().
>>>
>>> cifs_invalidate_cache() calls fscache_invalidate(), which works without
>>> holding i_rwsem: it unconditionally increments inval_counter and sets
>>> FSCACHE_COOKIE_NO_DATA_TO_READ, ensuring that stale cached data is not
>>> served once the cookie is later activated by fscache_use_cookie().
>>> Truncation to zero leaves no valid cached data, making invalidation the
>>> correct semantic here.
>> What happens if there's a concurrent read or write in another thread?
>> truncate(), buffered read/write and direct read/write() will play 
>> reasonably
>> with each other through a combination of i_rwsem and the stuff in
>> fs/netfs/locking.c.
>>
>> But apart from that, I think that invalidating the cache should work.  
>> It may
>> be slower, but since you're getting rid of all the data anyway...
>>
>> David
> 
> The lockless call is pre-existing; this patch just adds the 
> fscache_invalidate on top of it and doesn't worsen it, since it's safe 
> to call without i_rwsem.  Fixing the concurrent I/O issue properly is 
> out of scope for this series.
> 
> 
> Frank
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem
  2026-07-31 15:34 ` [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem Frank Sorenson
@ 2026-08-02 14:53   ` Huiwen He
  0 siblings, 0 replies; 9+ messages in thread
From: Huiwen He @ 2026-08-02 14:53 UTC (permalink / raw)
  To: Frank Sorenson, linux-cifs, pc, stfrench
  Cc: stable, David Howells, Paulo Alcantara

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: stable@vger.kernel.org
> Cc: David Howells <dhowells@redhat.com>
> Cc: Paulo Alcantara <pc@manguebit.com>
> Signed-off-by: Frank Sorenson <sorenson@redhat.com>
> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> ---
>   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,


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-08-02 14:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31 15:34 [PATCH v2 0/4] cifs: follow-on fixes after fscache_resize_cookie() consolidation Frank Sorenson
2026-07-31 15:34 ` [PATCH v2 1/4] cifs: use cifs_invalidate_cache() in cifs_do_truncate() for O_TRUNC Frank Sorenson
2026-07-31 22:37   ` David Howells
2026-08-02 11:56     ` Frank Sorenson
2026-08-02 14:44       ` Huiwen He
2026-07-31 15:34 ` [PATCH v2 2/4] cifs: add cifs_resize_file_locked() to guard fscache_resize_cookie() under i_rwsem Frank Sorenson
2026-08-02 14:53   ` Huiwen He
2026-07-31 15:34 ` [PATCH v2 3/4] cifs: remove redundant size-update block in cifs_remap_file_range() Frank Sorenson
2026-07-31 15:35 ` [PATCH v2 4/4] cifs: remove dead size-update blocks in cifs_setattr_unix/nounix Frank Sorenson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox