Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: Frank Sorenson <sorenson@redhat.com>
To: linux-cifs@vger.kernel.org, pc@manguebit.org, stfrench@microsoft.com
Subject: [PATCH 1/2] cifs: consolidate time_last_write stamp into _cifsFileInfo_put()
Date: Fri, 24 Jul 2026 11:30:35 -0500	[thread overview]
Message-ID: <20260724163036.2670328-2-sorenson@redhat.com> (raw)
In-Reply-To: <20260724163036.2670328-1-sorenson@redhat.com>

The time_last_write stamp was scattered across cifs_close(),
smb2_deferred_work_close(), and the three drain functions in misc.c.
This missed the case where background I/O holds the final reference
after userspace close() returns, and required explicit maintenance at
each close-path site.

Move the smp_store_release() into _cifsFileInfo_put(), immediately
before releasing open_file_lock.  This single location covers all
close paths unconditionally: normal close, background I/O dropping the
final reference, deferred close via timer or external drain.  The
spinlock's store-release/load-acquire pairing with is_inode_writable()
already provides the ordering guarantee documented in
is_size_safe_to_change().

Remove the now-redundant stamps from cifs_close(),
smb2_deferred_work_close(), and all six stamp sites in the misc.c
deferred-close drain functions.

Fixes: e8a8d54c2d50 ("cifs: prevent readdir from changing file size due to stale directory metadata")
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/file.c | 37 ++++++++++++++------------------
 fs/smb/client/misc.c | 51 ++++++--------------------------------------
 2 files changed, 22 insertions(+), 66 deletions(-)

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index b279a44be729..ac89c1ba56b1 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -915,6 +915,14 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
 		cifs_set_oplock_level(cifsi, 0);
 	}
 
+	if (OPEN_FMODE(cifs_file->f_flags) & FMODE_WRITE) {
+		/* Stamp while open_file_lock is held; covers all close paths
+		 * including background I/O. Pairs with smp_load_acquire() in
+		 * is_size_safe_to_change().
+		 */
+		smp_store_release(&cifsi->time_last_write, jiffies);
+	}
+
 	spin_unlock(&cifsi->open_file_lock);
 	spin_unlock(&tcon->open_file_lock);
 
@@ -1429,15 +1437,6 @@ void smb2_deferred_work_close(struct work_struct *work)
 	cifs_del_deferred_close(cfile);
 	cfile->deferred_close_scheduled = false;
 	spin_unlock(&cinode->deferred_lock);
-	/*
-	 * Refresh time_last_write immediately before the actual server close
-	 * so the protection window is anchored to the real close time, not
-	 * the earlier userspace close time stored by cifs_close().
-	 */
-	if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-		/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-		smp_store_release(&cinode->time_last_write, jiffies);
-	}
 	_cifsFileInfo_put(cfile, true, false);
 }
 
@@ -1467,10 +1466,6 @@ int cifs_close(struct inode *inode, struct file *file)
 	if (file->private_data != NULL) {
 		cfile = file->private_data;
 		file->private_data = NULL;
-		if (file->f_mode & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&cinode->time_last_write, jiffies);
-		}
 		dclose = kmalloc_obj(struct cifs_deferred_close);
 		if ((cfile->status_file_deleted == false) &&
 		    (smb2_can_defer_close(inode, dclose))) {
@@ -3276,13 +3271,13 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
 	 * No writable handles open. Check whether we are within the attribute
 	 * cache validity window of a recent local modification.
 	 *
-	 * For the close() path: cifs_close() calls smp_store_release() on
-	 * time_last_write before _cifsFileInfo_put() removes the handle under
-	 * open_file_lock. That spin_unlock() is a store-release that pairs
-	 * with the spin_lock() (load-acquire) in is_inode_writable() above,
-	 * so if is_inode_writable() returned false the smp_load_acquire()
-	 * below is guaranteed to observe any time_last_write update from a
-	 * concurrent close().
+	 * For the close() path: _cifsFileInfo_put() stamps time_last_write
+	 * (via smp_store_release()) before releasing open_file_lock. That
+	 * spin_unlock() is a store-release that pairs with the spin_lock()
+	 * (load-acquire) in is_inode_writable() above, so if
+	 * is_inode_writable() returned false the smp_load_acquire() below is
+	 * guaranteed to observe any time_last_write update from a concurrent
+	 * close(), covering all close paths including background I/O.
 	 *
 	 * For the setattr/truncate paths: those callers use smp_store_release()
 	 * directly; the smp_load_acquire() below pairs with that store. There
@@ -3297,7 +3292,7 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
 	 * jiffies is still close to INITIAL_JIFFIES on 32-bit systems.
 	 */
 	if (from_readdir) {
-		/* Pairs with smp_store_release() at close and truncate sites. */
+		/* Pairs with smp_store_release() in _cifsFileInfo_put() and setattr. */
 		tlw = smp_load_acquire(&cifsInode->time_last_write);
 		if (tlw && time_before(jiffies, tlw + cifs_sb->ctx->acregmax))
 			return false;
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 6edebc0807ea..46e1382e8e04 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -525,24 +525,11 @@ cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode)
 	}
 	spin_unlock(&cifs_inode->open_file_lock);
 
-	if (failed_cfile) {
-		if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write,
-					  jiffies);
-		}
+	if (failed_cfile)
 		_cifsFileInfo_put(failed_cfile, false, false);
-	}
 
 	list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) {
-		struct cifsFileInfo *cfile = tmp_list->cfile;
-
-		if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write,
-					  jiffies);
-		}
-		_cifsFileInfo_put(cfile, false, false);
+		_cifsFileInfo_put(tmp_list->cfile, false, false);
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
@@ -576,24 +563,11 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon)
 	}
 	spin_unlock(&tcon->open_file_lock);
 
-	if (failed_cfile) {
-		if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write,
-					  jiffies);
-		}
+	if (failed_cfile)
 		_cifsFileInfo_put(failed_cfile, true, false);
-	}
 
 	list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) {
-		struct cifsFileInfo *cfile = tmp_list->cfile;
-
-		if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write,
-					  jiffies);
-		}
-		_cifsFileInfo_put(cfile, true, false);
+		_cifsFileInfo_put(tmp_list->cfile, true, false);
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
@@ -663,24 +637,11 @@ void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon,
 	}
 	spin_unlock(&tcon->open_file_lock);
 
-	if (failed_cfile) {
-		if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write,
-					  jiffies);
-		}
+	if (failed_cfile)
 		_cifsFileInfo_put(failed_cfile, true, false);
-	}
 
 	list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) {
-		struct cifsFileInfo *cfile = tmp_list->cfile;
-
-		if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write,
-					  jiffies);
-		}
-		_cifsFileInfo_put(cfile, true, false);
+		_cifsFileInfo_put(tmp_list->cfile, true, false);
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
-- 
2.55.0


  reply	other threads:[~2026-07-24 16:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24 16:30 [PATCH 0/2] cifs: follow-on fixes to time_last_write mechanism Frank Sorenson
2026-07-24 16:30 ` Frank Sorenson [this message]
2026-07-24 16:30 ` [PATCH 2/2] cifs: fix time_last_write stamp placement in setattr/truncate paths Frank Sorenson
2026-07-27 18:18 ` [PATCH 0/2] cifs: follow-on fixes to time_last_write mechanism Paulo Alcantara
2026-07-28  2:45   ` Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260724163036.2670328-2-sorenson@redhat.com \
    --to=sorenson@redhat.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pc@manguebit.org \
    --cc=stfrench@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox