smb: client: rename server mid_lock to mid_queue_lock

This is step 1/4 of a patch series to fix mid_q_entry memory leaks
caused by race conditions in callback execution.

The current mid_lock name is somewhat ambiguous about what it protects.
To prepare for splitting this lock into separate, more granular locks,
this patch renames mid_lock to mid_queue_lock to clearly indicate its
specific responsibility for protecting the pending_mid_q list and
related queue operations.

No functional changes are made in this patch - it only prepares the
codebase for the lock splitting that follows.

- mid_queue_lock for queue operations
- mid_counter_lock for mid counter operations
- per-mid locks for individual mid state management

Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
Acked-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index f1cea365b6f10653a4aa78e1a58db20d3fcf5c68..80d6a51b8c11d9728f33d54a71255026ea9f8603 100644 (file)
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -60,7 +60,7 @@ void cifs_dump_mids(struct TCP_Server_Info *server)
                return;
 
        cifs_dbg(VFS, "Dump pending requests:\n");
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        list_for_each_entry(mid_entry, &server->pending_mid_q, qhead) {
                cifs_dbg(VFS, "State: %d Cmd: %d Pid: %d Cbdata: %p Mid %llu\n",
                         mid_entry->mid_state,
@@ -83,7 +83,7 @@ void cifs_dump_mids(struct TCP_Server_Info *server)
                                mid_entry->resp_buf, 62);
                }
        }
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
 #endif /* CONFIG_CIFS_DEBUG2 */
 }
 
@@ -672,7 +672,7 @@ skip_rdma:
 
                                seq_printf(m, "\n\tServer ConnectionId: 0x%llx",
                                           chan_server->conn_id);
-                               spin_lock(&chan_server->mid_lock);
+                               spin_lock(&chan_server->mid_queue_lock);
                                list_for_each_entry(mid_entry, &chan_server->pending_mid_q, qhead) {
                                        seq_printf(m, "\n\t\tState: %d com: %d pid: %d cbdata: %p mid %llu",
                                                   mid_entry->mid_state,
@@ -681,7 +681,7 @@ skip_rdma:
                                                   mid_entry->callback_data,
                                                   mid_entry->mid);
                                }
-                               spin_unlock(&chan_server->mid_lock);
+                               spin_unlock(&chan_server->mid_queue_lock);
                        }
                        spin_unlock(&ses->chan_lock);
                        seq_puts(m, "\n--\n");

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index a97e2cca2f5324bd6dbe507af5629cbed9b77d3f..2dd1ef27425095b4b3eecff61621359d2c38a46a 100644 (file)
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -732,7 +732,7 @@ struct TCP_Server_Info {
 #endif
        wait_queue_head_t response_q;
        wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
-       spinlock_t mid_lock;  /* protect mid queue and it's entries */
+       spinlock_t mid_queue_lock;  /* protect mid queue */
        struct list_head pending_mid_q;
        bool noblocksnd;                /* use blocking sendmsg */
        bool noautotune;                /* do not autotune send buf sizes */
@@ -2007,7 +2007,7 @@ require use of the stronger protocol */
  *                             GlobalCurrentXid
  *                             GlobalTotalActiveXid
  * TCP_Server_Info->srv_lock   (anything in struct not protected by another lock and can change)
- * TCP_Server_Info->mid_lock   TCP_Server_Info->pending_mid_q  cifs_get_tcp_session
+ * TCP_Server_Info->mid_queue_lock     TCP_Server_Info->pending_mid_q  cifs_get_tcp_session
  *                             ->CurrentMid
  *                             (any changes in mid_q_entry fields)
  * TCP_Server_Info->req_lock   TCP_Server_Info->in_flight      cifs_get_tcp_session

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 5eec8957f2a996605f4053c2f80f3b5f20af9074..e4b577ca48d5956ffa8c7acb6659cd69b42adc39 100644 (file)
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -321,7 +321,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
        /* mark submitted MIDs for retry and issue callback */
        INIT_LIST_HEAD(&retry_list);
        cifs_dbg(FYI, "%s: moving mids to private list\n", __func__);
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
                kref_get(&mid->refcount);
                if (mid->mid_state == MID_REQUEST_SUBMITTED)
@@ -329,7 +329,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
                list_move(&mid->qhead, &retry_list);
                mid->mid_flags |= MID_DELETED;
        }
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
        cifs_server_unlock(server);
 
        cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__);
