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(-)
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.0Hi 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
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
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
>
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
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
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.0On 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
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
© 2016 - 2025 Red Hat, Inc.