cifs: maintain a state machine for tcp/smb/tcon sessions
authorShyam Prasad N <sprasad@microsoft.com>
Mon, 19 Jul 2021 17:37:52 +0000 (17:37 +0000)
committerSteve French <stfrench@microsoft.com>
Sat, 8 Jan 2022 02:09:22 +0000 (20:09 -0600)
If functions like cifs_negotiate_protocol, cifs_setup_session,
cifs_tree_connect are called in parallel on different channels,
each of these will be execute the requests. This maybe unnecessary
in some cases, and only the first caller may need to do the work.

This is achieved by having more states for the tcp/smb/tcon session
status fields. And tracking the state of reconnection based on the
state machine.

For example:
for tcp connections:
CifsNew/CifsNeedReconnect ->
  CifsNeedNegotiate ->
    CifsInNegotiate ->
      CifsNeedSessSetup ->
        CifsInSessSetup ->
          CifsGood

for smb sessions:
CifsNew/CifsNeedReconnect ->
  CifsGood

for tcon:
CifsNew/CifsNeedReconnect ->
  CifsInFilesInvalidate ->
    CifsNeedTcon ->
      CifsInTcon ->
        CifsGood

If any channel reconnect sees that it's in the middle of
transition to CifsGood, then they can skip the function.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/cifs/cifsglob.h
fs/cifs/cifssmb.c
fs/cifs/connect.c
fs/cifs/sess.c
fs/cifs/smb2pdu.c

index 4ba35faff79c44f91a628208f10558d0527eb98d..f88d2b10045a081d420d07a3fad9270934f5d7d1 100644 (file)
@@ -113,7 +113,13 @@ enum statusEnum {
        CifsGood,
        CifsExiting,
        CifsNeedReconnect,
-       CifsNeedNegotiate
+       CifsNeedNegotiate,
+       CifsInNegotiate,
+       CifsNeedSessSetup,
+       CifsInSessSetup,
+       CifsNeedTcon,
+       CifsInTcon,
+       CifsInFilesInvalidate
 };
 
 enum securityEnum {
index 3ef2796e2f24348c936bac8be3181e9e9c96e418..071e2f21a7db7e4713f79278a8ea2c7d7418ee9a 100644 (file)
@@ -73,6 +73,16 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
        struct list_head *tmp;
        struct list_head *tmp1;
 
+       /* only send once per connect */
+       spin_lock(&cifs_tcp_ses_lock);
+       if (tcon->ses->status != CifsGood ||
+           tcon->tidStatus != CifsNeedReconnect) {
+               spin_unlock(&cifs_tcp_ses_lock);
+               return;
+       }
+       tcon->tidStatus = CifsInFilesInvalidate;
+       spin_unlock(&cifs_tcp_ses_lock);
+
        /* list all files open on tree connection and mark them invalid */
        spin_lock(&tcon->open_file_lock);
        list_for_each_safe(tmp, tmp1, &tcon->openFileList) {
@@ -89,6 +99,11 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
        memset(tcon->crfid.fid, 0, sizeof(struct cifs_fid));
        mutex_unlock(&tcon->crfid.fid_mutex);
 
+       spin_lock(&cifs_tcp_ses_lock);
+       if (tcon->tidStatus == CifsInFilesInvalidate)
+               tcon->tidStatus = CifsNeedTcon;
+       spin_unlock(&cifs_tcp_ses_lock);
+
        /*
         * BB Add call to invalidate_inodes(sb) for all superblocks mounted
         * to this tcon.
@@ -182,12 +197,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 
        nls_codepage = load_nls_default();
 
-       /*
-        * need to prevent multiple threads trying to simultaneously
-        * reconnect the same SMB session
-        */
-       mutex_lock(&ses->session_mutex);
-
        /*
         * Recheck after acquire mutex. If another thread is negotiating
         * and the server never sends an answer the socket will be closed
@@ -197,7 +206,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
        if (server->tcpStatus == CifsNeedReconnect) {
                spin_unlock(&cifs_tcp_ses_lock);
                rc = -EHOSTDOWN;
-               mutex_unlock(&ses->session_mutex);
                goto out;
        }
        spin_unlock(&cifs_tcp_ses_lock);
@@ -215,11 +223,11 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
                        goto skip_sess_setup;
 
                rc = -EHOSTDOWN;
-               mutex_unlock(&ses->session_mutex);
                goto out;
        }
        spin_unlock(&ses->chan_lock);
 
+       mutex_lock(&ses->session_mutex);
        rc = cifs_negotiate_protocol(0, ses, server);
        if (!rc)
                rc = cifs_setup_session(0, ses, server, nls_codepage);
index 815f629933dec340d38873064d684212cc139004..a408187c700265c8ac87eb4d40aef393ab36735f 100644 (file)
@@ -208,10 +208,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
                if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses))
                        goto next_session;
 
+               ses->status = CifsNeedReconnect;
                num_sessions++;
 
-               list_for_each_entry(tcon, &ses->tcon_list, tcon_list)
+               list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
                        tcon->need_reconnect = true;
+                       tcon->tidStatus = CifsNeedReconnect;
+               }
                if (ses->tcon_ipc)
                        ses->tcon_ipc->need_reconnect = true;
 
@@ -2035,12 +2038,12 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
                cifs_dbg(FYI, "Existing smb sess found (status=%d)\n",
                         ses->status);
 
-               mutex_lock(&ses->session_mutex);
                spin_lock(&ses->chan_lock);
                if (cifs_chan_needs_reconnect(ses, server)) {
                        spin_unlock(&ses->chan_lock);
                        cifs_dbg(FYI, "Session needs reconnect\n");
 
+                       mutex_lock(&ses->session_mutex);
                        rc = cifs_negotiate_protocol(xid, ses, server);
                        if (rc) {
                                mutex_unlock(&ses->session_mutex);
@@ -2059,10 +2062,11 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
                                free_xid(xid);
                                return ERR_PTR(rc);
                        }
+                       mutex_unlock(&ses->session_mutex);
+
                        spin_lock(&ses->chan_lock);
                }
                spin_unlock(&ses->chan_lock);
-               mutex_unlock(&ses->session_mutex);
 
                /* existing SMB ses has a server reference already */
                cifs_put_tcp_session(server, 0);
@@ -2112,7 +2116,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
 
        ses->sectype = ctx->sectype;
        ses->sign = ctx->sign;
-       mutex_lock(&ses->session_mutex);
 
        /* add server as first channel */
        spin_lock(&ses->chan_lock);
@@ -2122,15 +2125,16 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
        ses->chans_need_reconnect = 1;
        spin_unlock(&ses->chan_lock);
 
+       mutex_lock(&ses->session_mutex);
        rc = cifs_negotiate_protocol(xid, ses, server);
        if (!rc)
                rc = cifs_setup_session(xid, ses, server, ctx->local_nls);
+       mutex_unlock(&ses->session_mutex);
 
        /* each channel uses a different signing key */
        memcpy(ses->chans[0].signkey, ses->smb3signingkey,
               sizeof(ses->smb3signingkey));
 
-       mutex_unlock(&ses->session_mutex);
        if (rc)
                goto get_ses_fail;
 
@@ -2347,10 +2351,6 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
                }
        }
 
