All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.