cifs: reconnect work should have reference on server struct
authorShyam Prasad N <sprasad@microsoft.com>
Fri, 13 Oct 2023 11:43:09 +0000 (11:43 +0000)
committerSteve French <stfrench@microsoft.com>
Thu, 9 Nov 2023 16:25:32 +0000 (10:25 -0600)
The delayed work for reconnect takes server struct
as a parameter. But it does so without holding a ref
to it. Normally, this may not show a problem as
the reconnect work is only cancelled on umount.

However, since we now plan to support scaling down of
channels, and the scale down can happen from reconnect
work itself, we need to fix it.

This change takes a reference on the server struct
before it is passed to the delayed work. And drops
the reference in the delayed work itself. Or if
the delayed work is successfully cancelled, by the
process that cancels it.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/smb/client/connect.c
fs/smb/client/smb2pdu.c

index 947e3c362bebb5f8d6aa8ddc2b3a0e0b91a8c6b9..b514b41cc9f034b77e330b9fcfd739404af02ecc 100644 (file)
@@ -389,7 +389,13 @@ static int __cifs_reconnect(struct TCP_Server_Info *server,
                        spin_unlock(&server->srv_lock);
                        cifs_swn_reset_server_dstaddr(server);
                        cifs_server_unlock(server);
-                       mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+
+                       /* increase ref count which reconnect work will drop */
+                       spin_lock(&cifs_tcp_ses_lock);
+                       server->srv_count++;
+                       spin_unlock(&cifs_tcp_ses_lock);
+                       if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
+                               cifs_put_tcp_session(server, false);
                }
        } while (server->tcpStatus == CifsNeedReconnect);
 
@@ -519,7 +525,13 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server)
                spin_unlock(&server->srv_lock);
                cifs_swn_reset_server_dstaddr(server);
                cifs_server_unlock(server);
-               mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+
+               /* increase ref count which reconnect work will drop */
+               spin_lock(&cifs_tcp_ses_lock);
+               server->srv_count++;
+               spin_unlock(&cifs_tcp_ses_lock);
+               if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
+                       cifs_put_tcp_session(server, false);
        } while (server->tcpStatus == CifsNeedReconnect);
 
        mutex_lock(&server->refpath_lock);
@@ -1601,16 +1613,19 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
 
        cancel_delayed_work_sync(&server->echo);
 
-       if (from_reconnect)
+       if (from_reconnect) {
                /*
                 * Avoid deadlock here: reconnect work calls
                 * cifs_put_tcp_session() at its end. Need to be sure
                 * that reconnect work does nothing with server pointer after
                 * that step.
                 */
-               cancel_delayed_work(&server->reconnect);
-       else
-               cancel_delayed_work_sync(&server->reconnect);
+               if (cancel_delayed_work(&server->reconnect))
+                       cifs_put_tcp_session(server, from_reconnect);
+       } else {
+               if (cancel_delayed_work_sync(&server->reconnect))
+                       cifs_put_tcp_session(server, from_reconnect);
+       }
 
        spin_lock(&server->srv_lock);
        server->tcpStatus = CifsExiting;
index c75a80bb6d9eefa8a76e8c03baea7af8b7740c25..b7665155f4e2cf625f104498e168c522f1ce5133 100644 (file)
@@ -3852,12 +3852,6 @@ void smb2_reconnect_server(struct work_struct *work)
                }
                spin_unlock(&ses->chan_lock);
        }
-       /*
-        * Get the reference to server struct to be sure that the last call of
-        * cifs_put_tcon() in the loop below won't release the server pointer.
-        */
-       if (tcon_exist || ses_exist)
-               server->srv_count++;
 
        spin_unlock(&cifs_tcp_ses_lock);
 
@@ -3905,13 +3899,17 @@ void smb2_reconnect_server(struct work_struct *work)
 
 done:
        cifs_dbg(FYI, "Reconnecting tcons and channels finished\n");
-       if (resched)
+       if (resched) {
                queue_delayed_work(cifsiod_wq, &server->reconnect, 2 * HZ);
+               mutex_unlock(&pserver->reconnect_mutex);
+
+               /* no need to put tcp session as we're retrying */
+               return;
+       }
        mutex_unlock(&pserver->reconnect_mutex);
 
        /* now we can safely release srv struct */
-       if (tcon_exist || ses_exist)
-               cifs_put_tcp_session(server, 1);
+       cifs_put_tcp_session(server, true);
 }
 
 int
@@ -3931,7 +3929,12 @@ SMB2_echo(struct TCP_Server_Info *server)
            server->ops->need_neg(server)) {
                spin_unlock(&server->srv_lock);
                /* No need to send echo on newly established connections */
-               mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+               spin_lock(&cifs_tcp_ses_lock);
+               server->srv_count++;
+               spin_unlock(&cifs_tcp_ses_lock);
+               if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
+                       cifs_put_tcp_session(server, false);
+
                return rc;
        }
        spin_unlock(&server->srv_lock);