[PATCH] smb: client: fix cifs_pick_channel when channels are equally loaded

[PATCH] smb: client: fix cifs_pick_channel when channels are equally loaded

[Henrique Carvalho]

cifs_pick_channel uses (start % chan_count) when channels are equally
loaded, but that can return a channel that failed the eligibility
checks.

Drop the fallback and return the scan-selected channel instead. If none
is eligible, keep the existing behavior of using the primary channel.

Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
---
 fs/smb/client/transport.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 75697f6d2566..26987f261850 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -807,11 +807,16 @@ cifs_cancelled_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 }
 
 /*
- * Return a channel (master if none) of @ses that can be used to send
- * regular requests.
+ * cifs_pick_channel - pick an eligible channel for network operations
  *
- * If we are currently binding a new channel (negprot/sess.setup),
- * return the new incomplete channel.
+ * @ses: session reference
+ *
+ * Select an eligible channel (not terminating and not marked as needing
+ * reconnect), preferring the least loaded one. If no eligible channel is
+ * found, fall back to the primary channel (index 0).
+ *
+ * Return: TCP_Server_Info pointer for the chosen channel, or NULL if @ses is
+ * NULL.
  */
 struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
 {
@@ -850,10 +855,6 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
 			max_in_flight = server->in_flight;
 	}
 
-	/* if all channels are equally loaded, fall back to round-robin */
-	if (min_in_flight == max_in_flight)
-		index = (uint)start % ses->chan_count;
-
 	server = ses->chans[index].server;
 	spin_unlock(&ses->chan_lock);
 
-- 
2.50.1

Re: [PATCH] smb: client: fix cifs_pick_channel when channels are equally loaded

[Shyam Prasad N]

On Thu, Jan 22, 2026 at 8:13 AM Henrique Carvalho
<henrique.carvalho@suse.com> wrote:
>
> cifs_pick_channel uses (start % chan_count) when channels are equally
> loaded, but that can return a channel that failed the eligibility
> checks.
>
> Drop the fallback and return the scan-selected channel instead. If none
> is eligible, keep the existing behavior of using the primary channel.
>
> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> ---
>  fs/smb/client/transport.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
> index 75697f6d2566..26987f261850 100644
> --- a/fs/smb/client/transport.c
> +++ b/fs/smb/client/transport.c
> @@ -807,11 +807,16 @@ cifs_cancelled_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  }
>
>  /*
> - * Return a channel (master if none) of @ses that can be used to send
> - * regular requests.
> + * cifs_pick_channel - pick an eligible channel for network operations
>   *
> - * If we are currently binding a new channel (negprot/sess.setup),
> - * return the new incomplete channel.
> + * @ses: session reference
> + *
> + * Select an eligible channel (not terminating and not marked as needing
> + * reconnect), preferring the least loaded one. If no eligible channel is
> + * found, fall back to the primary channel (index 0).
> + *
> + * Return: TCP_Server_Info pointer for the chosen channel, or NULL if @ses is
> + * NULL.
>   */
>  struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
>  {
> @@ -850,10 +855,6 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
>                         max_in_flight = server->in_flight;
>         }
>
> -       /* if all channels are equally loaded, fall back to round-robin */
> -       if (min_in_flight == max_in_flight)
> -               index = (uint)start % ses->chan_count;
> -
>         server = ses->chans[index].server;
>         spin_unlock(&ses->chan_lock);
>
> --
> 2.50.1
>
>

Had a discussion with Henrique in another email (couldn't find this
one initially).
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>

-- 
Regards,
Shyam

Re: [PATCH] smb: client: fix cifs_pick_channel when channels are equally loaded

[Steve French]

merged into cifs-2.6.git for-next

On Wed, Jan 28, 2026 at 6:05 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> On Thu, Jan 22, 2026 at 8:13 AM Henrique Carvalho
> <henrique.carvalho@suse.com> wrote:
> >
> > cifs_pick_channel uses (start % chan_count) when channels are equally
> > loaded, but that can return a channel that failed the eligibility
> > checks.
> >
> > Drop the fallback and return the scan-selected channel instead. If none
> > is eligible, keep the existing behavior of using the primary channel.
> >
> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> > ---
> >  fs/smb/client/transport.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
> > index 75697f6d2566..26987f261850 100644
> > --- a/fs/smb/client/transport.c
> > +++ b/fs/smb/client/transport.c
> > @@ -807,11 +807,16 @@ cifs_cancelled_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
> >  }
> >
> >  /*
> > - * Return a channel (master if none) of @ses that can be used to send
> > - * regular requests.
> > + * cifs_pick_channel - pick an eligible channel for network operations
> >   *
> > - * If we are currently binding a new channel (negprot/sess.setup),
> > - * return the new incomplete channel.
> > + * @ses: session reference
> > + *
> > + * Select an eligible channel (not terminating and not marked as needing
> > + * reconnect), preferring the least loaded one. If no eligible channel is
> > + * found, fall back to the primary channel (index 0).
> > + *
> > + * Return: TCP_Server_Info pointer for the chosen channel, or NULL if @ses is
> > + * NULL.
> >   */
> >  struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
> >  {
> > @@ -850,10 +855,6 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
> >                         max_in_flight = server->in_flight;
> >         }
> >
> > -       /* if all channels are equally loaded, fall back to round-robin */
> > -       if (min_in_flight == max_in_flight)
> > -               index = (uint)start % ses->chan_count;
> > -
> >         server = ses->chans[index].server;
> >         spin_unlock(&ses->chan_lock);
> >
> > --
> > 2.50.1
> >
> >
>
> Had a discussion with Henrique in another email (couldn't find this
> one initially).
> Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
>
> --
> Regards,
> Shyam
>


