From 6e3524faf7b30c6eccfb68432613288a0f3d378e Mon Sep 17 00:00:00 2001 From: Enzo Matsumiya Date: Fri, 16 May 2025 17:31:58 -0300 Subject: smb: client: open secondary channels in parallel When mounting with max_channels > 1, if the server advertises an interface that's not reachable by the client, mount will hang until the connection fails. This patch moves cifs_try_adding_channels call to the query interfaces worker, and move the worker to IPC tcons. There's no need to have one of these for each non-IPC tcon. It's necessary to hold a reference to ses when running the worker, as the last non-IPC tcon might free ses. Worker is started as soon as IPC tcon is successful. If the primary channel is healthy/reachable, this allows the mount to complete faster by adding secondary channels in parallel right after. Extras: - Add cifs_{ses,server}_has_multichannel() helpers to simplify several checks - Remove ->query_server_interfaces ops function pointer Signed-off-by: Enzo Matsumiya --- fs/smb/client/cifsglob.h | 14 +++- fs/smb/client/cifsproto.h | 3 +- fs/smb/client/connect.c | 66 +++++------------- fs/smb/client/sess.c | 169 +++++++++++++++++++--------------------------- fs/smb/client/smb2ops.c | 92 ++++++++++++++++++++----- fs/smb/client/smb2pdu.c | 37 ++-------- 6 files changed, 181 insertions(+), 200 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index d50466930c13..04bc2056cd07 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -390,9 +390,6 @@ struct smb_version_operations { /* informational QFS call */ void (*qfs_tcon)(const unsigned int, struct cifs_tcon *, struct cifs_sb_info *); - /* query for server interfaces */ - int (*query_server_interfaces)(const unsigned int, struct cifs_tcon *, - bool); /* check if a path is accessible or not */ int (*is_path_accessible)(const unsigned int, struct cifs_tcon *, struct cifs_sb_info *, const char *); @@ -878,6 +875,12 @@ static inline void cifs_server_unlock(struct TCP_Server_Info *server) memalloc_nofs_restore(nofs_flag); } +static inline bool cifs_server_has_multichannel(struct TCP_Server_Info *server) +{ + return (server && (server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL) && + server->dialect >= SMB30_PROT_ID); +} + struct cifs_credits { unsigned int value; unsigned int instance; @@ -1185,6 +1188,11 @@ struct cifs_ses { char *dns_dom; /* FQDN of the domain */ }; +static inline bool cifs_ses_has_multichannel(struct cifs_ses *ses) +{ + return (ses && ses->chan_max > 1 && cifs_server_has_multichannel(ses->server)); +} + static inline bool cap_unix(struct cifs_ses *ses) { diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index ecf774a8f1ca..d5e66391c26f 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -647,8 +647,7 @@ void cifs_disable_secondary_channels(struct cifs_ses *ses); void cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); -int -SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount); +void smb2_query_server_interfaces(struct work_struct *work); void extract_unc_hostname(const char *unc, const char **h, size_t *len); int copy_path_name(char *dst, const char *src); diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index af728cfccf54..c2e859822744 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -108,37 +108,6 @@ static int reconn_set_ipaddr_from_hostname(struct TCP_Server_Info *server) return rc; } -static void smb2_query_server_interfaces(struct work_struct *work) -{ - int rc; - int xid; - struct cifs_tcon *tcon = container_of(work, - struct cifs_tcon, - query_interfaces.work); - struct TCP_Server_Info *server = tcon->ses->server; - - /* - * query server network interfaces, in case they change - */ - if (!server->ops->query_server_interfaces) - return; - - xid = get_xid(); - rc = server->ops->query_server_interfaces(xid, tcon, false); - free_xid(xid); - - if (rc) { - if (rc == -EOPNOTSUPP) - return; - - cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n", - __func__, rc); - } - - queue_delayed_work(cifsiod_wq, &tcon->query_interfaces, - (SMB_INTERFACE_POLL_INTERVAL * HZ)); -} - /* * Update the tcpStatus for the server. * This is used to signal the cifsd thread to call cifs_reconnect @@ -278,10 +247,10 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, spin_lock(&tcon->tc_lock); tcon->status = TID_NEED_RECON; spin_unlock(&tcon->tc_lock); - - cancel_delayed_work(&tcon->query_interfaces); } if (ses->tcon_ipc) { + if (cifs_ses_has_multichannel(ses)) + cancel_delayed_work(&tcon->query_interfaces); ses->tcon_ipc->need_reconnect = true; spin_lock(&ses->tcon_ipc->tc_lock); ses->tcon_ipc->status = TID_NEED_RECON; @@ -2084,6 +2053,8 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx) scnprintf(unc, sizeof(unc), "\\\\%s\\IPC$", server->hostname); spin_unlock(&server->srv_lock); + INIT_DELAYED_WORK(&tcon->query_interfaces, smb2_query_server_interfaces); + xid = get_xid(); tcon->ses = ses; tcon->ipc = true; @@ -2097,7 +2068,7 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx) goto out; } - cifs_dbg(FYI, "IPC tcon rc=%d ipc tid=0x%x\n", rc, tcon->tid); + cifs_dbg(FYI, "IPC tcon ipc tid=0x%x\n", tcon->tid); spin_lock(&tcon->tc_lock); tcon->status = TID_GOOD; @@ -2139,11 +2110,17 @@ void __cifs_put_smb_ses(struct cifs_ses *ses) { struct TCP_Server_Info *server = ses->server; struct cifs_tcon *tcon; + bool multichannel; unsigned int xid; size_t i; bool do_logoff; int rc; + multichannel = cifs_ses_has_multichannel(ses); + /* Temporarily disable query interfaces */ + if (multichannel && ses->tcon_ipc) + disable_delayed_work(&ses->tcon_ipc->query_interfaces); + spin_lock(&cifs_tcp_ses_lock); spin_lock(&ses->ses_lock); cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n", @@ -2152,6 +2129,9 @@ void __cifs_put_smb_ses(struct cifs_ses *ses) if (ses->ses_status == SES_EXITING || --ses->ses_count > 0) { spin_unlock(&ses->ses_lock); spin_unlock(&cifs_tcp_ses_lock); + /* Enable query interfaces back */ + if (multichannel && ses->tcon_ipc) + enable_delayed_work(&ses->tcon_ipc->query_interfaces); return; } /* ses_count can never go negative */ @@ -2556,7 +2536,10 @@ retry_new_session: list_add(&ses->smb_ses_list, &server->smb_ses_list); spin_unlock(&cifs_tcp_ses_lock); - cifs_setup_ipc(ses, ctx); + rc = cifs_setup_ipc(ses, ctx); + if (!rc && cifs_ses_has_multichannel(ses)) + /* query interfaces now */ + queue_delayed_work(cifsiod_wq, &ses->tcon_ipc->query_interfaces, 0); free_xid(xid); @@ -2656,8 +2639,6 @@ cifs_put_tcon(struct cifs_tcon *tcon, enum smb3_tcon_ref_trace trace) spin_unlock(&tcon->tc_lock); spin_unlock(&cifs_tcp_ses_lock); - /* cancel polling of interfaces */ - cancel_delayed_work_sync(&tcon->query_interfaces); #ifdef CONFIG_CIFS_DFS_UPCALL cancel_delayed_work_sync(&tcon->dfs_cache_work); list_replace_init(&tcon->dfs_ses_list, &ses_list); @@ -2914,15 +2895,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx) tcon->local_lease = ctx->local_lease; INIT_LIST_HEAD(&tcon->pending_opens); tcon->status = TID_GOOD; - - INIT_DELAYED_WORK(&tcon->query_interfaces, - smb2_query_server_interfaces); - if (ses->server->dialect >= SMB30_PROT_ID && - (ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { - /* schedule query interfaces poll */ - queue_delayed_work(cifsiod_wq, &tcon->query_interfaces, - (SMB_INTERFACE_POLL_INTERVAL * HZ)); - } #ifdef CONFIG_CIFS_DFS_UPCALL INIT_DELAYED_WORK(&tcon->dfs_cache_work, dfs_cache_refresh); #endif @@ -3967,9 +3939,7 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) kfree(cifs_sb->prepath); cifs_sb->prepath = ctx->prepath; ctx->prepath = NULL; - out: - cifs_try_adding_channels(mnt_ctx.ses); rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); if (rc) goto error; diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index b3fa9ee26912..ded2e38f97c0 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -143,52 +143,32 @@ cifs_chan_is_iface_active(struct cifs_ses *ses, ses->chans[chan_index].iface->is_active; } -/* returns number of channels added */ -int cifs_try_adding_channels(struct cifs_ses *ses) +/* + * Return: + * 0: all channels opened + * >0: number of channels remaining to be opened + * <0: error from cifs_ses_add_channel() + */ +static int try_adding_channels(struct cifs_ses *ses) { - struct TCP_Server_Info *server = ses->server; - int old_chan_count, new_chan_count; - int left; - int rc = 0; - int tries = 0; size_t iface_weight = 0, iface_min_speed = 0; struct cifs_server_iface *iface = NULL, *niface = NULL; struct cifs_server_iface *last_iface = NULL; + int rc = 0, left = ses->chan_max - ses->chan_count; - spin_lock(&ses->chan_lock); - - new_chan_count = old_chan_count = ses->chan_count; - left = ses->chan_max - ses->chan_count; - - if (left <= 0) { - spin_unlock(&ses->chan_lock); - cifs_dbg(FYI, - "ses already at max_channels (%zu), nothing to open\n", - ses->chan_max); - return 0; - } - - if (server->dialect < SMB30_PROT_ID) { - spin_unlock(&ses->chan_lock); - cifs_dbg(VFS, "multichannel is not supported on this protocol version, use 3.0 or above\n"); - return 0; - } + last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface, iface_head); + iface_min_speed = last_iface->speed; - if (!(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { - spin_unlock(&ses->chan_lock); - cifs_server_dbg(VFS, "no multichannel support\n"); - return 0; - } - spin_unlock(&ses->chan_lock); + list_for_each_entry_safe(iface, niface, &ses->iface_list, iface_head) { + rc = 0; - while (left > 0) { + /* do not mix rdma and non-rdma interfaces */ + if (iface->rdma_capable != ses->server->rdma) + continue; - tries++; - if (tries > 3*ses->chan_max) { - cifs_dbg(VFS, "too many channel open attempts (%d channels left to open)\n", - left); - break; - } + /* skip ifaces that are unusable */ + if (!iface->is_active || (is_ses_using_iface(ses, iface) && !iface->rss_capable)) + continue; spin_lock(&ses->iface_lock); if (!ses->iface_count) { @@ -198,70 +178,65 @@ int cifs_try_adding_channels(struct cifs_ses *ses) break; } - if (!iface) - iface = list_first_entry(&ses->iface_list, struct cifs_server_iface, - iface_head); - last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface, - iface_head); - iface_min_speed = last_iface->speed; - - list_for_each_entry_safe_from(iface, niface, &ses->iface_list, - iface_head) { - /* do not mix rdma and non-rdma interfaces */ - if (iface->rdma_capable != ses->server->rdma) - continue; - - /* skip ifaces that are unusable */ - if (!iface->is_active || - (is_ses_using_iface(ses, iface) && - !iface->rss_capable)) - continue; - - /* check if we already allocated enough channels */ - iface_weight = iface->speed / iface_min_speed; - - if (iface->weight_fulfilled >= iface_weight) - continue; - - /* take ref before unlock */ - kref_get(&iface->refcount); - + /* check if we already allocated enough channels */ + iface_weight = iface->speed / iface_min_speed; + if (iface->weight_fulfilled >= iface_weight) { spin_unlock(&ses->iface_lock); - rc = cifs_ses_add_channel(ses, iface); - spin_lock(&ses->iface_lock); + continue; + } - if (rc) { - cifs_dbg(VFS, "failed to open extra channel on iface:%pIS rc=%d\n", - &iface->sockaddr, - rc); - kref_put(&iface->refcount, release_iface); - /* failure to add chan should increase weight */ - iface->weight_fulfilled++; - continue; - } + /* take ref before unlock */ + kref_get(&iface->refcount); + spin_unlock(&ses->iface_lock); + rc = cifs_ses_add_channel(ses, iface); + spin_lock(&ses->iface_lock); + + if (!rc) { + cifs_info("successfully opened new channel on iface:%pIS\n", &iface->sockaddr); iface->num_channels++; iface->weight_fulfilled++; - cifs_info("successfully opened new channel on iface:%pIS\n", - &iface->sockaddr); - break; - } - - /* reached end of list. reset weight_fulfilled and start over */ - if (list_entry_is_head(iface, &ses->iface_list, iface_head)) { - list_for_each_entry(iface, &ses->iface_list, iface_head) - iface->weight_fulfilled = 0; - spin_unlock(&ses->iface_lock); - iface = NULL; - continue; + left--; + } else { + cifs_dbg(VFS, "failed to open extra channel on iface:%pIS rc=%d\n", + &iface->sockaddr, rc); + kref_put(&iface->refcount, release_iface); + iface->weight_fulfilled++; + if (rc != -EAGAIN) { + spin_unlock(&ses->iface_lock); + break; + } } spin_unlock(&ses->iface_lock); + } + + return rc ?: left; +} - left--; - new_chan_count++; +int cifs_try_adding_channels(struct cifs_ses *ses) +{ + int rc, retries = 0; + +try_again: + spin_lock(&ses->chan_lock); + if (ses->chan_max - ses->chan_count <= 0) { + spin_unlock(&ses->chan_lock); + cifs_dbg(FYI, "all channels opened (%zu), nothing left to open\n", ses->chan_max); + return 0; } + spin_unlock(&ses->chan_lock); + + rc = try_adding_channels(ses); + if (!rc) + return 0; + + if (rc < 0 && rc != -EAGAIN) + return rc; - return new_chan_count - old_chan_count; + if (++retries < 3 * ses->chan_max) + goto try_again; + + return -EINPROGRESS; } /* @@ -547,16 +522,14 @@ cifs_ses_add_channel(struct cifs_ses *ses, ctx->use_client_guid = true; chan_server = cifs_get_tcp_session(ctx, ses->server); + if (IS_ERR(chan_server)) { + rc = PTR_ERR(chan_server); + goto out; + } spin_lock(&ses->chan_lock); chan = &ses->chans[ses->chan_count]; chan->server = chan_server; - if (IS_ERR(chan->server)) { - rc = PTR_ERR(chan->server); - chan->server = NULL; - spin_unlock(&ses->chan_lock); - goto out; - } chan->iface = iface; ses->chan_count++; atomic_set(&ses->chan_seq, 0); diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 2fe8eeb98535..07650326397c 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -613,7 +613,7 @@ iface_cmp(struct cifs_server_iface *a, struct cifs_server_iface *b) static int parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf, - size_t buf_len, struct cifs_ses *ses, bool in_mount) + size_t buf_len, struct cifs_ses *ses) { struct network_interface_info_ioctl_rsp *p; struct sockaddr_in *addr4; @@ -653,12 +653,6 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf, * which would only be a problem if we were requesting multichannel */ if (bytes_left == 0) { - /* avoid spamming logs every 10 minutes, so log only in mount */ - if ((ses->chan_max > 1) && in_mount) - cifs_dbg(VFS, - "multichannel not available\n" - "Empty network interface list returned by server %s\n", - ses->server->hostname); rc = -EOPNOTSUPP; ses->iface_last_update = jiffies; goto out; @@ -800,8 +794,7 @@ out: return rc; } -int -SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount) +static int request_interfaces(const unsigned int xid, struct cifs_tcon *tcon) { int rc; unsigned int ret_data_len = 0; @@ -820,15 +813,25 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_ NULL /* no data input */, 0 /* no data input */, CIFSMaxBufSize, (char **)&out_buf, &ret_data_len); if (rc == -EOPNOTSUPP) { - cifs_dbg(FYI, - "server does not support query network interfaces\n"); + cifs_dbg(ONCE, "server does not support query network interfaces\n"); + + /* + * some servers like Azure SMB server do not advertise + * that multichannel has been disabled with server + * capabilities, rather return STATUS_NOT_IMPLEMENTED. + * treat this as server not supporting multichannel + */ + if (ses->chan_count > 1) { + cifs_disable_secondary_channels(ses); + goto out; + } ret_data_len = 0; } else if (rc != 0) { cifs_tcon_dbg(VFS, "error %d on ioctl to get interface list\n", rc); goto out; } - rc = parse_server_interfaces(out_buf, ret_data_len, ses, in_mount); + rc = parse_server_interfaces(out_buf, ret_data_len, ses); if (rc) goto out; @@ -847,6 +850,67 @@ out: return rc; } +void smb2_query_server_interfaces(struct work_struct *work) +{ + struct cifs_tcon *tcon = container_of(work, struct cifs_tcon, query_interfaces.work); + struct cifs_ses *ses = tcon->ses; + unsigned long delay = SMB_INTERFACE_POLL_INTERVAL * HZ; + int xid, rc; + + if (!tcon->ipc || !ses) + return; + + /* Hold a reference while we run as the last non-IPC tcon might free ses. */ + spin_lock(&cifs_tcp_ses_lock); + cifs_smb_ses_inc_refcount(ses); + spin_unlock(&cifs_tcp_ses_lock); + + spin_lock(&tcon->tc_lock); + if (tcon->status != TID_GOOD) { + spin_unlock(&tcon->tc_lock); + cifs_put_smb_ses(ses); + return; + } + spin_unlock(&tcon->tc_lock); + + spin_lock(&ses->ses_lock); + if (ses->ses_status != SES_GOOD) { + spin_unlock(&ses->ses_lock); + cifs_put_smb_ses(ses); + return; + } + spin_unlock(&ses->ses_lock); + + /* + * query server network interfaces, in case they change + */ + xid = get_xid(); + rc = request_interfaces(xid, tcon); + free_xid(xid); + + if (rc) + goto out; + + /* Got interfaces, try to add channels. */ + rc = cifs_try_adding_channels(ses); + if (rc) { + /* + * Interfaces are available, but failed to negotiate/sess setup on channel(s) -- + * try again in 2s if not a hard error. + */ + if (rc == -EAGAIN || rc == -EINPROGRESS) + delay = 2 * HZ; + } +out: + if (rc != -EOPNOTSUPP) { + if (rc) + cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n", __func__, rc); + queue_delayed_work(cifsiod_wq, &tcon->query_interfaces, delay); + } + + cifs_put_smb_ses(ses); +} + static void smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, struct cifs_sb_info *cifs_sb) @@ -876,8 +940,6 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, if (rc) return; - SMB3_request_interfaces(xid, tcon, true /* called during mount */); - SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid, FS_ATTRIBUTE_INFORMATION); SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid, @@ -5494,7 +5556,6 @@ struct smb_version_operations smb30_operations = { .tree_connect = SMB2_tcon, .tree_disconnect = SMB2_tdis, .qfs_tcon = smb3_qfs_tcon, - .query_server_interfaces = SMB3_request_interfaces, .is_path_accessible = smb2_is_path_accessible, .can_echo = smb2_can_echo, .echo = SMB2_echo, @@ -5610,7 +5671,6 @@ struct smb_version_operations smb311_operations = { .tree_connect = SMB2_tcon, .tree_disconnect = SMB2_tdis, .qfs_tcon = smb3_qfs_tcon, - .query_server_interfaces = SMB3_request_interfaces, .is_path_accessible = smb2_is_path_accessible, .can_echo = smb2_can_echo, .echo = SMB2_echo, diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 4e28632b5fd6..ee98f6c95451 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -408,43 +408,14 @@ skip_sess_setup: ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS; spin_unlock(&ses->ses_lock); - if (!rc && - (server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL) && - server->ops->query_server_interfaces) { + if (cifs_ses_has_multichannel(ses)) { mutex_unlock(&ses->session_mutex); - /* - * query server network interfaces, in case they change - */ - xid = get_xid(); - rc = server->ops->query_server_interfaces(xid, tcon, false); - free_xid(xid); - - if (rc == -EOPNOTSUPP && ses->chan_count > 1) { - /* - * some servers like Azure SMB server do not advertise - * that multichannel has been disabled with server - * capabilities, rather return STATUS_NOT_IMPLEMENTED. - * treat this as server not supporting multichannel - */ - - rc = cifs_chan_skip_or_disable(ses, server, - from_reconnect); - goto skip_add_channels; - } else if (rc) - cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n", - __func__, rc); - - if (ses->chan_max > ses->chan_count && - ses->iface_count && - !SERVER_IS_CHAN(server)) { - if (ses->chan_count == 1) { + if (ses->chan_max > ses->chan_count) { + if (ses->chan_count == 1) cifs_server_dbg(VFS, "supports multichannel now\n"); - queue_delayed_work(cifsiod_wq, &tcon->query_interfaces, - (SMB_INTERFACE_POLL_INTERVAL * HZ)); - } - cifs_try_adding_channels(ses); + queue_delayed_work(cifsiod_wq, &tcon->query_interfaces, 2 * HZ); } } else { mutex_unlock(&ses->session_mutex); -- cgit v1.2.3