Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: rajasimandalos@gmail.com
To: linux-cifs@vger.kernel.org
Cc: bharathsm@microsoft.com, msetiya@microsoft.com,
	smfrench@gmail.com, sfrench@samba.org, stfrench@microsoft.com,
	Rajasi Mandal <rajasimandal@microsoft.com>
Subject: [PATCH 9/9] smb: client: review fixes for nolease remount (patch 7)
Date: Thu,  2 Jul 2026 11:35:35 +0000	[thread overview]
Message-ID: <20260702113535.4044308-10-rajasimandalos@gmail.com> (raw)
In-Reply-To: <20260702113535.4044308-1-rajasimandalos@gmail.com>

From: Rajasi Mandal <rajasimandal@microsoft.com>

Fold the review feedback for the nolease reconfigure patch into one
change.  Each fix, in two lines:

- nolease transition: track the lease -> nolease transition
  (became_nolease) and only run the expensive cache eviction when nolease
  is actually switched on, not on every bare remount.
- nolease locking: set tcon->no_lease under tc_lock to pair with
  match_tcon(), which reads tcon->no_lease with tc_lock held.
- cached dir leak: add a close_handles arg to invalidate_all_cached_dirs()
  so a live remount keeps is_open and lets the laundromat send SMB2_close
  instead of leaking the server handle/lease.
- callers: cifs_debug.c, file.c and smb2pdu.c pass close_handles=false to
  preserve their existing teardown behaviour.

Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
---
 fs/smb/client/cached_dir.c | 32 ++++++++++++++++++++++----------
 fs/smb/client/cached_dir.h |  3 ++-
 fs/smb/client/cifs_debug.c |  2 +-
 fs/smb/client/file.c       |  2 +-
 fs/smb/client/fs_context.c | 20 +++++++++++++++++---
 fs/smb/client/smb2pdu.c    |  2 +-
 6 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 58d68a8f4d41..420a598f5963 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -598,10 +598,17 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb)
 }
 
 /*
- * Invalidate all cached dirs when a TCON has been reset
- * due to a session loss.
+ * Invalidate all cached dirs on a TCON, moving them to the dying list
+ * for the laundromat to clean up.
+ *
+ * @close_handles: if true, the connection is still live (e.g. remount),
+ * so leave cfid->is_open set and let smb2_close_cached_fid() send
+ * SMB2_close to release the server-side directory handle and lease.  If
+ * false (the session-loss teardown case), mark the cfids closed so the
+ * doomed SMB2_close is skipped -- the server has already dropped them.
  */
-void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync)
+void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync,
+				bool close_handles)
 {
 	struct cached_fids *cfids = tcon->cfids;
 	struct cached_fid *cfid, *q;
@@ -610,15 +617,16 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync)
 		return;
 
 	/*
-	 * Mark all the cfids as closed, and move them to the cfids->dying list.
-	 * They'll be cleaned up by laundromat.  Take a reference to each cfid
-	 * during this process.
+	 * Move all the cfids to the cfids->dying list.  They'll be cleaned
+	 * up by laundromat.  Take a reference to each cfid during this
+	 * process.
 	 */
 	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
 		list_move(&cfid->entry, &cfids->dying);
 		cfids->num_entries--;
-		cfid->is_open = false;
+		if (!close_handles)
+			cfid->is_open = false;
 		cfid->on_list = false;
 		if (cfid->has_lease) {
 			/*
@@ -640,10 +648,14 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync)
 /*
  * Invalidate cached directory entries across all tcons under a
  * superblock.  Collect references on each tcon under tlink_tree_lock,
- * then call invalidate_all_cached_dirs() outside the spinlock since it
- * can sleep.  Holding a tc_count reference prevents the tcon from being
+ * then close their cached dirs outside the spinlock since that can
+ * sleep.  Holding a tc_count reference prevents the tcon from being
  * freed by tlink_expire_delayed() between dropping the spinlock and
  * the call.
+ *
+ * Called on remount while the connection is live (e.g. switching to
+ * nolease), so pass close_handles=true to actually release the
+ * server-side directory handles and their leases.
  */
 void invalidate_all_cached_dirs_sb(struct cifs_sb_info *cifs_sb)
 {
@@ -674,7 +686,7 @@ void invalidate_all_cached_dirs_sb(struct cifs_sb_info *cifs_sb)
 	spin_unlock(&cifs_sb->tlink_tree_lock);
 
 	list_for_each_entry_safe(tmp_list, q, &tcon_head, entry) {
-		invalidate_all_cached_dirs(tmp_list->tcon, true);
+		invalidate_all_cached_dirs(tmp_list->tcon, true, true);
 		list_del(&tmp_list->entry);
 		cifs_put_tcon(tmp_list->tcon, netfs_trace_tcon_ref_put_cached_inval_sb);
 		kfree(tmp_list);
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 606ba2a0b64f..3b86339ca884 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -91,7 +91,8 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
 			     const char *name, struct cifs_sb_info *cifs_sb);
 void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
 void invalidate_all_cached_dirs_sb(struct cifs_sb_info *cifs_sb);
-void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync);
+void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync,
+				bool close_handles);
 bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]);
 
 #endif			/* _CACHED_DIR_H */
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 4ed4f55a0bb7..28b0d433810f 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -383,7 +383,7 @@ static ssize_t cifs_debug_dirs_proc_write(struct file *file, const char __user *
 				if (cifs_ses_exiting(ses))
 					continue;
 				list_for_each_entry(tcon, &ses->tcon_list, tcon_list)
-					invalidate_all_cached_dirs(tcon, false);
+					invalidate_all_cached_dirs(tcon, false, false);
 			}
 		}
 		spin_unlock(&cifs_tcp_ses_lock);
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 58430ba51b10..c06117b1975e 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -393,7 +393,7 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
 	}
 	spin_unlock(&tcon->open_file_lock);
 