-- 
Thanks,

Steve

Re: [PATCH] smb: client: fix cifs_pick_channel when channels are equally loaded

[Paulo Alcantara]

Henrique Carvalho <henrique.carvalho@suse.com> writes:

> cifs_pick_channel uses (start % chan_count) when channels are equally
> loaded, but that can return a channel that failed the eligibility
> checks.
>
> Drop the fallback and return the scan-selected channel instead. If none
> is eligible, keep the existing behavior of using the primary channel.
>
> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> ---
>  fs/smb/client/transport.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
> index 75697f6d2566..26987f261850 100644
> --- a/fs/smb/client/transport.c
> +++ b/fs/smb/client/transport.c
> @@ -807,11 +807,16 @@ cifs_cancelled_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>  }
>  
>  /*
> - * Return a channel (master if none) of @ses that can be used to send
> - * regular requests.
> + * cifs_pick_channel - pick an eligible channel for network operations
>   *
> - * If we are currently binding a new channel (negprot/sess.setup),
> - * return the new incomplete channel.
> + * @ses: session reference
> + *
> + * Select an eligible channel (not terminating and not marked as needing
> + * reconnect), preferring the least loaded one. If no eligible channel is
> + * found, fall back to the primary channel (index 0).
> + *
> + * Return: TCP_Server_Info pointer for the chosen channel, or NULL if @ses is
> + * NULL.
>   */
>  struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
>  {
> @@ -850,10 +855,6 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
>  			max_in_flight = server->in_flight;
>  	}
>  
> -	/* if all channels are equally loaded, fall back to round-robin */
> -	if (min_in_flight == max_in_flight)
> -		index = (uint)start % ses->chan_count;
> -

After removing the check, @max_in_flight is no longer neeeded.

Otherwise, looks good.

Re: [PATCH] smb: client: fix cifs_pick_channel when channels are equally loaded

[Henrique Carvalho]

On Fri, Feb 20, 2026 at 01:21:03PM -0300, Paulo Alcantara wrote:
> Henrique Carvalho <henrique.carvalho@suse.com> writes:
> 
> > cifs_pick_channel uses (start % chan_count) when channels are equally
> > loaded, but that can return a channel that failed the eligibility
> > checks.
> >
> > Drop the fallback and return the scan-selected channel instead. If none
> > is eligible, keep the existing behavior of using the primary channel.
> >
> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
> > ---
> >  fs/smb/client/transport.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
> > index 75697f6d2566..26987f261850 100644
> > --- a/fs/smb/client/transport.c
> > +++ b/fs/smb/client/transport.c
> > @@ -807,11 +807,16 @@ cifs_cancelled_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
> >  }
> >  
> >  /*
> > - * Return a channel (master if none) of @ses that can be used to send
> > - * regular requests.
> > + * cifs_pick_channel - pick an eligible channel for network operations
> >   *
> > - * If we are currently binding a new channel (negprot/sess.setup),
> > - * return the new incomplete channel.
> > + * @ses: session reference
> > + *
> > + * Select an eligible channel (not terminating and not marked as needing
> > + * reconnect), preferring the least loaded one. If no eligible channel is
> > + * found, fall back to the primary channel (index 0).
> > + *
> > + * Return: TCP_Server_Info pointer for the chosen channel, or NULL if @ses is
> > + * NULL.
> >   */
> >  struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
> >  {
> > @@ -850,10 +855,6 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
> >  			max_in_flight = server->in_flight;
> >  	}
> >  
> > -	/* if all channels are equally loaded, fall back to round-robin */
> > -	if (min_in_flight == max_in_flight)
> > -		index = (uint)start % ses->chan_count;
> > -
> 
> After removing the check, @max_in_flight is no longer neeeded.
> 
> Otherwise, looks good.

Thanks!

Will send a v2.