@@ -884,13 +884,13 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
                         * server there should be exactly one pending mid
                         * corresponding to SMB1/SMB2 Negotiate packet.
                         */
-                       spin_lock(&server->mid_lock);
+                       spin_lock(&server->mid_queue_lock);
                        list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
                                kref_get(&mid->refcount);
                                list_move(&mid->qhead, &dispose_list);
                                mid->mid_flags |= MID_DELETED;
                        }
-                       spin_unlock(&server->mid_lock);
+                       spin_unlock(&server->mid_queue_lock);
 
                        /* Now try to reconnect once with NetBIOS session. */
                        server->with_rfc1001 = true;
@@ -957,7 +957,7 @@ dequeue_mid(struct mid_q_entry *mid, bool malformed)
 #ifdef CONFIG_CIFS_STATS2
        mid->when_received = jiffies;
 #endif
-       spin_lock(&mid->server->mid_lock);
+       spin_lock(&mid->server->mid_queue_lock);
        if (!malformed)
                mid->mid_state = MID_RESPONSE_RECEIVED;
        else
@@ -967,12 +967,12 @@ dequeue_mid(struct mid_q_entry *mid, bool malformed)
         * function has finished processing it is a bug.
         */
        if (mid->mid_flags & MID_DELETED) {
-               spin_unlock(&mid->server->mid_lock);
+               spin_unlock(&mid->server->mid_queue_lock);
                pr_warn_once("trying to dequeue a deleted mid\n");
        } else {
                list_del_init(&mid->qhead);
                mid->mid_flags |= MID_DELETED;
-               spin_unlock(&mid->server->mid_lock);
+               spin_unlock(&mid->server->mid_queue_lock);
        }
 }
 
@@ -1101,7 +1101,7 @@ clean_demultiplex_info(struct TCP_Server_Info *server)
                struct list_head *tmp, *tmp2;
                LIST_HEAD(dispose_list);
 
-               spin_lock(&server->mid_lock);
+               spin_lock(&server->mid_queue_lock);
                list_for_each_safe(tmp, tmp2, &server->pending_mid_q) {
                        mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
                        cifs_dbg(FYI, "Clearing mid %llu\n", mid_entry->mid);
@@ -1110,7 +1110,7 @@ clean_demultiplex_info(struct TCP_Server_Info *server)
                        list_move(&mid_entry->qhead, &dispose_list);
                        mid_entry->mid_flags |= MID_DELETED;
                }
-               spin_unlock(&server->mid_lock);
+               spin_unlock(&server->mid_queue_lock);
 
                /* now walk dispose list and issue callbacks */
                list_for_each_safe(tmp, tmp2, &dispose_list) {
@@ -1822,7 +1822,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
        tcp_ses->compression.requested = ctx->compress;
        spin_lock_init(&tcp_ses->req_lock);
        spin_lock_init(&tcp_ses->srv_lock);
-       spin_lock_init(&tcp_ses->mid_lock);
+       spin_lock_init(&tcp_ses->mid_queue_lock);
        INIT_LIST_HEAD(&tcp_ses->tcp_ses_list);
        INIT_LIST_HEAD(&tcp_ses->smb_ses_list);
        INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request);

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index f722c7f47b07d61a9e5b419ff3bb4308c8b6cb3f..e16566d3c319318be8ef5c8ccc9c6177a4c3949c 100644 (file)
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -95,17 +95,17 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
        struct smb_hdr *buf = (struct smb_hdr *)buffer;
        struct mid_q_entry *mid;
 
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        list_for_each_entry(mid, &server->pending_mid_q, qhead) {
                if (compare_mid(mid->mid, buf) &&
                    mid->mid_state == MID_REQUEST_SUBMITTED &&
                    le16_to_cpu(mid->command) == buf->Command) {
                        kref_get(&mid->refcount);
-                       spin_unlock(&server->mid_lock);
+                       spin_unlock(&server->mid_queue_lock);
                        return mid;
                }
        }
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
        return NULL;
 }
 
@@ -169,7 +169,7 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
        __u16 last_mid, cur_mid;
        bool collision, reconnect = false;
 
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
 
        /* mid is 16 bit only for CIFS/SMB */
        cur_mid = (__u16)((server->CurrentMid) & 0xffff);
