Re: [RFC] smb: client: define async channel update serialization

["Henrique Carvalho" <henrique.carvalho@suse.com>]

On Wed Jul 22, 2026 at 1:10 AM -03, Pengpeng Hou wrote:
> Commit 556bb341f9f2 ("smb: client: introduce multichannel async work
> during mount") moved multichannel setup out of the mount path and into
> independent work items. Mounts can share an existing session, so more than
> one worker can update the same channel array concurrently.
>
> cifs_try_adding_channels() snapshots chan_count and the number of missing
> channels under chan_lock, but drops that lock while each channel is opened.
> Two workers can therefore act on the same initial count. More importantly,
> cifs_ses_add_channel() publishes a new slot before taking session_mutex, and
> its failure path later decrements the current chan_count. If another worker
> has appended a slot in between, that rollback can remove the wrong tail.
> At the array limit, a stale worker can also index chans[chan_count] after
> another worker has filled the last slot.
>
> I considered making the mount worker use
> CIFS_SES_FLAG_SCALE_CHANNELS, but that flag is not a blocking lock. A worker
> that fails to acquire it has no retry or completion to wait on.

I have a patch that uses the delaying idea, please see: https://lore.kernel.org/linux-cifs/20260429205236.456099-2-henrique.carvalho@suse.com/

It should be rebased and tweaked, though, I have been delaying this for
a while now.

Do you have any opinion?

> In addition,
> smb2_reconnect() currently clears the flag on a shared exit path even when it
> only observed another updater owning it,

Actually this looks like a bug to me, no? Why is smb2_reconnect()
clearing a flag it did not acquire?

> and an early channel-disable path
> runs before the existing flag acquisition.
>
> Extending session_mutex around slot publication and rollback would serialize
> concurrent additions, but it does not by itself serialize the reconfigure
> decrease path. Conversely, a new mutex around all of
> smb3_update_ses_channels() needs an explicit lock order because reconnect can
> call channel removal while already holding session_mutex, whereas channel
> addition later acquires session_mutex for session setup.
>
> Would you prefer this to be fixed by coalescing mount requests onto one
> per-session work item, or by introducing a dedicated channel-update
> serialization primitive with an agreed lock order? Once that lifetime and
> locking contract is clear, I can prepare a narrow patch and keep the capacity
> check at the actual chans[] append as a final invariant.

I like the idea to put everything in the same worker so instead of
having a mchan_mount work have just an mchan_work and reschedule it from
mount / remount / reconnect.

I have started this but abandoned because I couldn't find a good enough
reason to change things. It just seemed I was moving the complexity
elsewhere without any real benefit.

-- 
Henrique