Henrique patch review #117
Conversation
It was possible for two query interface works to be concurrently trying to update the interfaces. Prevent this by checking and updating iface_last_update under iface_lock. Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]>
There is a missing ses->iface_lock in cifs_setup_session, around ses->iface_last_update. Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]>
Mounts can experience large delays when servers advertise interfaces that are unreachable from the client. To fix this, decouple channel addition from the synchronous mount path by introducing struct mchan_mount and running channel setup as background work. Reviewed-by: Shyam Prasad N <[email protected]> Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]>
Multichannel support is independent of DFS configuration. Extend the async multichannel setup to non-DFS cifs.ko. Reviewed-by: Shyam Prasad N <[email protected]> Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]>
smfrench
requested review from
Copilot and
sprasad-microsoft
and removed request for
sprasad-microsoft
There was a problem hiding this comment.
Pull request overview
This PR refactors the SMB client multichannel interface querying mechanism to improve concurrency and defer channel setup during mount operations. The changes move the interface timestamp update logic to prevent duplicate checks and introduce asynchronous channel setup via a work queue.
Changes:
- Moved
iface_last_updatetimestamp check and update fromparse_server_interfaces()toSMB3_request_interfaces()with proper locking - Introduced asynchronous multichannel setup during mount using a work queue structure (
mchan_mount) - Added proper locking around
iface_last_updateaccess incifs_setup_session()
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| fs/smb/client/smb2ops.c | Refactored interface timestamp logic to prevent redundant checks; moved update to earlier in the call chain with proper spinlock protection |
| fs/smb/client/connect.c | Added mchan_mount structure and functions to defer smb3_update_ses_channels to a work queue; integrated into both DFS and non-DFS mount paths with proper error handling and locking for iface_last_update |
| fs/smb/client/cifsglob.h | Defined new mchan_mount structure containing work_struct and session pointer for asynchronous channel setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return 0; | ||
| } | ||
|
|
||
| ses->iface_last_update = jiffies; |
There was a problem hiding this comment.
The timestamp update occurs before the SMB2_ioctl call, which means if the ioctl fails (e.g., with -EOPNOTSUPP or any other error), the timestamp will still be set. This prevents retry attempts for the configured polling interval even when the query didn't succeed. Consider moving this update to after the successful completion of parse_server_interfaces to ensure the timestamp only reflects successful queries.
4 patches to review