@@ -228,7 +228,7 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
                }
                cur_mid++;
        }
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
 
        if (reconnect) {
                cifs_signal_cifsd_for_reconnect(server, false);

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index bd6c1fb2a9923520781cba719f5d76e69eb29a7a..7935f9b433ac2c198c8e0f6112ccec109ca8a0e9 100644 (file)
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -374,19 +374,19 @@ smb2_get_next_mid(struct TCP_Server_Info *server)
 {
        __u64 mid;
        /* for SMB2 we need the current value */
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        mid = server->CurrentMid++;
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
        return mid;
 }
 
 static void
 smb2_revert_current_mid(struct TCP_Server_Info *server, const unsigned int val)
 {
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        if (server->CurrentMid >= val)
                server->CurrentMid -= val;
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
 }
 
 static struct mid_q_entry *
@@ -401,7 +401,7 @@ __smb2_find_mid(struct TCP_Server_Info *server, char *buf, bool dequeue)
                return NULL;
        }
 
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        list_for_each_entry(mid, &server->pending_mid_q, qhead) {
                if ((mid->mid == wire_mid) &&
                    (mid->mid_state == MID_REQUEST_SUBMITTED) &&
@@ -411,11 +411,11 @@ __smb2_find_mid(struct TCP_Server_Info *server, char *buf, bool dequeue)
                                list_del_init(&mid->qhead);
                                mid->mid_flags |= MID_DELETED;
                        }
-                       spin_unlock(&server->mid_lock);
+                       spin_unlock(&server->mid_queue_lock);
                        return mid;
                }
        }
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
        return NULL;
 }
 
@@ -460,9 +460,9 @@ smb2_negotiate(const unsigned int xid,
 {
        int rc;
 
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        server->CurrentMid = 0;
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
        rc = SMB2_negotiate(xid, ses, server);
        return rc;
 }
@@ -4809,18 +4809,18 @@ static void smb2_decrypt_offload(struct work_struct *work)
                } else {
                        spin_lock(&dw->server->srv_lock);
                        if (dw->server->tcpStatus == CifsNeedReconnect) {
-                               spin_lock(&dw->server->mid_lock);
+                               spin_lock(&dw->server->mid_queue_lock);
                                mid->mid_state = MID_RETRY_NEEDED;
-                               spin_unlock(&dw->server->mid_lock);
+                               spin_unlock(&dw->server->mid_queue_lock);
                                spin_unlock(&dw->server->srv_lock);
                                mid->callback(mid);
                        } else {
-                               spin_lock(&dw->server->mid_lock);
+                               spin_lock(&dw->server->mid_queue_lock);
                                mid->mid_state = MID_REQUEST_SUBMITTED;
                                mid->mid_flags &= ~(MID_DELETED);
                                list_add_tail(&mid->qhead,
                                        &dw->server->pending_mid_q);
-                               spin_unlock(&dw->server->mid_lock);
+                               spin_unlock(&dw->server->mid_queue_lock);
                                spin_unlock(&dw->server->srv_lock);
                        }
                }

diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
index 475b36c27f65436a13ab46015cb6debbede95cee..ff9ef7fcd0105e4bd881bf2b2ebdaeb60d7f4d83 100644 (file)
--- a/fs/smb/client/smb2transport.c
+++ b/fs/smb/client/smb2transport.c
@@ -840,9 +840,9 @@ smb2_get_mid_entry(struct cifs_ses *ses, struct TCP_Server_Info *server,
        *mid = smb2_mid_entry_alloc(shdr, server);
        if (*mid == NULL)
                return -ENOMEM;
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        list_add_tail(&(*mid)->qhead, &server->pending_mid_q);
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
 
        return 0;
 }

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 191783f553ce88a34c83b8175cc5b74120b82201..12dc927aa4a2011910cec29ed8a2d2d310293f4f 100644 (file)
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -160,12 +160,12 @@ void __release_mid(struct kref *refcount)
 void
 delete_mid(struct mid_q_entry *mid)
 {
-       spin_lock(&mid->server->mid_lock);
+       spin_lock(&mid->server->mid_queue_lock);
        if (!(mid->mid_flags & MID_DELETED)) {
                list_del_init(&mid->qhead);
                mid->mid_flags |= MID_DELETED;
        }
-       spin_unlock(&mid->server->mid_lock);
+       spin_unlock(&mid->server->mid_queue_lock);
 
        release_mid(mid);
 }
@@ -716,9 +716,9 @@ static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
        *ppmidQ = alloc_mid(in_buf, ses->server);
        if (*ppmidQ == NULL)
                return -ENOMEM;