-	invalidate_all_cached_dirs(tcon, true);
+	invalidate_all_cached_dirs(tcon, true, false);
 	spin_lock(&tcon->tc_lock);
 	if (tcon->status == TID_IN_FILES_INVALIDATE)
 		tcon->status = TID_NEED_TCON;
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 89bf6f4604b3..70fa869b9b6d 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1333,6 +1333,7 @@ static void smb3_sync_tcon_opts(struct cifs_sb_info *cifs_sb,
 	struct tcon_link *tlink;
 	struct cifs_tcon *tcon;
 	struct rb_node *node;
+	bool became_nolease = false;
 
 	spin_lock(&cifs_sb->tlink_tree_lock);
 	for (node = rb_first(&cifs_sb->tlink_tree); node; node = rb_next(node)) {
@@ -1340,16 +1341,29 @@ static void smb3_sync_tcon_opts(struct cifs_sb_info *cifs_sb,
 		tcon = tlink_tcon(tlink);
 		if (IS_ERR(tcon))
 			continue;
+		/*
+		 * Update under tc_lock to pair with match_tcon(), which reads
+		 * tcon->no_lease with tc_lock held.  Track whether this is the
+		 * lease -> nolease transition so the expensive cleanup below
+		 * only runs when nolease is actually being switched on.
+		 */
+		spin_lock(&tcon->tc_lock);
+		if (ctx->no_lease && !tcon->no_lease)
+			became_nolease = true;
 		tcon->no_lease = ctx->no_lease;
+		spin_unlock(&tcon->tc_lock);
 	}
 	spin_unlock(&cifs_sb->tlink_tree_lock);
 
 	/*
-	 * Both _sb() helpers iterate all tcons internally and handle
-	 * their own locking.  They can sleep, so they must be called
+	 * Only when switching to nolease must we evict lease-bearing cached
+	 * state (deferred handles and cached dir fids).  Skipping this when
+	 * nolease was already set avoids dropping caches on every bare
+	 * remount.  Both _sb() helpers iterate all tcons internally and
+	 * handle their own locking; they can sleep, so they must be called
 	 * outside tlink_tree_lock.
 	 */
-	if (ctx->no_lease) {
+	if (became_nolease) {
 		cifs_close_all_deferred_files_sb(cifs_sb);
 		invalidate_all_cached_dirs_sb(cifs_sb);
 	}
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 958ca0753774..e68f96df8ee3 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2278,7 +2278,7 @@ SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon)
 	}
 	spin_unlock(&ses->chan_lock);
 
-	invalidate_all_cached_dirs(tcon, true);
+	invalidate_all_cached_dirs(tcon, true, false);
 
 	rc = smb2_plain_req_init(SMB2_TREE_DISCONNECT, tcon, server,
 				 (void **) &req,
-- 
2.43.0


      parent reply	other threads:[~2026-07-02 11:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02 11:35 [PATCH RESEND 0/9] smb: client: remount reconfigure option fixes rajasimandalos
2026-07-02 11:35 ` [PATCH 1/9] smb: client: sync runtime state into ctx on reconfigure rajasimandalos
2026-07-02 11:35 ` [PATCH 2/9] smb: client: block non-reconfigurable option changes on remount rajasimandalos
2026-07-02 11:35 ` [PATCH 3/9] smb: client: sync retrans " rajasimandalos
2026-07-02 11:35 ` [PATCH 4/9] smb: client: block cache=ro and cache=singleclient " rajasimandalos
2026-07-02 11:35 ` [PATCH 5/9] smb: client: apply rasize " rajasimandalos
2026-07-02 11:35 ` [PATCH 6/9] smb: client: move struct tcon_list to cifsglob.h rajasimandalos
2026-07-02 11:35 ` [PATCH 7/9] smb: client: allow nolease option to be reconfigured on remount rajasimandalos
2026-07-02 11:35 ` [PATCH 8/9] smb: client: review fixes for remount ctx/retrans sync (patches 1-4) rajasimandalos
2026-07-02 11:35 ` rajasimandalos [this message]

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=20260702113535.4044308-10-rajasimandalos@gmail.com \
    --to=rajasimandalos@gmail.com \
    --cc=bharathsm@microsoft.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=msetiya@microsoft.com \
    --cc=rajasimandal@microsoft.com \
    --cc=sfrench@samba.org \
    --cc=smfrench@gmail.com \
    --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