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
next prev parent 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