-       spin_lock(&ses->server->mid_lock);
+       spin_lock(&ses->server->mid_queue_lock);
        list_add_tail(&(*ppmidQ)->qhead, &ses->server->pending_mid_q);
-       spin_unlock(&ses->server->mid_lock);
+       spin_unlock(&ses->server->mid_queue_lock);
        return 0;
 }
 
@@ -819,9 +819,9 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
        mid->mid_state = MID_REQUEST_SUBMITTED;
 
        /* put it on the pending_mid_q */
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        list_add_tail(&mid->qhead, &server->pending_mid_q);
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
 
        /*
         * Need to store the time in mid before calling I/O. For call_async,
@@ -880,10 +880,10 @@ cifs_sync_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server)
        cifs_dbg(FYI, "%s: cmd=%d mid=%llu state=%d\n",
                 __func__, le16_to_cpu(mid->command), mid->mid, mid->mid_state);
 
-       spin_lock(&server->mid_lock);
+       spin_lock(&server->mid_queue_lock);
        switch (mid->mid_state) {
        case MID_RESPONSE_READY:
-               spin_unlock(&server->mid_lock);
+               spin_unlock(&server->mid_queue_lock);
                return rc;
        case MID_RETRY_NEEDED:
                rc = -EAGAIN;
@@ -902,13 +902,13 @@ cifs_sync_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server)
                        list_del_init(&mid->qhead);
                        mid->mid_flags |= MID_DELETED;
                }
-               spin_unlock(&server->mid_lock);
+               spin_unlock(&server->mid_queue_lock);
                cifs_server_dbg(VFS, "%s: invalid mid state mid=%llu state=%d\n",
                         __func__, mid->mid, mid->mid_state);
                rc = -EIO;
                goto sync_mid_done;
        }
-       spin_unlock(&server->mid_lock);
+       spin_unlock(&server->mid_queue_lock);
 
 sync_mid_done:
        release_mid(mid);
@@ -1213,7 +1213,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
                        cifs_server_dbg(FYI, "Cancelling wait for mid %llu cmd: %d\n",
                                 midQ[i]->mid, le16_to_cpu(midQ[i]->command));
                        send_cancel(server, &rqst[i], midQ[i]);
-                       spin_lock(&server->mid_lock);
+                       spin_lock(&server->mid_queue_lock);
                        midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
                        if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED ||
                            midQ[i]->mid_state == MID_RESPONSE_RECEIVED) {
@@ -1221,7 +1221,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
                                cancelled_mid[i] = true;
                                credits[i].value = 0;
                        }
-                       spin_unlock(&server->mid_lock);
+                       spin_unlock(&server->mid_queue_lock);
                }
        }
 
@@ -1423,16 +1423,16 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
        rc = wait_for_response(server, midQ);
        if (rc != 0) {
                send_cancel(server, &rqst, midQ);
-               spin_lock(&server->mid_lock);
+               spin_lock(&server->mid_queue_lock);
                if (midQ->mid_state == MID_REQUEST_SUBMITTED ||
                    midQ->mid_state == MID_RESPONSE_RECEIVED) {
                        /* no longer considered to be "in-flight" */
                        midQ->callback = release_mid;
-                       spin_unlock(&server->mid_lock);
+                       spin_unlock(&server->mid_queue_lock);
                        add_credits(server, &credits, 0);
                        return rc;
                }
-               spin_unlock(&server->mid_lock);
+               spin_unlock(&server->mid_queue_lock);
        }
 
        rc = cifs_sync_mid_result(midQ, server);
@@ -1605,15 +1605,15 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon,
                rc = wait_for_response(server, midQ);
                if (rc) {
                        send_cancel(server, &rqst, midQ);
-                       spin_lock(&server->mid_lock);
+                       spin_lock(&server->mid_queue_lock);
                        if (midQ->mid_state == MID_REQUEST_SUBMITTED ||
                            midQ->mid_state == MID_RESPONSE_RECEIVED) {
                                /* no longer considered to be "in-flight" */
                                midQ->callback = release_mid;
-                               spin_unlock(&server->mid_lock);
+                               spin_unlock(&server->mid_queue_lock);
                                return rc;
                        }
-                       spin_unlock(&server->mid_lock);
+                       spin_unlock(&server->mid_queue_lock);
                }
 
                /* We got the response - restart system call. */