-       /*
-        * BB Do we need to wrap session_mutex around this TCon call and Unix
-        * SetFS as we do on SessSetup and reconnect?
-        */
        xid = get_xid();
        rc = ses->server->ops->tree_connect(xid, ses, ctx->UNC, tcon,
                                            ctx->local_nls);
@@ -3870,14 +3870,20 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
                return -ENOSYS;
 
        /* only send once per connect */
-       if (!server->ops->need_neg(server))
+       spin_lock(&cifs_tcp_ses_lock);
+       if (!server->ops->need_neg(server) ||
+           server->tcpStatus != CifsNeedNegotiate) {
+               spin_unlock(&cifs_tcp_ses_lock);
                return 0;
+       }
+       server->tcpStatus = CifsInNegotiate;
+       spin_unlock(&cifs_tcp_ses_lock);
 
        rc = server->ops->negotiate(xid, ses, server);
        if (rc == 0) {
                spin_lock(&cifs_tcp_ses_lock);
-               if (server->tcpStatus == CifsNeedNegotiate)
-                       server->tcpStatus = CifsGood;
+               if (server->tcpStatus == CifsInNegotiate)
+                       server->tcpStatus = CifsNeedSessSetup;
                else
                        rc = -EHOSTDOWN;
                spin_unlock(&cifs_tcp_ses_lock);
@@ -3894,6 +3900,15 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
        int rc = -ENOSYS;
        bool is_binding = false;
 
+       /* only send once per connect */
+       spin_lock(&cifs_tcp_ses_lock);
+       if (server->tcpStatus != CifsNeedSessSetup) {
+               spin_unlock(&cifs_tcp_ses_lock);
+               return 0;
+       }
+       ses->status = CifsInSessSetup;
+       spin_unlock(&cifs_tcp_ses_lock);
+
        spin_lock(&ses->chan_lock);
        is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
        spin_unlock(&ses->chan_lock);
@@ -4264,6 +4279,17 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
        struct dfs_cache_tgt_iterator *tit;
        bool target_match;
 
+       /* only send once per connect */
+       spin_lock(&cifs_tcp_ses_lock);
+       if (tcon->ses->status != CifsGood ||
+           (tcon->tidStatus != CifsNew &&
+           tcon->tidStatus != CifsNeedTcon)) {
+               spin_unlock(&cifs_tcp_ses_lock);
+               return 0;
+       }
+       tcon->tidStatus = CifsInTcon;
+       spin_unlock(&cifs_tcp_ses_lock);
+
        extract_unc_hostname(server->hostname, &tcp_host, &tcp_host_len);
 
        tit = dfs_cache_get_tgt_iterator(tl);
@@ -4422,6 +4448,17 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
 {
        const struct smb_version_operations *ops = tcon->ses->server->ops;
 
+       /* only send once per connect */
+       spin_lock(&cifs_tcp_ses_lock);
+       if (tcon->ses->status != CifsGood ||
+           (tcon->tidStatus != CifsNew &&
+           tcon->tidStatus != CifsNeedTcon)) {
+               spin_unlock(&cifs_tcp_ses_lock);
+               return 0;
+       }
+       tcon->tidStatus = CifsInTcon;
+       spin_unlock(&cifs_tcp_ses_lock);
+
        return ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
 }
 #endif
index 03ba3084395017eeda84a24ab575162c1cc469f8..d12490e12be53ed18ca6e4f6718b96273e1891b1 100644 (file)
@@ -308,7 +308,6 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
 
        chan_server = cifs_get_tcp_session(&ctx, ses->server);
 
-       mutex_lock(&ses->session_mutex);
        spin_lock(&ses->chan_lock);
        chan = &ses->chans[ses->chan_count];
        chan->server = chan_server;
@@ -326,6 +325,7 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
 
        spin_unlock(&ses->chan_lock);
 
+       mutex_lock(&ses->session_mutex);
        /*
         * We need to allocate the server crypto now as we will need
         * to sign packets before we generate the channel signing key
@@ -334,6 +334,7 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
        rc = smb311_crypto_shash_allocate(chan->server);
        if (rc) {
                cifs_dbg(VFS, "%s: crypto alloc failed\n", __func__);
+               mutex_unlock(&ses->session_mutex);
                goto out;
        }
 
@@ -341,6 +342,8 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
        if (!rc)
                rc = cifs_setup_session(xid, ses, chan->server, cifs_sb->local_nls);
 
+       mutex_unlock(&ses->session_mutex);
+
 out:
        if (rc && chan->server) {
                spin_lock(&ses->chan_lock);
@@ -355,8 +358,6 @@ out:
                spin_unlock(&ses->chan_lock);
        }
 
-       mutex_unlock(&ses->session_mutex);
-
        if (rc && chan->server)
                cifs_put_tcp_session(chan->server, 0);
 
@@ -1053,6 +1054,7 @@ sess_establish_session(struct sess_data *sess_data)
 
        /* Even if one channel is active, session is in good state */
        spin_lock(&cifs_tcp_ses_lock);
+       server->tcpStatus = CifsGood;
        ses->status = CifsGood;
        spin_unlock(&cifs_tcp_ses_lock);
 
index 0c18b6f4f9eb1676cb827e16b817a06b1385d566..2725e62470e43f328f0aeb3b4beea85acd03c3c3 100644 (file)
@@ -251,12 +251,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 
        nls_codepage = load_nls_default();
 
-       /*
-        * need to prevent multiple threads trying to simultaneously reconnect
-        * the same SMB session
-        */
-       mutex_lock(&ses->session_mutex);
-
        /*
         * Recheck after acquire mutex. If another thread is negotiating
         * and the server never sends an answer the socket will be closed
@@ -266,7 +260,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
        if (server->tcpStatus == CifsNeedReconnect) {
                spin_unlock(&cifs_tcp_ses_lock);
                rc = -EHOSTDOWN;
-               mutex_unlock(&ses->session_mutex);
                goto out;
        }
        spin_unlock(&cifs_tcp_ses_lock);
@@ -284,23 +277,23 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
                        goto skip_sess_setup;
 
                rc = -EHOSTDOWN;
-               mutex_unlock(&ses->session_mutex);
                goto out;
        }
        spin_unlock(&ses->chan_lock);
 
+       mutex_lock(&ses->session_mutex);
        rc = cifs_negotiate_protocol(0, ses, server);
        if (!rc) {
                rc = cifs_setup_session(0, ses, server, nls_codepage);
                if ((rc == -EACCES) && !tcon->retry) {
-                       rc = -EHOSTDOWN;
                        mutex_unlock(&ses->session_mutex);
+                       rc = -EHOSTDOWN;
                        goto failed;
                }
        }
 
        if (rc || !tcon->need_reconnect) {
-               mutex_unlock(&tcon->ses->session_mutex);
+               mutex_unlock(&ses->session_mutex);
                goto out;
        }
 
@@ -310,7 +303,7 @@ skip_sess_setup:
                tcon->need_reopen_files = true;
 
        rc = cifs_tree_connect(0, tcon, nls_codepage);
-       mutex_unlock(&tcon->ses->session_mutex);
+       mutex_unlock(&ses->session_mutex);
 
        cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
        if (rc) {
@@ -1397,6 +1390,7 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data)
 
        /* Even if one channel is active, session is in good state */
        spin_lock(&cifs_tcp_ses_lock);
+       server->tcpStatus = CifsGood;
        ses->status = CifsGood;
        spin_unlock(&cifs_tcp_ses_lock);