* [PATCH] cifs: client: allow changing multichannel mount options on remount
@ 2025-11-18 2:26 Rajasi Mandal
2025-11-18 2:26 ` [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels Rajasi Mandal
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Rajasi Mandal @ 2025-11-18 2:26 UTC (permalink / raw)
To: sfrench, linux-cifs
Cc: pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical,
linux-kernel, Rajasi Mandal
From: Rajasi Mandal <rajasimandal@microsoft.com>
Previously, the client did not update a session's channel state when
multichannel or max_channels mount options were changed via remount.
This led to inconsistent behavior and prevented enabling or disabling
multichannel support without a full unmount/remount cycle.
Enable dynamic reconfiguration of multichannel and max_channels during
remount by:
- Introducing smb3_sync_ses_chan_max(), a centralized function for
channel updates which synchronizes the session's channels with the
updated configuration.
- Replacing cifs_disable_secondary_channels() with
cifs_decrease_secondary_channels(), which accepts a from_reconfigure
flag to support targeted cleanup during reconfiguration.
- Updating remount logic to detect changes in multichannel or
max_channels and trigger appropriate session/channel updates.
Current limitation:
- The query_interfaces worker runs even when max_channels=1 so that
multichannel can be enabled later via remount without requiring an
unmount. This is a temporary approach and may be refined in the
future.
Users can safely modify multichannel and max_channels on an existing
mount. The client will correctly adjust the session's channel state to
match the new configuration, preserving durability where possible and
avoiding unnecessary disconnects.
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
---
fs/smb/client/cifsproto.h | 4 ++-
fs/smb/client/connect.c | 4 ++-
fs/smb/client/fs_context.c | 50 +++++++++++++++++++++++++++++++++-
fs/smb/client/sess.c | 32 +++++++++++++++-------
fs/smb/client/smb2pdu.c | 55 ++++++++++++++++++++++++++++----------
5 files changed, 119 insertions(+), 26 deletions(-)
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 3528c365a452..a1fc9e1918bc 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct shash_desc **sdesc);
void cifs_free_hash(struct shash_desc **sdesc);
int cifs_try_adding_channels(struct cifs_ses *ses);
+int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server,
+ bool from_reconnect, bool from_reconfigure);
bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface);
void cifs_ses_mark_for_reconnect(struct cifs_ses *ses);
@@ -674,7 +676,7 @@ bool
cifs_chan_is_iface_active(struct cifs_ses *ses,
struct TCP_Server_Info *server);
void
-cifs_disable_secondary_channels(struct cifs_ses *ses);
+cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure);
void
cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
int
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 55cb4b0cbd48..cebe4a5f54f2 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
ctx->prepath = NULL;
out:
- cifs_try_adding_channels(mnt_ctx.ses);
+ smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server,
+ false /* from_reconnect */,
+ false /* from_reconfigure */);
rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon);
if (rc)
goto error;
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 0f894d09157b..751ed6ebd458 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
void *data);
static int smb3_get_tree(struct fs_context *fc);
+static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels);
static int smb3_reconfigure(struct fs_context *fc);
static const struct fs_context_operations smb3_fs_context_ops = {
@@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_se
return 0;
}
+/*
+ * smb3_sync_ses_chan_max - Synchronize the session's maximum channel count
+ * @ses: pointer to the old CIFS session structure
+ * @max_channels: new maximum number of channels to allow
+ *
+ * Updates the session's chan_max field to the new value, protecting the update
+ * with the session's channel lock. This should be called whenever the maximum
+ * allowed channels for a session changes (e.g., after a remount or reconfigure).
+ */
+static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels)
+{
+ spin_lock(&ses->chan_lock);
+ ses->chan_max = max_channels;
+ spin_unlock(&ses->chan_lock);
+}
+
static int smb3_reconfigure(struct fs_context *fc)
{
struct smb3_fs_context *ctx = smb3_fc2context(fc);
@@ -1095,7 +1112,38 @@ static int smb3_reconfigure(struct fs_context *fc)
ses->password2 = new_password2;
}
- mutex_unlock(&ses->session_mutex);
+ /*
+ * If multichannel or max_channels has changed, update the session's channels accordingly.
+ * This may add or remove channels to match the new configuration.
+ */
+ if ((ctx->multichannel != cifs_sb->ctx->multichannel) ||
+ (ctx->max_channels != cifs_sb->ctx->max_channels)) {
+
+ /* Synchronize ses->chan_max with the new mount context */
+ smb3_sync_ses_chan_max(ses, ctx->max_channels);
+ /* Now update the session's channels to match the new configuration */
+ /* Prevent concurrent scaling operations */
+ spin_lock(&ses->ses_lock);
+ if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) {
+ spin_unlock(&ses->ses_lock);
+ return -EINVAL;
+ }
+ ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS;
+ spin_unlock(&ses->ses_lock);
+
+ mutex_unlock(&ses->session_mutex);
+
+ rc = smb3_update_ses_channels(ses, ses->server,
+ false /* from_reconnect */,
+ true /* from_reconfigure */);
+
+ /* Clear scaling flag after operation */
+ spin_lock(&ses->ses_lock);
+ ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS;
+ spin_unlock(&ses->ses_lock);
+ } else {
+ mutex_unlock(&ses->session_mutex);
+ }
STEAL_STRING(cifs_sb, ctx, domainname);
STEAL_STRING(cifs_sb, ctx, nodename);
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index ef3b498b0a02..cfd83986a84a 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
}
/*
- * called when multichannel is disabled by the server.
- * this always gets called from smb2_reconnect
- * and cannot get called in parallel threads.
+ * cifs_decrease_secondary_channels - Reduce the number of active secondary channels
+ * @ses: pointer to the CIFS session structure
+ * @from_reconfigure: if true, only reduce to chan_max; if false, reduce to a single channel
+ *
+ * This function disables and cleans up extra secondary channels for a CIFS session.
+ * If called during reconfiguration, it reduces the channel count to the new maximum (chan_max).
+ * Otherwise, it disables all but the primary channel.
*/
void
-cifs_disable_secondary_channels(struct cifs_ses *ses)
+cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure)
{
int i, chan_count;
struct TCP_Server_Info *server;
@@ -281,12 +285,13 @@ cifs_disable_secondary_channels(struct cifs_ses *ses)
if (chan_count == 1)
goto done;
- ses->chan_count = 1;
-
- /* for all secondary channels reset the need reconnect bit */
- ses->chans_need_reconnect &= 1;
+ /* Update the chan_count to the new maximum */
+ if (from_reconfigure)
+ ses->chan_count = ses->chan_max;
+ else
+ ses->chan_count = 1;
- for (i = 1; i < chan_count; i++) {
+ for (i = ses->chan_max ; i < chan_count; i++) {
iface = ses->chans[i].iface;
server = ses->chans[i].server;
@@ -318,6 +323,15 @@ cifs_disable_secondary_channels(struct cifs_ses *ses)
spin_lock(&ses->chan_lock);
}
+ /* For extra secondary channels, reset the need reconnect bit */
+ if (ses->chan_count == 1) {
+ cifs_dbg(VFS, "server does not support multichannel anymore. Disable all other channels\n");
+ ses->chans_need_reconnect &= 1;
+ } else {
+ cifs_dbg(VFS, "Disable extra secondary channels\n");
+ ses->chans_need_reconnect &= ((1UL << ses->chan_max) - 1);
+ }
+
done:
spin_unlock(&ses->chan_lock);
}
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 8b4a4573e9c3..d051da46eaab 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd,
static int
cifs_chan_skip_or_disable(struct cifs_ses *ses,
struct TCP_Server_Info *server,
- bool from_reconnect)
+ bool from_reconnect, bool from_reconfigure)
{
struct TCP_Server_Info *pserver;
unsigned int chan_index;
@@ -206,14 +206,41 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
return -EHOSTDOWN;
}
- cifs_server_dbg(VFS,
- "server does not support multichannel anymore. Disable all other channels\n");
- cifs_disable_secondary_channels(ses);
-
+ cifs_decrease_secondary_channels(ses, from_reconfigure);
return 0;
}
+/*
+ * smb3_update_ses_channels - Synchronize session channels with new configuration
+ * @ses: pointer to the CIFS session structure
+ * @server: pointer to the TCP server info structure
+ * @from_reconnect: indicates if called from reconnect context
+ * @from_reconfigure: indicates if called from reconfigure context
+ *
+ * Returns 0 on success or error code on failure.
+ *
+ * Note: Outside of reconfigure, this function is only called from reconnect scenarios
+ * when the server stops advertising multichannel (MC) capability.
+ */
+int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server,
+ bool from_reconnect, bool from_reconfigure)
+{
+ int rc = 0;
+ /*
+ * If the current channel count is less than the new chan_max,
+ * try to add channels to reach the new maximum.
+ * Otherwise, disable or skip extra channels to match the new configuration.
+ */
+ if (ses->chan_count < ses->chan_max)
+ rc = cifs_try_adding_channels(ses);
+ else
+ rc = cifs_chan_skip_or_disable(ses, server, from_reconnect,
+ from_reconfigure);
+
+ return rc;
+}
+
static int
smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
struct TCP_Server_Info *server, bool from_reconnect)
@@ -355,8 +382,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
*/
if (ses->chan_count > 1 &&
!(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
- rc = cifs_chan_skip_or_disable(ses, server,
- from_reconnect);
+ rc = smb3_update_ses_channels(ses, server,
+ from_reconnect, false /* from_reconfigure */);
if (rc) {
mutex_unlock(&ses->session_mutex);
goto out;
@@ -438,8 +465,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
* treat this as server not supporting multichannel
*/
- rc = cifs_chan_skip_or_disable(ses, server,
- from_reconnect);
+ rc = smb3_update_ses_channels(ses, server,
+ from_reconnect,
+ false /* from_reconfigure */);
goto skip_add_channels;
} else if (rc)
cifs_tcon_dbg(FYI, "%s: failed to query server interfaces: %d\n",
@@ -451,7 +479,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
if (ses->chan_count == 1)
cifs_server_dbg(VFS, "supports multichannel now\n");
- cifs_try_adding_channels(ses);
+ smb3_update_ses_channels(ses, server, from_reconnect,
+ false /* from_reconfigure */);
}
} else {
mutex_unlock(&ses->session_mutex);
@@ -1105,8 +1134,7 @@ SMB2_negotiate(const unsigned int xid,
req->SecurityMode = 0;
req->Capabilities = cpu_to_le32(server->vals->req_capabilities);
- if (ses->chan_max > 1)
- req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
+ req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
/* ClientGUID must be zero for SMB2.02 dialect */
if (server->vals->protocol_id == SMB20_PROT_ID)
@@ -1312,8 +1340,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
pneg_inbuf->Capabilities =
cpu_to_le32(server->vals->req_capabilities);
- if (tcon->ses->chan_max > 1)
- pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
+ pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
memcpy(pneg_inbuf->Guid, server->client_guid,
SMB2_CLIENT_GUID_SIZE);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels 2025-11-18 2:26 [PATCH] cifs: client: allow changing multichannel mount options on remount Rajasi Mandal @ 2025-11-18 2:26 ` Rajasi Mandal 2025-11-18 14:00 ` Enzo Matsumiya 2025-11-20 6:10 ` [PATCH] cifs: client: allow changing multichannel mount options on remount Meetakshi Setiya 2025-12-05 20:11 ` [PATCH v2] " rajasimandalos 2 siblings, 1 reply; 9+ messages in thread From: Rajasi Mandal @ 2025-11-18 2:26 UTC (permalink / raw) To: sfrench, linux-cifs Cc: pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical, linux-kernel, Rajasi Mandal From: Rajasi Mandal <rajasimandal@microsoft.com> Previously, the behavior of the multichannel and max_channels mount options was inconsistent and order-dependent. For example, specifying "multichannel,max_channels=1" would result in 2 channels, while "max_channels=1,multichannel" would result in 1 channel. Additionally, conflicting combinations such as "nomultichannel,max_channels=3" or "multichannel,max_channels=1" did not produce errors and could lead to unexpected channel counts. This commit introduces two new fields in smb3_fs_context to explicitly track whether multichannel and max_channels were specified during mount. The option parsing and validation logic is updated to ensure: - The outcome is no longer dependent on the order of options. - Conflicting combinations (e.g., "nomultichannel,max_channels=3" or "multichannel,max_channels=1") are detected and result in an error. - The number of channels created is consistent with the specified options. This improves the reliability and predictability of mount option handling for SMB3 multichannel support. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> --- fs/smb/client/cifsfs.c | 1 - fs/smb/client/fs_context.c | 65 ++++++++++++++++++++++++++++---------- fs/smb/client/fs_context.h | 2 ++ 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index 185ac41bd7e9..4d53ec53d8db 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -1016,7 +1016,6 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, } else { cifs_info("Attempting to mount %s\n", old_ctx->source); } - cifs_sb = kzalloc(sizeof(*cifs_sb), GFP_KERNEL); if (!cifs_sb) return ERR_PTR(-ENOMEM); diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 46d1eaae62da..1794a31541fe 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -711,6 +711,47 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx) return 0; } +static int smb3_handle_conflicting_options(struct fs_context *fc) +{ + struct smb3_fs_context *ctx = smb3_fc2context(fc); + + if (ctx->multichannel_specified) { + if (ctx->multichannel) { + if (!ctx->max_channels_specified) { + ctx->max_channels = 2; + } else if (ctx->max_channels == 1) { + cifs_errorf(fc, + "max_channels must be greater than 1 when multichannel is enabled\n"); + return -EINVAL; + } + } else { + if (!ctx->max_channels_specified) { + ctx->max_channels = 1; + } else if (ctx->max_channels > 1) { + cifs_errorf(fc, + "max_channels must be equal to 1 when multichannel is disabled\n"); + return -EINVAL; + } + } + } else { + if (ctx->max_channels_specified) { + if (ctx->max_channels > 1) + ctx->multichannel = true; + else + ctx->multichannel = false; + } else { + ctx->multichannel = false; + ctx->max_channels = 1; + } + } + + //resetting default values as remount doesn't initialize fs_context again + ctx->multichannel_specified = false; + ctx->max_channels_specified = false; + + return 0; +} + static void smb3_fs_context_free(struct fs_context *fc); static int smb3_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param); @@ -785,6 +826,7 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc, if (ret < 0) break; } + ret = smb3_handle_conflicting_options(fc); return ret; } @@ -1296,15 +1338,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, ctx->nodelete = 1; break; case Opt_multichannel: - if (result.negated) { + ctx->multichannel_specified = true; + if (result.negated) ctx->multichannel = false; - ctx->max_channels = 1; - } else { + else ctx->multichannel = true; - /* if number of channels not specified, default to 2 */ - if (ctx->max_channels < 2) - ctx->max_channels = 2; - } break; case Opt_uid: ctx->linux_uid = result.uid; @@ -1440,15 +1478,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, ctx->max_credits = result.uint_32; break; case Opt_max_channels: + ctx->max_channels_specified = true; if (result.uint_32 < 1 || result.uint_32 > CIFS_MAX_CHANNELS) { cifs_errorf(fc, "%s: Invalid max_channels value, needs to be 1-%d\n", __func__, CIFS_MAX_CHANNELS); goto cifs_parse_mount_err; } ctx->max_channels = result.uint_32; - /* If more than one channel requested ... they want multichan */ - if (result.uint_32 > 1) - ctx->multichannel = true; break; case Opt_max_cached_dirs: if (result.uint_32 < 1) { @@ -1866,13 +1902,6 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, goto cifs_parse_mount_err; } - /* - * Multichannel is not meaningful if max_channels is 1. - * Force multichannel to false to ensure consistent configuration. - */ - if (ctx->multichannel && ctx->max_channels == 1) - ctx->multichannel = false; - return 0; cifs_parse_mount_err: @@ -1955,6 +1984,8 @@ int smb3_init_fs_context(struct fs_context *fc) /* default to no multichannel (single server connection) */ ctx->multichannel = false; + ctx->multichannel_specified = false; + ctx->max_channels_specified = false; ctx->max_channels = 1; ctx->backupuid_specified = false; /* no backup intent for a user */ diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h index b0fec6b9a23b..7af7cbbe4208 100644 --- a/fs/smb/client/fs_context.h +++ b/fs/smb/client/fs_context.h @@ -294,6 +294,8 @@ struct smb3_fs_context { bool domainauto:1; bool rdma:1; bool multichannel:1; + bool multichannel_specified:1; /* true if user specified multichannel or nomultichannel */ + bool max_channels_specified:1; /* true if user specified max_channels */ bool use_client_guid:1; /* reuse existing guid for multichannel */ u8 client_guid[SMB2_CLIENT_GUID_SIZE]; -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels 2025-11-18 2:26 ` [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels Rajasi Mandal @ 2025-11-18 14:00 ` Enzo Matsumiya 2025-11-20 15:04 ` Steve French 0 siblings, 1 reply; 9+ messages in thread From: Enzo Matsumiya @ 2025-11-18 14:00 UTC (permalink / raw) To: Rajasi Mandal Cc: sfrench, linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical, linux-kernel, Rajasi Mandal On 11/18, Rajasi Mandal wrote: >From: Rajasi Mandal <rajasimandal@microsoft.com> > >Previously, the behavior of the multichannel and max_channels mount >options was inconsistent and order-dependent. For example, specifying >"multichannel,max_channels=1" would result in 2 channels, while >"max_channels=1,multichannel" would result in 1 channel. Additionally, >conflicting combinations such as "nomultichannel,max_channels=3" or >"multichannel,max_channels=1" did not produce errors and could lead to >unexpected channel counts. > >This commit introduces two new fields in smb3_fs_context to explicitly >track whether multichannel and max_channels were specified during >mount. The option parsing and validation logic is updated to ensure: >- The outcome is no longer dependent on the order of options. >- Conflicting combinations (e.g., "nomultichannel,max_channels=3" or > "multichannel,max_channels=1") are detected and result in an error. >- The number of channels created is consistent with the specified > options. > >This improves the reliability and predictability of mount option >handling for SMB3 multichannel support. > >Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> >Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> It's conflicting because it's already too complex for something that should've been simple. This patch introduces a new field + unnecessary logic on top if it all. cf. a PoC patch I sent a while ago, we can (ab)use fsparam with same key name, but different key types, so we could only deal with: 'nomultichannel', 'multichannel={0,1,off,no}' as multichannel disabled 'multichannel' as ctx->max_channels=2 (multichannel enabled, obviously) 'multichannel=X' as ctx->max_channels=X (ditto) Makes 0 sense to have both multichannel and max_channels mount options. >--- > fs/smb/client/cifsfs.c | 1 - > fs/smb/client/fs_context.c | 65 ++++++++++++++++++++++++++++---------- > fs/smb/client/fs_context.h | 2 ++ > 3 files changed, 50 insertions(+), 18 deletions(-) > >diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c >index 185ac41bd7e9..4d53ec53d8db 100644 >--- a/fs/smb/client/cifsfs.c >+++ b/fs/smb/client/cifsfs.c >@@ -1016,7 +1016,6 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, > } else { > cifs_info("Attempting to mount %s\n", old_ctx->source); > } >- > cifs_sb = kzalloc(sizeof(*cifs_sb), GFP_KERNEL); > if (!cifs_sb) > return ERR_PTR(-ENOMEM); >diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c >index 46d1eaae62da..1794a31541fe 100644 >--- a/fs/smb/client/fs_context.c >+++ b/fs/smb/client/fs_context.c >@@ -711,6 +711,47 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx) > return 0; > } > >+static int smb3_handle_conflicting_options(struct fs_context *fc) >+{ >+ struct smb3_fs_context *ctx = smb3_fc2context(fc); >+ >+ if (ctx->multichannel_specified) { >+ if (ctx->multichannel) { >+ if (!ctx->max_channels_specified) { >+ ctx->max_channels = 2; >+ } else if (ctx->max_channels == 1) { >+ cifs_errorf(fc, >+ "max_channels must be greater than 1 when multichannel is enabled\n"); >+ return -EINVAL; >+ } >+ } else { >+ if (!ctx->max_channels_specified) { >+ ctx->max_channels = 1; >+ } else if (ctx->max_channels > 1) { >+ cifs_errorf(fc, >+ "max_channels must be equal to 1 when multichannel is disabled\n"); >+ return -EINVAL; >+ } >+ } >+ } else { >+ if (ctx->max_channels_specified) { >+ if (ctx->max_channels > 1) >+ ctx->multichannel = true; >+ else >+ ctx->multichannel = false; >+ } else { >+ ctx->multichannel = false; >+ ctx->max_channels = 1; >+ } >+ } >+ >+ //resetting default values as remount doesn't initialize fs_context again Please stick to /* ... */ comments style. >+ ctx->multichannel_specified = false; >+ ctx->max_channels_specified = false; >+ >+ return 0; >+} >+ > static void smb3_fs_context_free(struct fs_context *fc); > static int smb3_fs_context_parse_param(struct fs_context *fc, > struct fs_parameter *param); >@@ -785,6 +826,7 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc, > if (ret < 0) > break; > } >+ ret = smb3_handle_conflicting_options(fc); > > return ret; > } >@@ -1296,15 +1338,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > ctx->nodelete = 1; > break; > case Opt_multichannel: >- if (result.negated) { >+ ctx->multichannel_specified = true; >+ if (result.negated) > ctx->multichannel = false; >- ctx->max_channels = 1; >- } else { >+ else > ctx->multichannel = true; >- /* if number of channels not specified, default to 2 */ >- if (ctx->max_channels < 2) >- ctx->max_channels = 2; >- } > break; > case Opt_uid: > ctx->linux_uid = result.uid; >@@ -1440,15 +1478,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > ctx->max_credits = result.uint_32; > break; > case Opt_max_channels: >+ ctx->max_channels_specified = true; > if (result.uint_32 < 1 || result.uint_32 > CIFS_MAX_CHANNELS) { > cifs_errorf(fc, "%s: Invalid max_channels value, needs to be 1-%d\n", > __func__, CIFS_MAX_CHANNELS); > goto cifs_parse_mount_err; > } > ctx->max_channels = result.uint_32; >- /* If more than one channel requested ... they want multichan */ >- if (result.uint_32 > 1) >- ctx->multichannel = true; > break; > case Opt_max_cached_dirs: > if (result.uint_32 < 1) { >@@ -1866,13 +1902,6 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > goto cifs_parse_mount_err; > } > >- /* >- * Multichannel is not meaningful if max_channels is 1. >- * Force multichannel to false to ensure consistent configuration. >- */ >- if (ctx->multichannel && ctx->max_channels == 1) >- ctx->multichannel = false; >- > return 0; > > cifs_parse_mount_err: >@@ -1955,6 +1984,8 @@ int smb3_init_fs_context(struct fs_context *fc) > > /* default to no multichannel (single server connection) */ > ctx->multichannel = false; >+ ctx->multichannel_specified = false; >+ ctx->max_channels_specified = false; > ctx->max_channels = 1; > > ctx->backupuid_specified = false; /* no backup intent for a user */ >diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h >index b0fec6b9a23b..7af7cbbe4208 100644 >--- a/fs/smb/client/fs_context.h >+++ b/fs/smb/client/fs_context.h >@@ -294,6 +294,8 @@ struct smb3_fs_context { > bool domainauto:1; > bool rdma:1; > bool multichannel:1; >+ bool multichannel_specified:1; /* true if user specified multichannel or nomultichannel */ >+ bool max_channels_specified:1; /* true if user specified max_channels */ > bool use_client_guid:1; > /* reuse existing guid for multichannel */ > u8 client_guid[SMB2_CLIENT_GUID_SIZE]; >-- >2.43.0 Cheers, Enzo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels 2025-11-18 14:00 ` Enzo Matsumiya @ 2025-11-20 15:04 ` Steve French 0 siblings, 0 replies; 9+ messages in thread From: Steve French @ 2025-11-20 15:04 UTC (permalink / raw) To: Enzo Matsumiya Cc: Rajasi Mandal, Rajasi Mandal, linux-cifs, sprasad, pc, samba-technical, linux-kernel, sfrench, bharathsm, tom On Tue, Nov 18, 2025 at 8:01 AM Enzo Matsumiya via samba-technical <samba-technical@lists.samba.org> wrote: > > On 11/18, Rajasi Mandal wrote: > >From: Rajasi Mandal <rajasimandal@microsoft.com> > > > >Previously, the behavior of the multichannel and max_channels mount > >options was inconsistent and order-dependent. For example, specifying > >"multichannel,max_channels=1" would result in 2 channels, while > >"max_channels=1,multichannel" would result in 1 channel. Additionally, > >conflicting combinations such as "nomultichannel,max_channels=3" or > >"multichannel,max_channels=1" did not produce errors and could lead to > >unexpected channel counts. > > > >This commit introduces two new fields in smb3_fs_context to explicitly > >track whether multichannel and max_channels were specified during > >mount. The option parsing and validation logic is updated to ensure: > >- The outcome is no longer dependent on the order of options. > >- Conflicting combinations (e.g., "nomultichannel,max_channels=3" or > > "multichannel,max_channels=1") are detected and result in an error. > >- The number of channels created is consistent with the specified > > options. > > > >This improves the reliability and predictability of mount option > >handling for SMB3 multichannel support. > > > >Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> > >Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> > > It's conflicting because it's already too complex for something that > should've been simple. This patch introduces a new field + unnecessary > logic on top if it all. > > cf. a PoC patch I sent a while ago, we can (ab)use fsparam with same key > name, but different key types, so we could only deal with: > > 'nomultichannel', 'multichannel={0,1,off,no}' as multichannel disabled > 'multichannel' as ctx->max_channels=2 (multichannel enabled, obviously) > 'multichannel=X' as ctx->max_channels=X (ditto) > > Makes 0 sense to have both multichannel and max_channels mount options. We can't regress customers who use common mount options without warning them for multiple releases that parm is going to be removed. I don't object to changing Opt_max_channels parsing so ctx->max_channels in fs_context fgoes rom a # to a combination of number and something which can be mapped to on/off (for on client picks default on the fly while for off sets channels to 1) and removing ctx->multichannel - so if you specify "multichannel" it sets ctx->max_channels to something like -1 (or whatever max # is) and if you set nomultichannel it sets ctx->max_channels to 1 -- Thanks, Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cifs: client: allow changing multichannel mount options on remount 2025-11-18 2:26 [PATCH] cifs: client: allow changing multichannel mount options on remount Rajasi Mandal 2025-11-18 2:26 ` [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels Rajasi Mandal @ 2025-11-20 6:10 ` Meetakshi Setiya 2025-11-24 11:49 ` Shyam Prasad N 2025-12-05 20:11 ` [PATCH v2] " rajasimandalos 2 siblings, 1 reply; 9+ messages in thread From: Meetakshi Setiya @ 2025-11-20 6:10 UTC (permalink / raw) To: Rajasi Mandal Cc: sfrench, linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical, linux-kernel, Rajasi Mandal Hi Rajasi, On Tue, Nov 18, 2025 at 7:59 AM Rajasi Mandal <rajasimandalos@gmail.com> wrote: > > From: Rajasi Mandal <rajasimandal@microsoft.com> > > Previously, the client did not update a session's channel state when > multichannel or max_channels mount options were changed via remount. > This led to inconsistent behavior and prevented enabling or disabling > multichannel support without a full unmount/remount cycle. > > Enable dynamic reconfiguration of multichannel and max_channels during > remount by: > - Introducing smb3_sync_ses_chan_max(), a centralized function for > channel updates which synchronizes the session's channels with the > updated configuration. > - Replacing cifs_disable_secondary_channels() with > cifs_decrease_secondary_channels(), which accepts a from_reconfigure > flag to support targeted cleanup during reconfiguration. > - Updating remount logic to detect changes in multichannel or > max_channels and trigger appropriate session/channel updates. > > Current limitation: > - The query_interfaces worker runs even when max_channels=1 so that > multichannel can be enabled later via remount without requiring an > unmount. This is a temporary approach and may be refined in the > future. > > Users can safely modify multichannel and max_channels on an existing > mount. The client will correctly adjust the session's channel state to > match the new configuration, preserving durability where possible and > avoiding unnecessary disconnects. > > Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> > Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> > --- > fs/smb/client/cifsproto.h | 4 ++- > fs/smb/client/connect.c | 4 ++- > fs/smb/client/fs_context.c | 50 +++++++++++++++++++++++++++++++++- > fs/smb/client/sess.c | 32 +++++++++++++++------- > fs/smb/client/smb2pdu.c | 55 ++++++++++++++++++++++++++++---------- > 5 files changed, 119 insertions(+), 26 deletions(-) > > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h > index 3528c365a452..a1fc9e1918bc 100644 > --- a/fs/smb/client/cifsproto.h > +++ b/fs/smb/client/cifsproto.h > @@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct shash_desc **sdesc); > void cifs_free_hash(struct shash_desc **sdesc); > > int cifs_try_adding_channels(struct cifs_ses *ses); > +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, > + bool from_reconnect, bool from_reconfigure); > bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); > void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); > > @@ -674,7 +676,7 @@ bool > cifs_chan_is_iface_active(struct cifs_ses *ses, > struct TCP_Server_Info *server); > void > -cifs_disable_secondary_channels(struct cifs_ses *ses); > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure); > void > cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); > int > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c > index 55cb4b0cbd48..cebe4a5f54f2 100644 > --- a/fs/smb/client/connect.c > +++ b/fs/smb/client/connect.c > @@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) > ctx->prepath = NULL; > > out: > - cifs_try_adding_channels(mnt_ctx.ses); > + smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server, > + false /* from_reconnect */, > + false /* from_reconfigure */); Shouldn't this be added to the non-dfs cifs_mount() too? I see that even cifs_try_adding_channels() is not present in this function, @Shyam Prasad is this expected? > rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); > if (rc) > goto error; > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c > index 0f894d09157b..751ed6ebd458 100644 > --- a/fs/smb/client/fs_context.c > +++ b/fs/smb/client/fs_context.c > @@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > static int smb3_fs_context_parse_monolithic(struct fs_context *fc, > void *data); > static int smb3_get_tree(struct fs_context *fc); > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels); > static int smb3_reconfigure(struct fs_context *fc); > > static const struct fs_context_operations smb3_fs_context_ops = { > @@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_se > return 0; > } > > +/* > + * smb3_sync_ses_chan_max - Synchronize the session's maximum channel count > + * @ses: pointer to the old CIFS session structure > + * @max_channels: new maximum number of channels to allow > + * > + * Updates the session's chan_max field to the new value, protecting the update > + * with the session's channel lock. This should be called whenever the maximum > + * allowed channels for a session changes (e.g., after a remount or reconfigure). > + */ > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels) > +{ > + spin_lock(&ses->chan_lock); > + ses->chan_max = max_channels; > + spin_unlock(&ses->chan_lock); > +} > + > static int smb3_reconfigure(struct fs_context *fc) > { > struct smb3_fs_context *ctx = smb3_fc2context(fc); > @@ -1095,7 +1112,38 @@ static int smb3_reconfigure(struct fs_context *fc) > ses->password2 = new_password2; > } > > - mutex_unlock(&ses->session_mutex); > + /* > + * If multichannel or max_channels has changed, update the session's channels accordingly. > + * This may add or remove channels to match the new configuration. > + */ > + if ((ctx->multichannel != cifs_sb->ctx->multichannel) || > + (ctx->max_channels != cifs_sb->ctx->max_channels)) { > + > + /* Synchronize ses->chan_max with the new mount context */ > + smb3_sync_ses_chan_max(ses, ctx->max_channels); > + /* Now update the session's channels to match the new configuration */ > + /* Prevent concurrent scaling operations */ > + spin_lock(&ses->ses_lock); > + if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) { > + spin_unlock(&ses->ses_lock); > + return -EINVAL; > + } > + ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; > + spin_unlock(&ses->ses_lock); > + > + mutex_unlock(&ses->session_mutex); > + > + rc = smb3_update_ses_channels(ses, ses->server, > + false /* from_reconnect */, > + true /* from_reconfigure */); > + > + /* Clear scaling flag after operation */ > + spin_lock(&ses->ses_lock); > + ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS; > + spin_unlock(&ses->ses_lock); > + } else { > + mutex_unlock(&ses->session_mutex); > + } > > STEAL_STRING(cifs_sb, ctx, domainname); > STEAL_STRING(cifs_sb, ctx, nodename); > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c > index ef3b498b0a02..cfd83986a84a 100644 > --- a/fs/smb/client/sess.c > +++ b/fs/smb/client/sess.c > @@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses *ses) > } > > /* > - * called when multichannel is disabled by the server. > - * this always gets called from smb2_reconnect > - * and cannot get called in parallel threads. > + * cifs_decrease_secondary_channels - Reduce the number of active secondary channels > + * @ses: pointer to the CIFS session structure > + * @from_reconfigure: if true, only reduce to chan_max; if false, reduce to a single channel > + * > + * This function disables and cleans up extra secondary channels for a CIFS session. > + * If called during reconfiguration, it reduces the channel count to the new maximum (chan_max). > + * Otherwise, it disables all but the primary channel. > */ > void > -cifs_disable_secondary_channels(struct cifs_ses *ses) > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure) > { > int i, chan_count; > struct TCP_Server_Info *server; > @@ -281,12 +285,13 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) > if (chan_count == 1) > goto done; > > - ses->chan_count = 1; > - > - /* for all secondary channels reset the need reconnect bit */ > - ses->chans_need_reconnect &= 1; > + /* Update the chan_count to the new maximum */ > + if (from_reconfigure) > + ses->chan_count = ses->chan_max; > + else > + ses->chan_count = 1; > > - for (i = 1; i < chan_count; i++) { > + for (i = ses->chan_max ; i < chan_count; i++) { > iface = ses->chans[i].iface; > server = ses->chans[i].server; > > @@ -318,6 +323,15 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) > spin_lock(&ses->chan_lock); > } > > + /* For extra secondary channels, reset the need reconnect bit */ > + if (ses->chan_count == 1) { > + cifs_dbg(VFS, "server does not support multichannel anymore. Disable all other channels\n"); > + ses->chans_need_reconnect &= 1; > + } else { > + cifs_dbg(VFS, "Disable extra secondary channels\n"); > + ses->chans_need_reconnect &= ((1UL << ses->chan_max) - 1); > + } > + > done: > spin_unlock(&ses->chan_lock); > } > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c > index 8b4a4573e9c3..d051da46eaab 100644 > --- a/fs/smb/client/smb2pdu.c > +++ b/fs/smb/client/smb2pdu.c > @@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd, > static int > cifs_chan_skip_or_disable(struct cifs_ses *ses, > struct TCP_Server_Info *server, > - bool from_reconnect) > + bool from_reconnect, bool from_reconfigure) > { > struct TCP_Server_Info *pserver; > unsigned int chan_index; > @@ -206,14 +206,41 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses, > return -EHOSTDOWN; > } > > - cifs_server_dbg(VFS, > - "server does not support multichannel anymore. Disable all other channels\n"); > - cifs_disable_secondary_channels(ses); > - > + cifs_decrease_secondary_channels(ses, from_reconfigure); > > return 0; > } > > +/* > + * smb3_update_ses_channels - Synchronize session channels with new configuration > + * @ses: pointer to the CIFS session structure > + * @server: pointer to the TCP server info structure > + * @from_reconnect: indicates if called from reconnect context > + * @from_reconfigure: indicates if called from reconfigure context > + * > + * Returns 0 on success or error code on failure. > + * > + * Note: Outside of reconfigure, this function is only called from reconnect scenarios > + * when the server stops advertising multichannel (MC) capability. > + */ I see that function is being called during mount too. Did you mean cifs_decrease_secondary_channels()? > +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, > + bool from_reconnect, bool from_reconfigure) > +{ > + int rc = 0; > + /* > + * If the current channel count is less than the new chan_max, > + * try to add channels to reach the new maximum. > + * Otherwise, disable or skip extra channels to match the new configuration. > + */ > + if (ses->chan_count < ses->chan_max) > + rc = cifs_try_adding_channels(ses); > + else > + rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, > + from_reconfigure); > + > + return rc; > +} > + > static int > smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > struct TCP_Server_Info *server, bool from_reconnect) > @@ -355,8 +382,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > */ > if (ses->chan_count > 1 && > !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { > - rc = cifs_chan_skip_or_disable(ses, server, > - from_reconnect); > + rc = smb3_update_ses_channels(ses, server, > + from_reconnect, false /* from_reconfigure */); > if (rc) { > mutex_unlock(&ses->session_mutex); > goto out; > @@ -438,8 +465,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > * treat this as server not supporting multichannel > */ > > - rc = cifs_chan_skip_or_disable(ses, server, > - from_reconnect); > + rc = smb3_update_ses_channels(ses, server, > + from_reconnect, > + false /* from_reconfigure */); > goto skip_add_channels; > } else if (rc) > cifs_tcon_dbg(FYI, "%s: failed to query server interfaces: %d\n", > @@ -451,7 +479,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > if (ses->chan_count == 1) > cifs_server_dbg(VFS, "supports multichannel now\n"); > > - cifs_try_adding_channels(ses); > + smb3_update_ses_channels(ses, server, from_reconnect, > + false /* from_reconfigure */); > } > } else { > mutex_unlock(&ses->session_mutex); > @@ -1105,8 +1134,7 @@ SMB2_negotiate(const unsigned int xid, > req->SecurityMode = 0; > > req->Capabilities = cpu_to_le32(server->vals->req_capabilities); > - if (ses->chan_max > 1) > - req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > + req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > /* ClientGUID must be zero for SMB2.02 dialect */ > if (server->vals->protocol_id == SMB20_PROT_ID) > @@ -1312,8 +1340,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) > > pneg_inbuf->Capabilities = > cpu_to_le32(server->vals->req_capabilities); > - if (tcon->ses->chan_max > 1) > - pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > + pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > memcpy(pneg_inbuf->Guid, server->client_guid, > SMB2_CLIENT_GUID_SIZE); > -- > 2.43.0 > > Thanks Meetakshi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cifs: client: allow changing multichannel mount options on remount 2025-11-20 6:10 ` [PATCH] cifs: client: allow changing multichannel mount options on remount Meetakshi Setiya @ 2025-11-24 11:49 ` Shyam Prasad N 2025-12-01 9:21 ` RAJASI MANDAL 0 siblings, 1 reply; 9+ messages in thread From: Shyam Prasad N @ 2025-11-24 11:49 UTC (permalink / raw) To: Meetakshi Setiya Cc: Rajasi Mandal, sfrench, linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical, linux-kernel, Rajasi Mandal On Thu, Nov 20, 2025 at 11:40 AM Meetakshi Setiya <meetakshisetiyaoss@gmail.com> wrote: > > Hi Rajasi, > > On Tue, Nov 18, 2025 at 7:59 AM Rajasi Mandal <rajasimandalos@gmail.com> wrote: > > > > From: Rajasi Mandal <rajasimandal@microsoft.com> > > > > Previously, the client did not update a session's channel state when > > multichannel or max_channels mount options were changed via remount. > > This led to inconsistent behavior and prevented enabling or disabling > > multichannel support without a full unmount/remount cycle. > > > > Enable dynamic reconfiguration of multichannel and max_channels during > > remount by: > > - Introducing smb3_sync_ses_chan_max(), a centralized function for > > channel updates which synchronizes the session's channels with the > > updated configuration. > > - Replacing cifs_disable_secondary_channels() with > > cifs_decrease_secondary_channels(), which accepts a from_reconfigure > > flag to support targeted cleanup during reconfiguration. > > - Updating remount logic to detect changes in multichannel or > > max_channels and trigger appropriate session/channel updates. > > > > Current limitation: > > - The query_interfaces worker runs even when max_channels=1 so that > > multichannel can be enabled later via remount without requiring an > > unmount. This is a temporary approach and may be refined in the > > future. > > > > Users can safely modify multichannel and max_channels on an existing > > mount. The client will correctly adjust the session's channel state to > > match the new configuration, preserving durability where possible and > > avoiding unnecessary disconnects. > > > > Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> > > Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> > > --- > > fs/smb/client/cifsproto.h | 4 ++- > > fs/smb/client/connect.c | 4 ++- > > fs/smb/client/fs_context.c | 50 +++++++++++++++++++++++++++++++++- > > fs/smb/client/sess.c | 32 +++++++++++++++------- > > fs/smb/client/smb2pdu.c | 55 ++++++++++++++++++++++++++++---------- > > 5 files changed, 119 insertions(+), 26 deletions(-) > > > > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h > > index 3528c365a452..a1fc9e1918bc 100644 > > --- a/fs/smb/client/cifsproto.h > > +++ b/fs/smb/client/cifsproto.h > > @@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct shash_desc **sdesc); > > void cifs_free_hash(struct shash_desc **sdesc); > > > > int cifs_try_adding_channels(struct cifs_ses *ses); > > +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, > > + bool from_reconnect, bool from_reconfigure); > > bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); > > void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); > > > > @@ -674,7 +676,7 @@ bool > > cifs_chan_is_iface_active(struct cifs_ses *ses, > > struct TCP_Server_Info *server); > > void > > -cifs_disable_secondary_channels(struct cifs_ses *ses); > > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure); > > void > > cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); > > int > > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c > > index 55cb4b0cbd48..cebe4a5f54f2 100644 > > --- a/fs/smb/client/connect.c > > +++ b/fs/smb/client/connect.c > > @@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) > > ctx->prepath = NULL; > > > > out: > > - cifs_try_adding_channels(mnt_ctx.ses); > > + smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server, > > + false /* from_reconnect */, > > + false /* from_reconfigure */); > > Shouldn't this be added to the non-dfs cifs_mount() too? I see that > even cifs_try_adding_channels() is not present in this function, > @Shyam Prasad is this expected? That's a good catch. I think this is a day-0 bug. I can see that the original change by Aurelien has the change only in DFS cifs_mount: commit d70e9fa55884760b6d6c293dbf20d8c52ce11fb7 Author: Aurelien Aptel <aaptel@suse.com> Date: Fri Sep 20 06:31:10 2019 +0200 cifs: try opening channels after mounting We should have a follow-up patch to fix this. > > > rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); > > if (rc) > > goto error; > > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c > > index 0f894d09157b..751ed6ebd458 100644 > > --- a/fs/smb/client/fs_context.c > > +++ b/fs/smb/client/fs_context.c > > @@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > > static int smb3_fs_context_parse_monolithic(struct fs_context *fc, > > void *data); > > static int smb3_get_tree(struct fs_context *fc); > > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels); > > static int smb3_reconfigure(struct fs_context *fc); > > > > static const struct fs_context_operations smb3_fs_context_ops = { > > @@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_se > > return 0; > > } > > > > +/* > > + * smb3_sync_ses_chan_max - Synchronize the session's maximum channel count > > + * @ses: pointer to the old CIFS session structure > > + * @max_channels: new maximum number of channels to allow > > + * > > + * Updates the session's chan_max field to the new value, protecting the update > > + * with the session's channel lock. This should be called whenever the maximum > > + * allowed channels for a session changes (e.g., after a remount or reconfigure). > > + */ > > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels) > > +{ > > + spin_lock(&ses->chan_lock); > > + ses->chan_max = max_channels; > > + spin_unlock(&ses->chan_lock); > > +} > > + > > static int smb3_reconfigure(struct fs_context *fc) > > { > > struct smb3_fs_context *ctx = smb3_fc2context(fc); > > @@ -1095,7 +1112,38 @@ static int smb3_reconfigure(struct fs_context *fc) > > ses->password2 = new_password2; > > } > > > > - mutex_unlock(&ses->session_mutex); > > + /* > > + * If multichannel or max_channels has changed, update the session's channels accordingly. > > + * This may add or remove channels to match the new configuration. > > + */ > > + if ((ctx->multichannel != cifs_sb->ctx->multichannel) || > > + (ctx->max_channels != cifs_sb->ctx->max_channels)) { > > + > > + /* Synchronize ses->chan_max with the new mount context */ > > + smb3_sync_ses_chan_max(ses, ctx->max_channels); > > + /* Now update the session's channels to match the new configuration */ > > + /* Prevent concurrent scaling operations */ > > + spin_lock(&ses->ses_lock); > > + if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) { > > + spin_unlock(&ses->ses_lock); > > + return -EINVAL; > > + } > > + ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; > > + spin_unlock(&ses->ses_lock); > > + > > + mutex_unlock(&ses->session_mutex); > > + > > + rc = smb3_update_ses_channels(ses, ses->server, > > + false /* from_reconnect */, > > + true /* from_reconfigure */); > > + > > + /* Clear scaling flag after operation */ > > + spin_lock(&ses->ses_lock); > > + ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS; > > + spin_unlock(&ses->ses_lock); > > + } else { > > + mutex_unlock(&ses->session_mutex); > > + } > > > > STEAL_STRING(cifs_sb, ctx, domainname); > > STEAL_STRING(cifs_sb, ctx, nodename); > > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c > > index ef3b498b0a02..cfd83986a84a 100644 > > --- a/fs/smb/client/sess.c > > +++ b/fs/smb/client/sess.c > > @@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses *ses) > > } > > > > /* > > - * called when multichannel is disabled by the server. > > - * this always gets called from smb2_reconnect > > - * and cannot get called in parallel threads. > > + * cifs_decrease_secondary_channels - Reduce the number of active secondary channels > > + * @ses: pointer to the CIFS session structure > > + * @from_reconfigure: if true, only reduce to chan_max; if false, reduce to a single channel > > + * > > + * This function disables and cleans up extra secondary channels for a CIFS session. > > + * If called during reconfiguration, it reduces the channel count to the new maximum (chan_max). > > + * Otherwise, it disables all but the primary channel. > > */ > > void > > -cifs_disable_secondary_channels(struct cifs_ses *ses) > > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure) > > { > > int i, chan_count; > > struct TCP_Server_Info *server; > > @@ -281,12 +285,13 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) > > if (chan_count == 1) > > goto done; > > > > - ses->chan_count = 1; > > - > > - /* for all secondary channels reset the need reconnect bit */ > > - ses->chans_need_reconnect &= 1; > > + /* Update the chan_count to the new maximum */ > > + if (from_reconfigure) > > + ses->chan_count = ses->chan_max; > > + else > > + ses->chan_count = 1; > > > > - for (i = 1; i < chan_count; i++) { > > + for (i = ses->chan_max ; i < chan_count; i++) { > > iface = ses->chans[i].iface; > > server = ses->chans[i].server; > > > > @@ -318,6 +323,15 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) > > spin_lock(&ses->chan_lock); > > } > > > > + /* For extra secondary channels, reset the need reconnect bit */ > > + if (ses->chan_count == 1) { > > + cifs_dbg(VFS, "server does not support multichannel anymore. Disable all other channels\n"); > > + ses->chans_need_reconnect &= 1; > > + } else { > > + cifs_dbg(VFS, "Disable extra secondary channels\n"); > > + ses->chans_need_reconnect &= ((1UL << ses->chan_max) - 1); > > + } > > + > > done: > > spin_unlock(&ses->chan_lock); > > } > > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c > > index 8b4a4573e9c3..d051da46eaab 100644 > > --- a/fs/smb/client/smb2pdu.c > > +++ b/fs/smb/client/smb2pdu.c > > @@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd, > > static int > > cifs_chan_skip_or_disable(struct cifs_ses *ses, > > struct TCP_Server_Info *server, > > - bool from_reconnect) > > + bool from_reconnect, bool from_reconfigure) > > { > > struct TCP_Server_Info *pserver; > > unsigned int chan_index; > > @@ -206,14 +206,41 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses, > > return -EHOSTDOWN; > > } > > > > - cifs_server_dbg(VFS, > > - "server does not support multichannel anymore. Disable all other channels\n"); > > - cifs_disable_secondary_channels(ses); > > - > > + cifs_decrease_secondary_channels(ses, from_reconfigure); > > > > return 0; > > } > > > > +/* > > + * smb3_update_ses_channels - Synchronize session channels with new configuration > > + * @ses: pointer to the CIFS session structure > > + * @server: pointer to the TCP server info structure > > + * @from_reconnect: indicates if called from reconnect context > > + * @from_reconfigure: indicates if called from reconfigure context > > + * > > + * Returns 0 on success or error code on failure. > > + * > > + * Note: Outside of reconfigure, this function is only called from reconnect scenarios > > + * when the server stops advertising multichannel (MC) capability. > > + */ > > I see that function is being called during mount too. Did you > mean cifs_decrease_secondary_channels()? > > > +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, > > + bool from_reconnect, bool from_reconfigure) > > +{ > > + int rc = 0; > > + /* > > + * If the current channel count is less than the new chan_max, > > + * try to add channels to reach the new maximum. > > + * Otherwise, disable or skip extra channels to match the new configuration. > > + */ > > + if (ses->chan_count < ses->chan_max) > > + rc = cifs_try_adding_channels(ses); > > + else > > + rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, > > + from_reconfigure); > > + > > + return rc; > > +} > > + > > static int > > smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > > struct TCP_Server_Info *server, bool from_reconnect) > > @@ -355,8 +382,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > > */ > > if (ses->chan_count > 1 && > > !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { > > - rc = cifs_chan_skip_or_disable(ses, server, > > - from_reconnect); > > + rc = smb3_update_ses_channels(ses, server, > > + from_reconnect, false /* from_reconfigure */); > > if (rc) { > > mutex_unlock(&ses->session_mutex); > > goto out; > > @@ -438,8 +465,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > > * treat this as server not supporting multichannel > > */ > > > > - rc = cifs_chan_skip_or_disable(ses, server, > > - from_reconnect); > > + rc = smb3_update_ses_channels(ses, server, > > + from_reconnect, > > + false /* from_reconfigure */); > > goto skip_add_channels; > > } else if (rc) > > cifs_tcon_dbg(FYI, "%s: failed to query server interfaces: %d\n", > > @@ -451,7 +479,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > > if (ses->chan_count == 1) > > cifs_server_dbg(VFS, "supports multichannel now\n"); > > > > - cifs_try_adding_channels(ses); > > + smb3_update_ses_channels(ses, server, from_reconnect, > > + false /* from_reconfigure */); > > } > > } else { > > mutex_unlock(&ses->session_mutex); > > @@ -1105,8 +1134,7 @@ SMB2_negotiate(const unsigned int xid, > > req->SecurityMode = 0; > > > > req->Capabilities = cpu_to_le32(server->vals->req_capabilities); > > - if (ses->chan_max > 1) > > - req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > + req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > > > /* ClientGUID must be zero for SMB2.02 dialect */ > > if (server->vals->protocol_id == SMB20_PROT_ID) > > @@ -1312,8 +1340,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) > > > > pneg_inbuf->Capabilities = > > cpu_to_le32(server->vals->req_capabilities); > > - if (tcon->ses->chan_max > 1) > > - pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > + pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > > > memcpy(pneg_inbuf->Guid, server->client_guid, > > SMB2_CLIENT_GUID_SIZE); > > -- > > 2.43.0 > > > > > > Thanks > Meetakshi > -- Regards, Shyam ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cifs: client: allow changing multichannel mount options on remount 2025-11-24 11:49 ` Shyam Prasad N @ 2025-12-01 9:21 ` RAJASI MANDAL 2025-12-01 18:35 ` Steve French 0 siblings, 1 reply; 9+ messages in thread From: RAJASI MANDAL @ 2025-12-01 9:21 UTC (permalink / raw) To: Shyam Prasad N Cc: Meetakshi Setiya, sfrench, linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical, linux-kernel, Rajasi Mandal [-- Attachment #1.1: Type: text/plain, Size: 17904 bytes --] Sending a follow-up patch with corrected comment as pointed out by Meetakshi. Attaching the new patch here for easy reference. Regards, Rajasi On Mon, Nov 24, 2025 at 5:20 PM Shyam Prasad N <nspmangalore@gmail.com> wrote: > On Thu, Nov 20, 2025 at 11:40 AM Meetakshi Setiya > <meetakshisetiyaoss@gmail.com> wrote: > > > > Hi Rajasi, > > > > On Tue, Nov 18, 2025 at 7:59 AM Rajasi Mandal <rajasimandalos@gmail.com> > wrote: > > > > > > From: Rajasi Mandal <rajasimandal@microsoft.com> > > > > > > Previously, the client did not update a session's channel state when > > > multichannel or max_channels mount options were changed via remount. > > > This led to inconsistent behavior and prevented enabling or disabling > > > multichannel support without a full unmount/remount cycle. > > > > > > Enable dynamic reconfiguration of multichannel and max_channels during > > > remount by: > > > - Introducing smb3_sync_ses_chan_max(), a centralized function for > > > channel updates which synchronizes the session's channels with the > > > updated configuration. > > > - Replacing cifs_disable_secondary_channels() with > > > cifs_decrease_secondary_channels(), which accepts a from_reconfigure > > > flag to support targeted cleanup during reconfiguration. > > > - Updating remount logic to detect changes in multichannel or > > > max_channels and trigger appropriate session/channel updates. > > > > > > Current limitation: > > > - The query_interfaces worker runs even when max_channels=1 so that > > > multichannel can be enabled later via remount without requiring an > > > unmount. This is a temporary approach and may be refined in the > > > future. > > > > > > Users can safely modify multichannel and max_channels on an existing > > > mount. The client will correctly adjust the session's channel state to > > > match the new configuration, preserving durability where possible and > > > avoiding unnecessary disconnects. > > > > > > Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> > > > Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> > > > --- > > > fs/smb/client/cifsproto.h | 4 ++- > > > fs/smb/client/connect.c | 4 ++- > > > fs/smb/client/fs_context.c | 50 +++++++++++++++++++++++++++++++++- > > > fs/smb/client/sess.c | 32 +++++++++++++++------- > > > fs/smb/client/smb2pdu.c | 55 ++++++++++++++++++++++++++++---------- > > > 5 files changed, 119 insertions(+), 26 deletions(-) > > > > > > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h > > > index 3528c365a452..a1fc9e1918bc 100644 > > > --- a/fs/smb/client/cifsproto.h > > > +++ b/fs/smb/client/cifsproto.h > > > @@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct > shash_desc **sdesc); > > > void cifs_free_hash(struct shash_desc **sdesc); > > > > > > int cifs_try_adding_channels(struct cifs_ses *ses); > > > +int smb3_update_ses_channels(struct cifs_ses *ses, struct > TCP_Server_Info *server, > > > + bool from_reconnect, bool > from_reconfigure); > > > bool is_ses_using_iface(struct cifs_ses *ses, struct > cifs_server_iface *iface); > > > void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); > > > > > > @@ -674,7 +676,7 @@ bool > > > cifs_chan_is_iface_active(struct cifs_ses *ses, > > > struct TCP_Server_Info *server); > > > void > > > -cifs_disable_secondary_channels(struct cifs_ses *ses); > > > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool > from_reconfigure); > > > void > > > cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info > *server); > > > int > > > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c > > > index 55cb4b0cbd48..cebe4a5f54f2 100644 > > > --- a/fs/smb/client/connect.c > > > +++ b/fs/smb/client/connect.c > > > @@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, > struct smb3_fs_context *ctx) > > > ctx->prepath = NULL; > > > > > > out: > > > - cifs_try_adding_channels(mnt_ctx.ses); > > > + smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server, > > > + false /* from_reconnect */, > > > + false /* from_reconfigure */); > > > > Shouldn't this be added to the non-dfs cifs_mount() too? I see that > > even cifs_try_adding_channels() is not present in this function, > > @Shyam Prasad is this expected? > > That's a good catch. > I think this is a day-0 bug. > I can see that the original change by Aurelien has the change only in > DFS cifs_mount: > commit d70e9fa55884760b6d6c293dbf20d8c52ce11fb7 > Author: Aurelien Aptel <aaptel@suse.com> > Date: Fri Sep 20 06:31:10 2019 +0200 > > cifs: try opening channels after mounting > > We should have a follow-up patch to fix this. > > > > > > rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); > > > if (rc) > > > goto error; > > > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c > > > index 0f894d09157b..751ed6ebd458 100644 > > > --- a/fs/smb/client/fs_context.c > > > +++ b/fs/smb/client/fs_context.c > > > @@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct > fs_context *fc, > > > static int smb3_fs_context_parse_monolithic(struct fs_context *fc, > > > void *data); > > > static int smb3_get_tree(struct fs_context *fc); > > > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int > max_channels); > > > static int smb3_reconfigure(struct fs_context *fc); > > > > > > static const struct fs_context_operations smb3_fs_context_ops = { > > > @@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct > cifs_sb_info *cifs_sb, struct cifs_se > > > return 0; > > > } > > > > > > +/* > > > + * smb3_sync_ses_chan_max - Synchronize the session's maximum channel > count > > > + * @ses: pointer to the old CIFS session structure > > > + * @max_channels: new maximum number of channels to allow > > > + * > > > + * Updates the session's chan_max field to the new value, protecting > the update > > > + * with the session's channel lock. This should be called whenever > the maximum > > > + * allowed channels for a session changes (e.g., after a remount or > reconfigure). > > > + */ > > > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int > max_channels) > > > +{ > > > + spin_lock(&ses->chan_lock); > > > + ses->chan_max = max_channels; > > > + spin_unlock(&ses->chan_lock); > > > +} > > > + > > > static int smb3_reconfigure(struct fs_context *fc) > > > { > > > struct smb3_fs_context *ctx = smb3_fc2context(fc); > > > @@ -1095,7 +1112,38 @@ static int smb3_reconfigure(struct fs_context > *fc) > > > ses->password2 = new_password2; > > > } > > > > > > - mutex_unlock(&ses->session_mutex); > > > + /* > > > + * If multichannel or max_channels has changed, update the > session's channels accordingly. > > > + * This may add or remove channels to match the new > configuration. > > > + */ > > > + if ((ctx->multichannel != cifs_sb->ctx->multichannel) || > > > + (ctx->max_channels != cifs_sb->ctx->max_channels)) { > > > + > > > + /* Synchronize ses->chan_max with the new mount > context */ > > > + smb3_sync_ses_chan_max(ses, ctx->max_channels); > > > + /* Now update the session's channels to match the new > configuration */ > > > + /* Prevent concurrent scaling operations */ > > > + spin_lock(&ses->ses_lock); > > > + if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) { > > > + spin_unlock(&ses->ses_lock); > > > + return -EINVAL; > > > + } > > > + ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; > > > + spin_unlock(&ses->ses_lock); > > > + > > > + mutex_unlock(&ses->session_mutex); > > > + > > > + rc = smb3_update_ses_channels(ses, ses->server, > > > + false /* from_reconnect > */, > > > + true /* > from_reconfigure */); > > > + > > > + /* Clear scaling flag after operation */ > > > + spin_lock(&ses->ses_lock); > > > + ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS; > > > + spin_unlock(&ses->ses_lock); > > > + } else { > > > + mutex_unlock(&ses->session_mutex); > > > + } > > > > > > STEAL_STRING(cifs_sb, ctx, domainname); > > > STEAL_STRING(cifs_sb, ctx, nodename); > > > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c > > > index ef3b498b0a02..cfd83986a84a 100644 > > > --- a/fs/smb/client/sess.c > > > +++ b/fs/smb/client/sess.c > > > @@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses > *ses) > > > } > > > > > > /* > > > - * called when multichannel is disabled by the server. > > > - * this always gets called from smb2_reconnect > > > - * and cannot get called in parallel threads. > > > + * cifs_decrease_secondary_channels - Reduce the number of active > secondary channels > > > + * @ses: pointer to the CIFS session structure > > > + * @from_reconfigure: if true, only reduce to chan_max; if false, > reduce to a single channel > > > + * > > > + * This function disables and cleans up extra secondary channels for > a CIFS session. > > > + * If called during reconfiguration, it reduces the channel count to > the new maximum (chan_max). > > > + * Otherwise, it disables all but the primary channel. > > > */ > > > void > > > -cifs_disable_secondary_channels(struct cifs_ses *ses) > > > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool > from_reconfigure) > > > { > > > int i, chan_count; > > > struct TCP_Server_Info *server; > > > @@ -281,12 +285,13 @@ cifs_disable_secondary_channels(struct cifs_ses > *ses) > > > if (chan_count == 1) > > > goto done; > > > > > > - ses->chan_count = 1; > > > - > > > - /* for all secondary channels reset the need reconnect bit */ > > > - ses->chans_need_reconnect &= 1; > > > + /* Update the chan_count to the new maximum */ > > > + if (from_reconfigure) > > > + ses->chan_count = ses->chan_max; > > > + else > > > + ses->chan_count = 1; > > > > > > - for (i = 1; i < chan_count; i++) { > > > + for (i = ses->chan_max ; i < chan_count; i++) { > > > iface = ses->chans[i].iface; > > > server = ses->chans[i].server; > > > > > > @@ -318,6 +323,15 @@ cifs_disable_secondary_channels(struct cifs_ses > *ses) > > > spin_lock(&ses->chan_lock); > > > } > > > > > > + /* For extra secondary channels, reset the need reconnect bit > */ > > > + if (ses->chan_count == 1) { > > > + cifs_dbg(VFS, "server does not support multichannel > anymore. Disable all other channels\n"); > > > + ses->chans_need_reconnect &= 1; > > > + } else { > > > + cifs_dbg(VFS, "Disable extra secondary channels\n"); > > > + ses->chans_need_reconnect &= ((1UL << ses->chan_max) - > 1); > > > + } > > > + > > > done: > > > spin_unlock(&ses->chan_lock); > > > } > > > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c > > > index 8b4a4573e9c3..d051da46eaab 100644 > > > --- a/fs/smb/client/smb2pdu.c > > > +++ b/fs/smb/client/smb2pdu.c > > > @@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 > smb2_cmd, > > > static int > > > cifs_chan_skip_or_disable(struct cifs_ses *ses, > > > struct TCP_Server_Info *server, > > > - bool from_reconnect) > > > + bool from_reconnect, bool from_reconfigure) > > > { > > > struct TCP_Server_Info *pserver; > > > unsigned int chan_index; > > > @@ -206,14 +206,41 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses, > > > return -EHOSTDOWN; > > > } > > > > > > - cifs_server_dbg(VFS, > > > - "server does not support multichannel anymore. Disable > all other channels\n"); > > > - cifs_disable_secondary_channels(ses); > > > - > > > + cifs_decrease_secondary_channels(ses, from_reconfigure); > > > > > > return 0; > > > } > > > > > > +/* > > > + * smb3_update_ses_channels - Synchronize session channels with new > configuration > > > + * @ses: pointer to the CIFS session structure > > > + * @server: pointer to the TCP server info structure > > > + * @from_reconnect: indicates if called from reconnect context > > > + * @from_reconfigure: indicates if called from reconfigure context > > > + * > > > + * Returns 0 on success or error code on failure. > > > + * > > > + * Note: Outside of reconfigure, this function is only called from > reconnect scenarios > > > + * when the server stops advertising multichannel (MC) capability. > > > + */ > > > > I see that function is being called during mount too. Did you > > mean cifs_decrease_secondary_channels()? > > > > > +int smb3_update_ses_channels(struct cifs_ses *ses, struct > TCP_Server_Info *server, > > > + bool from_reconnect, bool from_reconfigure) > > > +{ > > > + int rc = 0; > > > + /* > > > + * If the current channel count is less than the new chan_max, > > > + * try to add channels to reach the new maximum. > > > + * Otherwise, disable or skip extra channels to match the new > configuration. > > > + */ > > > + if (ses->chan_count < ses->chan_max) > > > + rc = cifs_try_adding_channels(ses); > > > + else > > > + rc = cifs_chan_skip_or_disable(ses, server, > from_reconnect, > > > + from_reconfigure); > > > + > > > + return rc; > > > +} > > > + > > > static int > > > smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, > > > struct TCP_Server_Info *server, bool from_reconnect) > > > @@ -355,8 +382,8 @@ smb2_reconnect(__le16 smb2_command, struct > cifs_tcon *tcon, > > > */ > > > if (ses->chan_count > 1 && > > > !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { > > > - rc = cifs_chan_skip_or_disable(ses, server, > > > - from_reconnect); > > > + rc = smb3_update_ses_channels(ses, server, > > > + from_reconnect, false > /* from_reconfigure */); > > > if (rc) { > > > mutex_unlock(&ses->session_mutex); > > > goto out; > > > @@ -438,8 +465,9 @@ smb2_reconnect(__le16 smb2_command, struct > cifs_tcon *tcon, > > > * treat this as server not supporting > multichannel > > > */ > > > > > > - rc = cifs_chan_skip_or_disable(ses, server, > > > - from_reconnect); > > > + rc = smb3_update_ses_channels(ses, server, > > > + from_reconnect, > > > + false /* > from_reconfigure */); > > > goto skip_add_channels; > > > } else if (rc) > > > cifs_tcon_dbg(FYI, "%s: failed to query server > interfaces: %d\n", > > > @@ -451,7 +479,8 @@ smb2_reconnect(__le16 smb2_command, struct > cifs_tcon *tcon, > > > if (ses->chan_count == 1) > > > cifs_server_dbg(VFS, "supports > multichannel now\n"); > > > > > > - cifs_try_adding_channels(ses); > > > + smb3_update_ses_channels(ses, server, > from_reconnect, > > > + false /* > from_reconfigure */); > > > } > > > } else { > > > mutex_unlock(&ses->session_mutex); > > > @@ -1105,8 +1134,7 @@ SMB2_negotiate(const unsigned int xid, > > > req->SecurityMode = 0; > > > > > > req->Capabilities = > cpu_to_le32(server->vals->req_capabilities); > > > - if (ses->chan_max > 1) > > > - req->Capabilities |= > cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > > + req->Capabilities |= > cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > > > > > /* ClientGUID must be zero for SMB2.02 dialect */ > > > if (server->vals->protocol_id == SMB20_PROT_ID) > > > @@ -1312,8 +1340,7 @@ int smb3_validate_negotiate(const unsigned int > xid, struct cifs_tcon *tcon) > > > > > > pneg_inbuf->Capabilities = > > > cpu_to_le32(server->vals->req_capabilities); > > > - if (tcon->ses->chan_max > 1) > > > - pneg_inbuf->Capabilities |= > cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > > + pneg_inbuf->Capabilities |= > cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); > > > > > > memcpy(pneg_inbuf->Guid, server->client_guid, > > > SMB2_CLIENT_GUID_SIZE); > > > -- > > > 2.43.0 > > > > > > > > > > Thanks > > Meetakshi > > > > > -- > Regards, > Shyam > [-- Attachment #1.2: Type: text/html, Size: 22897 bytes --] [-- Attachment #2: [PATCH] cifs_ client_ allow changing multichannel mount options on remount (1).eml --] [-- Type: message/rfc822, Size: 13344 bytes --] From: rajasimandalos@gmail.com To: linux-cifs@vger.kernel.org Cc: sfrench@samba.org, pc@manguebit.org, ronniesahlberg@gmail.com, sprasad@microsoft.com, tom@talpey.com, bharathsm@microsoft.com, samba-technical@lists.samba.org, Rajasi Mandal <rajasimandal@microsoft.com> Subject: [PATCH] cifs: client: allow changing multichannel mount options on remount Date: Mon, 1 Dec 2025 06:25:17 +0000 Message-ID: <20251201062517.1513683-1-rajasimandalos@gmail.com> From: Rajasi Mandal <rajasimandal@microsoft.com> Previously, the client did not update a session's channel state when multichannel or max_channels mount options were changed via remount. This led to inconsistent behavior and prevented enabling or disabling multichannel support without a full unmount/remount cycle. Enable dynamic reconfiguration of multichannel and max_channels during remount by: - Introducing smb3_sync_ses_chan_max(), a centralized function for channel updates which synchronizes the session's channels with the updated configuration. - Replacing cifs_disable_secondary_channels() with cifs_decrease_secondary_channels(), which accepts a from_reconfigure flag to support targeted cleanup during reconfiguration. - Updating remount logic to detect changes in multichannel or max_channels and trigger appropriate session/channel updates. Current limitation: - The query_interfaces worker runs even when max_channels=1 so that multichannel can be enabled later via remount without requiring an unmount. This is a temporary approach and may be refined in the future. Users can safely modify multichannel and max_channels on an existing mount. The client will correctly adjust the session's channel state to match the new configuration, preserving durability where possible and avoiding unnecessary disconnects. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> --- fs/smb/client/cifsproto.h | 4 ++- fs/smb/client/connect.c | 4 ++- fs/smb/client/fs_context.c | 50 +++++++++++++++++++++++++++++++++- fs/smb/client/sess.c | 32 ++++++++++++++++------ fs/smb/client/smb2pdu.c | 56 ++++++++++++++++++++++++++++---------- 5 files changed, 120 insertions(+), 26 deletions(-) diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 3528c365a452..a1fc9e1918bc 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct shash_desc **sdesc); void cifs_free_hash(struct shash_desc **sdesc); int cifs_try_adding_channels(struct cifs_ses *ses); +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, + bool from_reconnect, bool from_reconfigure); bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); @@ -674,7 +676,7 @@ bool cifs_chan_is_iface_active(struct cifs_ses *ses, struct TCP_Server_Info *server); void -cifs_disable_secondary_channels(struct cifs_ses *ses); +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure); void cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); int diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 2f94d93b95e9..6557b3546398 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) ctx->prepath = NULL; out: - cifs_try_adding_channels(mnt_ctx.ses); + smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server, + false /* from_reconnect */, + false /* from_reconfigure */); rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); if (rc) goto error; diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 2a0d8b87bd8e..46428181699e 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, static int smb3_fs_context_parse_monolithic(struct fs_context *fc, void *data); static int smb3_get_tree(struct fs_context *fc); +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels); static int smb3_reconfigure(struct fs_context *fc); static const struct fs_context_operations smb3_fs_context_ops = { @@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_se return 0; } +/* + * smb3_sync_ses_chan_max - Synchronize the session's maximum channel count + * @ses: pointer to the old CIFS session structure + * @max_channels: new maximum number of channels to allow + * + * Updates the session's chan_max field to the new value, protecting the update + * with the session's channel lock. This should be called whenever the maximum + * allowed channels for a session changes (e.g., after a remount or reconfigure). + */ +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels) +{ + spin_lock(&ses->chan_lock); + ses->chan_max = max_channels; + spin_unlock(&ses->chan_lock); +} + static int smb3_reconfigure(struct fs_context *fc) { struct smb3_fs_context *ctx = smb3_fc2context(fc); @@ -1095,7 +1112,38 @@ static int smb3_reconfigure(struct fs_context *fc) ses->password2 = new_password2; } - mutex_unlock(&ses->session_mutex); + /* + * If multichannel or max_channels has changed, update the session's channels accordingly. + * This may add or remove channels to match the new configuration. + */ + if ((ctx->multichannel != cifs_sb->ctx->multichannel) || + (ctx->max_channels != cifs_sb->ctx->max_channels)) { + + /* Synchronize ses->chan_max with the new mount context */ + smb3_sync_ses_chan_max(ses, ctx->max_channels); + /* Now update the session's channels to match the new configuration */ + /* Prevent concurrent scaling operations */ + spin_lock(&ses->ses_lock); + if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) { + spin_unlock(&ses->ses_lock); + return -EINVAL; + } + ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; + spin_unlock(&ses->ses_lock); + + mutex_unlock(&ses->session_mutex); + + rc = smb3_update_ses_channels(ses, ses->server, + false /* from_reconnect */, + true /* from_reconfigure */); + + /* Clear scaling flag after operation */ + spin_lock(&ses->ses_lock); + ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS; + spin_unlock(&ses->ses_lock); + } else { + mutex_unlock(&ses->session_mutex); + } STEAL_STRING(cifs_sb, ctx, domainname); STEAL_STRING(cifs_sb, ctx, nodename); diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index ef3b498b0a02..cfd83986a84a 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses *ses) } /* - * called when multichannel is disabled by the server. - * this always gets called from smb2_reconnect - * and cannot get called in parallel threads. + * cifs_decrease_secondary_channels - Reduce the number of active secondary channels + * @ses: pointer to the CIFS session structure + * @from_reconfigure: if true, only reduce to chan_max; if false, reduce to a single channel + * + * This function disables and cleans up extra secondary channels for a CIFS session. + * If called during reconfiguration, it reduces the channel count to the new maximum (chan_max). + * Otherwise, it disables all but the primary channel. */ void -cifs_disable_secondary_channels(struct cifs_ses *ses) +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure) { int i, chan_count; struct TCP_Server_Info *server; @@ -281,12 +285,13 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) if (chan_count == 1) goto done; - ses->chan_count = 1; - - /* for all secondary channels reset the need reconnect bit */ - ses->chans_need_reconnect &= 1; + /* Update the chan_count to the new maximum */ + if (from_reconfigure) + ses->chan_count = ses->chan_max; + else + ses->chan_count = 1; - for (i = 1; i < chan_count; i++) { + for (i = ses->chan_max ; i < chan_count; i++) { iface = ses->chans[i].iface; server = ses->chans[i].server; @@ -318,6 +323,15 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) spin_lock(&ses->chan_lock); } + /* For extra secondary channels, reset the need reconnect bit */ + if (ses->chan_count == 1) { + cifs_dbg(VFS, "server does not support multichannel anymore. Disable all other channels\n"); + ses->chans_need_reconnect &= 1; + } else { + cifs_dbg(VFS, "Disable extra secondary channels\n"); + ses->chans_need_reconnect &= ((1UL << ses->chan_max) - 1); + } + done: spin_unlock(&ses->chan_lock); } diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 8b4a4573e9c3..37a3216b37e4 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd, static int cifs_chan_skip_or_disable(struct cifs_ses *ses, struct TCP_Server_Info *server, - bool from_reconnect) + bool from_reconnect, bool from_reconfigure) { struct TCP_Server_Info *pserver; unsigned int chan_index; @@ -206,14 +206,42 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses, return -EHOSTDOWN; } - cifs_server_dbg(VFS, - "server does not support multichannel anymore. Disable all other channels\n"); - cifs_disable_secondary_channels(ses); - + cifs_decrease_secondary_channels(ses, from_reconfigure); return 0; } +/* + * smb3_update_ses_channels - Synchronize session channels with new configuration + * @ses: pointer to the CIFS session structure + * @server: pointer to the TCP server info structure + * @from_reconnect: indicates if called from reconnect context + * @from_reconfigure: indicates if called from reconfigure context + * + * Returns 0 on success or error code on failure. + * + * Outside of reconfigure, this function is called from cifs_mount() during mount + * and from reconnect scenarios to adjust channel count when the + * server's multichannel support changes. + */ +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, + bool from_reconnect, bool from_reconfigure) +{ + int rc = 0; + /* + * If the current channel count is less than the new chan_max, + * try to add channels to reach the new maximum. + * Otherwise, disable or skip extra channels to match the new configuration. + */ + if (ses->chan_count < ses->chan_max) + rc = cifs_try_adding_channels(ses); + else + rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, + from_reconfigure); + + return rc; +} + static int smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, struct TCP_Server_Info *server, bool from_reconnect) @@ -355,8 +383,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, */ if (ses->chan_count > 1 && !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { - rc = cifs_chan_skip_or_disable(ses, server, - from_reconnect); + rc = smb3_update_ses_channels(ses, server, + from_reconnect, false /* from_reconfigure */); if (rc) { mutex_unlock(&ses->session_mutex); goto out; @@ -438,8 +466,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, * treat this as server not supporting multichannel */ - rc = cifs_chan_skip_or_disable(ses, server, - from_reconnect); + rc = smb3_update_ses_channels(ses, server, + from_reconnect, + false /* from_reconfigure */); goto skip_add_channels; } else if (rc) cifs_tcon_dbg(FYI, "%s: failed to query server interfaces: %d\n", @@ -451,7 +480,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, if (ses->chan_count == 1) cifs_server_dbg(VFS, "supports multichannel now\n"); - cifs_try_adding_channels(ses); + smb3_update_ses_channels(ses, server, from_reconnect, + false /* from_reconfigure */); } } else { mutex_unlock(&ses->session_mutex); @@ -1105,8 +1135,7 @@ SMB2_negotiate(const unsigned int xid, req->SecurityMode = 0; req->Capabilities = cpu_to_le32(server->vals->req_capabilities); - if (ses->chan_max > 1) - req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); + req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); /* ClientGUID must be zero for SMB2.02 dialect */ if (server->vals->protocol_id == SMB20_PROT_ID) @@ -1312,8 +1341,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) pneg_inbuf->Capabilities = cpu_to_le32(server->vals->req_capabilities); - if (tcon->ses->chan_max > 1) - pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); + pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); memcpy(pneg_inbuf->Guid, server->client_guid, SMB2_CLIENT_GUID_SIZE); -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] cifs: client: allow changing multichannel mount options on remount 2025-12-01 9:21 ` RAJASI MANDAL @ 2025-12-01 18:35 ` Steve French 0 siblings, 0 replies; 9+ messages in thread From: Steve French @ 2025-12-01 18:35 UTC (permalink / raw) To: RAJASI MANDAL Cc: Shyam Prasad N, Meetakshi Setiya, sfrench, linux-cifs, pc, ronniesahlberg, sprasad, tom, bharathsm, samba-technical, linux-kernel, Rajasi Mandal merged the two patches into ksmbd-for-next (will move to cifs-2.6.git for-next once the next P/R is complete, but want to avoid merge conflicts for now) pending additional testing and review On Mon, Dec 1, 2025 at 3:21 AM RAJASI MANDAL <rajasimandalos@gmail.com> wrote: > > Sending a follow-up patch with corrected comment as pointed out by Meetakshi. Attaching the new patch here for easy reference. > > Regards, > Rajasi > > On Mon, Nov 24, 2025 at 5:20 PM Shyam Prasad N <nspmangalore@gmail.com> wrote: >> >> On Thu, Nov 20, 2025 at 11:40 AM Meetakshi Setiya >> <meetakshisetiyaoss@gmail.com> wrote: >> > >> > Hi Rajasi, >> > >> > On Tue, Nov 18, 2025 at 7:59 AM Rajasi Mandal <rajasimandalos@gmail.com> wrote: >> > > >> > > From: Rajasi Mandal <rajasimandal@microsoft.com> >> > > >> > > Previously, the client did not update a session's channel state when >> > > multichannel or max_channels mount options were changed via remount. >> > > This led to inconsistent behavior and prevented enabling or disabling >> > > multichannel support without a full unmount/remount cycle. >> > > >> > > Enable dynamic reconfiguration of multichannel and max_channels during >> > > remount by: >> > > - Introducing smb3_sync_ses_chan_max(), a centralized function for >> > > channel updates which synchronizes the session's channels with the >> > > updated configuration. >> > > - Replacing cifs_disable_secondary_channels() with >> > > cifs_decrease_secondary_channels(), which accepts a from_reconfigure >> > > flag to support targeted cleanup during reconfiguration. >> > > - Updating remount logic to detect changes in multichannel or >> > > max_channels and trigger appropriate session/channel updates. >> > > >> > > Current limitation: >> > > - The query_interfaces worker runs even when max_channels=1 so that >> > > multichannel can be enabled later via remount without requiring an >> > > unmount. This is a temporary approach and may be refined in the >> > > future. >> > > >> > > Users can safely modify multichannel and max_channels on an existing >> > > mount. The client will correctly adjust the session's channel state to >> > > match the new configuration, preserving durability where possible and >> > > avoiding unnecessary disconnects. >> > > >> > > Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> >> > > Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> >> > > --- >> > > fs/smb/client/cifsproto.h | 4 ++- >> > > fs/smb/client/connect.c | 4 ++- >> > > fs/smb/client/fs_context.c | 50 +++++++++++++++++++++++++++++++++- >> > > fs/smb/client/sess.c | 32 +++++++++++++++------- >> > > fs/smb/client/smb2pdu.c | 55 ++++++++++++++++++++++++++++---------- >> > > 5 files changed, 119 insertions(+), 26 deletions(-) >> > > >> > > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h >> > > index 3528c365a452..a1fc9e1918bc 100644 >> > > --- a/fs/smb/client/cifsproto.h >> > > +++ b/fs/smb/client/cifsproto.h >> > > @@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct shash_desc **sdesc); >> > > void cifs_free_hash(struct shash_desc **sdesc); >> > > >> > > int cifs_try_adding_channels(struct cifs_ses *ses); >> > > +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, >> > > + bool from_reconnect, bool from_reconfigure); >> > > bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); >> > > void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); >> > > >> > > @@ -674,7 +676,7 @@ bool >> > > cifs_chan_is_iface_active(struct cifs_ses *ses, >> > > struct TCP_Server_Info *server); >> > > void >> > > -cifs_disable_secondary_channels(struct cifs_ses *ses); >> > > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure); >> > > void >> > > cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); >> > > int >> > > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c >> > > index 55cb4b0cbd48..cebe4a5f54f2 100644 >> > > --- a/fs/smb/client/connect.c >> > > +++ b/fs/smb/client/connect.c >> > > @@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) >> > > ctx->prepath = NULL; >> > > >> > > out: >> > > - cifs_try_adding_channels(mnt_ctx.ses); >> > > + smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server, >> > > + false /* from_reconnect */, >> > > + false /* from_reconfigure */); >> > >> > Shouldn't this be added to the non-dfs cifs_mount() too? I see that >> > even cifs_try_adding_channels() is not present in this function, >> > @Shyam Prasad is this expected? >> >> That's a good catch. >> I think this is a day-0 bug. >> I can see that the original change by Aurelien has the change only in >> DFS cifs_mount: >> commit d70e9fa55884760b6d6c293dbf20d8c52ce11fb7 >> Author: Aurelien Aptel <aaptel@suse.com> >> Date: Fri Sep 20 06:31:10 2019 +0200 >> >> cifs: try opening channels after mounting >> >> We should have a follow-up patch to fix this. >> >> > >> > > rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); >> > > if (rc) >> > > goto error; >> > > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c >> > > index 0f894d09157b..751ed6ebd458 100644 >> > > --- a/fs/smb/client/fs_context.c >> > > +++ b/fs/smb/client/fs_context.c >> > > @@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, >> > > static int smb3_fs_context_parse_monolithic(struct fs_context *fc, >> > > void *data); >> > > static int smb3_get_tree(struct fs_context *fc); >> > > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels); >> > > static int smb3_reconfigure(struct fs_context *fc); >> > > >> > > static const struct fs_context_operations smb3_fs_context_ops = { >> > > @@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_se >> > > return 0; >> > > } >> > > >> > > +/* >> > > + * smb3_sync_ses_chan_max - Synchronize the session's maximum channel count >> > > + * @ses: pointer to the old CIFS session structure >> > > + * @max_channels: new maximum number of channels to allow >> > > + * >> > > + * Updates the session's chan_max field to the new value, protecting the update >> > > + * with the session's channel lock. This should be called whenever the maximum >> > > + * allowed channels for a session changes (e.g., after a remount or reconfigure). >> > > + */ >> > > +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels) >> > > +{ >> > > + spin_lock(&ses->chan_lock); >> > > + ses->chan_max = max_channels; >> > > + spin_unlock(&ses->chan_lock); >> > > +} >> > > + >> > > static int smb3_reconfigure(struct fs_context *fc) >> > > { >> > > struct smb3_fs_context *ctx = smb3_fc2context(fc); >> > > @@ -1095,7 +1112,38 @@ static int smb3_reconfigure(struct fs_context *fc) >> > > ses->password2 = new_password2; >> > > } >> > > >> > > - mutex_unlock(&ses->session_mutex); >> > > + /* >> > > + * If multichannel or max_channels has changed, update the session's channels accordingly. >> > > + * This may add or remove channels to match the new configuration. >> > > + */ >> > > + if ((ctx->multichannel != cifs_sb->ctx->multichannel) || >> > > + (ctx->max_channels != cifs_sb->ctx->max_channels)) { >> > > + >> > > + /* Synchronize ses->chan_max with the new mount context */ >> > > + smb3_sync_ses_chan_max(ses, ctx->max_channels); >> > > + /* Now update the session's channels to match the new configuration */ >> > > + /* Prevent concurrent scaling operations */ >> > > + spin_lock(&ses->ses_lock); >> > > + if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) { >> > > + spin_unlock(&ses->ses_lock); >> > > + return -EINVAL; >> > > + } >> > > + ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; >> > > + spin_unlock(&ses->ses_lock); >> > > + >> > > + mutex_unlock(&ses->session_mutex); >> > > + >> > > + rc = smb3_update_ses_channels(ses, ses->server, >> > > + false /* from_reconnect */, >> > > + true /* from_reconfigure */); >> > > + >> > > + /* Clear scaling flag after operation */ >> > > + spin_lock(&ses->ses_lock); >> > > + ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS; >> > > + spin_unlock(&ses->ses_lock); >> > > + } else { >> > > + mutex_unlock(&ses->session_mutex); >> > > + } >> > > >> > > STEAL_STRING(cifs_sb, ctx, domainname); >> > > STEAL_STRING(cifs_sb, ctx, nodename); >> > > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c >> > > index ef3b498b0a02..cfd83986a84a 100644 >> > > --- a/fs/smb/client/sess.c >> > > +++ b/fs/smb/client/sess.c >> > > @@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses *ses) >> > > } >> > > >> > > /* >> > > - * called when multichannel is disabled by the server. >> > > - * this always gets called from smb2_reconnect >> > > - * and cannot get called in parallel threads. >> > > + * cifs_decrease_secondary_channels - Reduce the number of active secondary channels >> > > + * @ses: pointer to the CIFS session structure >> > > + * @from_reconfigure: if true, only reduce to chan_max; if false, reduce to a single channel >> > > + * >> > > + * This function disables and cleans up extra secondary channels for a CIFS session. >> > > + * If called during reconfiguration, it reduces the channel count to the new maximum (chan_max). >> > > + * Otherwise, it disables all but the primary channel. >> > > */ >> > > void >> > > -cifs_disable_secondary_channels(struct cifs_ses *ses) >> > > +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconfigure) >> > > { >> > > int i, chan_count; >> > > struct TCP_Server_Info *server; >> > > @@ -281,12 +285,13 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) >> > > if (chan_count == 1) >> > > goto done; >> > > >> > > - ses->chan_count = 1; >> > > - >> > > - /* for all secondary channels reset the need reconnect bit */ >> > > - ses->chans_need_reconnect &= 1; >> > > + /* Update the chan_count to the new maximum */ >> > > + if (from_reconfigure) >> > > + ses->chan_count = ses->chan_max; >> > > + else >> > > + ses->chan_count = 1; >> > > >> > > - for (i = 1; i < chan_count; i++) { >> > > + for (i = ses->chan_max ; i < chan_count; i++) { >> > > iface = ses->chans[i].iface; >> > > server = ses->chans[i].server; >> > > >> > > @@ -318,6 +323,15 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) >> > > spin_lock(&ses->chan_lock); >> > > } >> > > >> > > + /* For extra secondary channels, reset the need reconnect bit */ >> > > + if (ses->chan_count == 1) { >> > > + cifs_dbg(VFS, "server does not support multichannel anymore. Disable all other channels\n"); >> > > + ses->chans_need_reconnect &= 1; >> > > + } else { >> > > + cifs_dbg(VFS, "Disable extra secondary channels\n"); >> > > + ses->chans_need_reconnect &= ((1UL << ses->chan_max) - 1); >> > > + } >> > > + >> > > done: >> > > spin_unlock(&ses->chan_lock); >> > > } >> > > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c >> > > index 8b4a4573e9c3..d051da46eaab 100644 >> > > --- a/fs/smb/client/smb2pdu.c >> > > +++ b/fs/smb/client/smb2pdu.c >> > > @@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd, >> > > static int >> > > cifs_chan_skip_or_disable(struct cifs_ses *ses, >> > > struct TCP_Server_Info *server, >> > > - bool from_reconnect) >> > > + bool from_reconnect, bool from_reconfigure) >> > > { >> > > struct TCP_Server_Info *pserver; >> > > unsigned int chan_index; >> > > @@ -206,14 +206,41 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses, >> > > return -EHOSTDOWN; >> > > } >> > > >> > > - cifs_server_dbg(VFS, >> > > - "server does not support multichannel anymore. Disable all other channels\n"); >> > > - cifs_disable_secondary_channels(ses); >> > > - >> > > + cifs_decrease_secondary_channels(ses, from_reconfigure); >> > > >> > > return 0; >> > > } >> > > >> > > +/* >> > > + * smb3_update_ses_channels - Synchronize session channels with new configuration >> > > + * @ses: pointer to the CIFS session structure >> > > + * @server: pointer to the TCP server info structure >> > > + * @from_reconnect: indicates if called from reconnect context >> > > + * @from_reconfigure: indicates if called from reconfigure context >> > > + * >> > > + * Returns 0 on success or error code on failure. >> > > + * >> > > + * Note: Outside of reconfigure, this function is only called from reconnect scenarios >> > > + * when the server stops advertising multichannel (MC) capability. >> > > + */ >> > >> > I see that function is being called during mount too. Did you >> > mean cifs_decrease_secondary_channels()? >> > >> > > +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, >> > > + bool from_reconnect, bool from_reconfigure) >> > > +{ >> > > + int rc = 0; >> > > + /* >> > > + * If the current channel count is less than the new chan_max, >> > > + * try to add channels to reach the new maximum. >> > > + * Otherwise, disable or skip extra channels to match the new configuration. >> > > + */ >> > > + if (ses->chan_count < ses->chan_max) >> > > + rc = cifs_try_adding_channels(ses); >> > > + else >> > > + rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, >> > > + from_reconfigure); >> > > + >> > > + return rc; >> > > +} >> > > + >> > > static int >> > > smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, >> > > struct TCP_Server_Info *server, bool from_reconnect) >> > > @@ -355,8 +382,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, >> > > */ >> > > if (ses->chan_count > 1 && >> > > !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { >> > > - rc = cifs_chan_skip_or_disable(ses, server, >> > > - from_reconnect); >> > > + rc = smb3_update_ses_channels(ses, server, >> > > + from_reconnect, false /* from_reconfigure */); >> > > if (rc) { >> > > mutex_unlock(&ses->session_mutex); >> > > goto out; >> > > @@ -438,8 +465,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, >> > > * treat this as server not supporting multichannel >> > > */ >> > > >> > > - rc = cifs_chan_skip_or_disable(ses, server, >> > > - from_reconnect); >> > > + rc = smb3_update_ses_channels(ses, server, >> > > + from_reconnect, >> > > + false /* from_reconfigure */); >> > > goto skip_add_channels; >> > > } else if (rc) >> > > cifs_tcon_dbg(FYI, "%s: failed to query server interfaces: %d\n", >> > > @@ -451,7 +479,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, >> > > if (ses->chan_count == 1) >> > > cifs_server_dbg(VFS, "supports multichannel now\n"); >> > > >> > > - cifs_try_adding_channels(ses); >> > > + smb3_update_ses_channels(ses, server, from_reconnect, >> > > + false /* from_reconfigure */); >> > > } >> > > } else { >> > > mutex_unlock(&ses->session_mutex); >> > > @@ -1105,8 +1134,7 @@ SMB2_negotiate(const unsigned int xid, >> > > req->SecurityMode = 0; >> > > >> > > req->Capabilities = cpu_to_le32(server->vals->req_capabilities); >> > > - if (ses->chan_max > 1) >> > > - req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); >> > > + req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); >> > > >> > > /* ClientGUID must be zero for SMB2.02 dialect */ >> > > if (server->vals->protocol_id == SMB20_PROT_ID) >> > > @@ -1312,8 +1340,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) >> > > >> > > pneg_inbuf->Capabilities = >> > > cpu_to_le32(server->vals->req_capabilities); >> > > - if (tcon->ses->chan_max > 1) >> > > - pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); >> > > + pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); >> > > >> > > memcpy(pneg_inbuf->Guid, server->client_guid, >> > > SMB2_CLIENT_GUID_SIZE); >> > > -- >> > > 2.43.0 >> > > >> > > >> > >> > Thanks >> > Meetakshi >> > >> >> >> -- >> Regards, >> Shyam -- Thanks, Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] cifs: client: allow changing multichannel mount options on remount 2025-11-18 2:26 [PATCH] cifs: client: allow changing multichannel mount options on remount Rajasi Mandal 2025-11-18 2:26 ` [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels Rajasi Mandal 2025-11-20 6:10 ` [PATCH] cifs: client: allow changing multichannel mount options on remount Meetakshi Setiya @ 2025-12-05 20:11 ` rajasimandalos 2 siblings, 0 replies; 9+ messages in thread From: rajasimandalos @ 2025-12-05 20:11 UTC (permalink / raw) To: Steve French Cc: Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel, Rajasi Mandal From: Rajasi Mandal <rajasimandal@microsoft.com> Previously, the client did not update a session's channel state when multichannel or max_channels mount options were changed via remount. This led to inconsistent behavior and prevented enabling or disabling multichannel support without a full unmount/remount cycle. Enable dynamic reconfiguration of multichannel and max_channels during remount by: - Introducing smb3_sync_ses_chan_max(), a centralized function for channel updates which synchronizes the session's channels with the updated configuration. - Replacing cifs_disable_secondary_channels() with cifs_decrease_secondary_channels(), which accepts a disable_mchan flag to support multichannel disable when the server stops supporting multichannel. - Updating remount logic to detect changes in multichannel or max_channels and trigger appropriate session/channel updates. Current limitation: - The query_interfaces worker runs even when max_channels=1 so that multichannel can be enabled later via remount without requiring an unmount. This is a temporary approach and may be refined in the future. Users can safely modify multichannel and max_channels on an existing mount. The client will correctly adjust the session's channel state to match the new configuration, preserving durability where possible and avoiding unnecessary disconnects. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> --- fs/smb/client/cifsproto.h | 4 ++- fs/smb/client/connect.c | 4 ++- fs/smb/client/fs_context.c | 51 +++++++++++++++++++++++++++++++- fs/smb/client/sess.c | 35 ++++++++++++++++------ fs/smb/client/smb2pdu.c | 60 +++++++++++++++++++++++++++++--------- 5 files changed, 128 insertions(+), 26 deletions(-) diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 3528c365a452..f2733d18c162 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -649,6 +649,8 @@ int cifs_alloc_hash(const char *name, struct shash_desc **sdesc); void cifs_free_hash(struct shash_desc **sdesc); int cifs_try_adding_channels(struct cifs_ses *ses); +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, + bool from_reconnect, bool disable_mchan); bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); @@ -674,7 +676,7 @@ bool cifs_chan_is_iface_active(struct cifs_ses *ses, struct TCP_Server_Info *server); void -cifs_disable_secondary_channels(struct cifs_ses *ses); +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool disable_mchan); void cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); int diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 2f94d93b95e9..962f1bfdffdd 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -3927,7 +3927,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) ctx->prepath = NULL; out: - cifs_try_adding_channels(mnt_ctx.ses); + smb3_update_ses_channels(mnt_ctx.ses, mnt_ctx.server, + false /* from_reconnect */, + false /* disable_mchan */); rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); if (rc) goto error; diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 2a0d8b87bd8e..214babb012f2 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -717,6 +717,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, static int smb3_fs_context_parse_monolithic(struct fs_context *fc, void *data); static int smb3_get_tree(struct fs_context *fc); +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels); static int smb3_reconfigure(struct fs_context *fc); static const struct fs_context_operations smb3_fs_context_ops = { @@ -1013,6 +1014,22 @@ int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_se return 0; } +/* + * smb3_sync_ses_chan_max - Synchronize the session's maximum channel count + * @ses: pointer to the old CIFS session structure + * @max_channels: new maximum number of channels to allow + * + * Updates the session's chan_max field to the new value, protecting the update + * with the session's channel lock. This should be called whenever the maximum + * allowed channels for a session changes (e.g., after a remount or reconfigure). + */ +static void smb3_sync_ses_chan_max(struct cifs_ses *ses, unsigned int max_channels) +{ + spin_lock(&ses->chan_lock); + ses->chan_max = max_channels; + spin_unlock(&ses->chan_lock); +} + static int smb3_reconfigure(struct fs_context *fc) { struct smb3_fs_context *ctx = smb3_fc2context(fc); @@ -1095,7 +1112,39 @@ static int smb3_reconfigure(struct fs_context *fc) ses->password2 = new_password2; } - mutex_unlock(&ses->session_mutex); + /* + * If multichannel or max_channels has changed, update the session's channels accordingly. + * This may add or remove channels to match the new configuration. + */ + if ((ctx->multichannel != cifs_sb->ctx->multichannel) || + (ctx->max_channels != cifs_sb->ctx->max_channels)) { + + /* Synchronize ses->chan_max with the new mount context */ + smb3_sync_ses_chan_max(ses, ctx->max_channels); + /* Now update the session's channels to match the new configuration */ + /* Prevent concurrent scaling operations */ + spin_lock(&ses->ses_lock); + if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) { + spin_unlock(&ses->ses_lock); + mutex_unlock(&ses->session_mutex); + return -EINVAL; + } + ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; + spin_unlock(&ses->ses_lock); + + mutex_unlock(&ses->session_mutex); + + rc = smb3_update_ses_channels(ses, ses->server, + false /* from_reconnect */, + false /* disable_mchan */); + + /* Clear scaling flag after operation */ + spin_lock(&ses->ses_lock); + ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS; + spin_unlock(&ses->ses_lock); + } else { + mutex_unlock(&ses->session_mutex); + } STEAL_STRING(cifs_sb, ctx, domainname); STEAL_STRING(cifs_sb, ctx, nodename); diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index ef3b498b0a02..239cd5c8208d 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -265,12 +265,16 @@ int cifs_try_adding_channels(struct cifs_ses *ses) } /* - * called when multichannel is disabled by the server. - * this always gets called from smb2_reconnect - * and cannot get called in parallel threads. + * cifs_decrease_secondary_channels - Reduce the number of active secondary channels + * @ses: pointer to the CIFS session structure + * @disable_mchan: if true, reduce to a single channel; if false, reduce to chan_max + * + * This function disables and cleans up extra secondary channels for a CIFS session. + * If called during reconfiguration, it reduces the channel count to the new maximum (chan_max). + * Otherwise, it disables all but the primary channel. */ void -cifs_disable_secondary_channels(struct cifs_ses *ses) +cifs_decrease_secondary_channels(struct cifs_ses *ses, bool disable_mchan) { int i, chan_count; struct TCP_Server_Info *server; @@ -281,12 +285,16 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) if (chan_count == 1) goto done; - ses->chan_count = 1; - - /* for all secondary channels reset the need reconnect bit */ - ses->chans_need_reconnect &= 1; + /* Update the chan_count to the new maximum */ + if (disable_mchan) { + cifs_dbg(FYI, "server does not support multichannel anymore.\n"); + ses->chan_count = 1; + } else { + ses->chan_count = ses->chan_max; + } - for (i = 1; i < chan_count; i++) { + /* Disable all secondary channels beyond the new chan_count */ + for (i = ses->chan_count ; i < chan_count; i++) { iface = ses->chans[i].iface; server = ses->chans[i].server; @@ -318,6 +326,15 @@ cifs_disable_secondary_channels(struct cifs_ses *ses) spin_lock(&ses->chan_lock); } + /* For extra secondary channels, reset the need reconnect bit */ + if (ses->chan_count == 1) { + cifs_dbg(VFS, "Disable all secondary channels\n"); + ses->chans_need_reconnect &= 1; + } else { + cifs_dbg(VFS, "Disable extra secondary channels\n"); + ses->chans_need_reconnect &= ((1UL << ses->chan_max) - 1); + } + done: spin_unlock(&ses->chan_lock); } diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 8b4a4573e9c3..23b7792f983f 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -168,7 +168,7 @@ smb2_hdr_assemble(struct smb2_hdr *shdr, __le16 smb2_cmd, static int cifs_chan_skip_or_disable(struct cifs_ses *ses, struct TCP_Server_Info *server, - bool from_reconnect) + bool from_reconnect, bool disable_mchan) { struct TCP_Server_Info *pserver; unsigned int chan_index; @@ -206,14 +206,46 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses, return -EHOSTDOWN; } - cifs_server_dbg(VFS, - "server does not support multichannel anymore. Disable all other channels\n"); - cifs_disable_secondary_channels(ses); - + cifs_decrease_secondary_channels(ses, disable_mchan); return 0; } +/* + * smb3_update_ses_channels - Synchronize session channels with new configuration + * @ses: pointer to the CIFS session structure + * @server: pointer to the TCP server info structure + * @from_reconnect: indicates if called from reconnect context + * @disable_mchan: indicates if called from reconnect to disable multichannel + * + * Returns 0 on success or error code on failure. + * + * Outside of reconfigure, this function is called from cifs_mount() during mount + * and from reconnect scenarios to adjust channel count when the + * server's multichannel support changes. + */ +int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *server, + bool from_reconnect, bool disable_mchan) +{ + int rc = 0; + /* + * Manage session channels based on current count vs max: + * - If disable requested, skip or disable the channel + * - If below max channels, attempt to add more + * - If above max channels, skip or disable excess channels + */ + if (disable_mchan) + rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, disable_mchan); + else { + if (ses->chan_count < ses->chan_max) + rc = cifs_try_adding_channels(ses); + else if (ses->chan_count > ses->chan_max) + rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, disable_mchan); + } + + return rc; +} + static int smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, struct TCP_Server_Info *server, bool from_reconnect) @@ -355,8 +387,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, */ if (ses->chan_count > 1 && !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { - rc = cifs_chan_skip_or_disable(ses, server, - from_reconnect); + rc = smb3_update_ses_channels(ses, server, + from_reconnect, true /* disable_mchan */); if (rc) { mutex_unlock(&ses->session_mutex); goto out; @@ -438,8 +470,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, * treat this as server not supporting multichannel */ - rc = cifs_chan_skip_or_disable(ses, server, - from_reconnect); + rc = smb3_update_ses_channels(ses, server, + from_reconnect, + true /* disable_mchan */); goto skip_add_channels; } else if (rc) cifs_tcon_dbg(FYI, "%s: failed to query server interfaces: %d\n", @@ -451,7 +484,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, if (ses->chan_count == 1) cifs_server_dbg(VFS, "supports multichannel now\n"); - cifs_try_adding_channels(ses); + smb3_update_ses_channels(ses, server, from_reconnect, + false /* disable_mchan */); } } else { mutex_unlock(&ses->session_mutex); @@ -1105,8 +1139,7 @@ SMB2_negotiate(const unsigned int xid, req->SecurityMode = 0; req->Capabilities = cpu_to_le32(server->vals->req_capabilities); - if (ses->chan_max > 1) - req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); + req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); /* ClientGUID must be zero for SMB2.02 dialect */ if (server->vals->protocol_id == SMB20_PROT_ID) @@ -1312,8 +1345,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) pneg_inbuf->Capabilities = cpu_to_le32(server->vals->req_capabilities); - if (tcon->ses->chan_max > 1) - pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); + pneg_inbuf->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL); memcpy(pneg_inbuf->Guid, server->client_guid, SMB2_CLIENT_GUID_SIZE); -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-12-05 20:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-18 2:26 [PATCH] cifs: client: allow changing multichannel mount options on remount Rajasi Mandal 2025-11-18 2:26 ` [PATCH] cifs: client: enforce consistent handling of multichannel and max_channels Rajasi Mandal 2025-11-18 14:00 ` Enzo Matsumiya 2025-11-20 15:04 ` Steve French 2025-11-20 6:10 ` [PATCH] cifs: client: allow changing multichannel mount options on remount Meetakshi Setiya 2025-11-24 11:49 ` Shyam Prasad N 2025-12-01 9:21 ` RAJASI MANDAL 2025-12-01 18:35 ` Steve French 2025-12-05 20:11 ` [PATCH v2] " rajasimandalos
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox