Skip to content

Commit 9e6368d

Browse files
pcacjrpcacjr
committed
smb: client: add mid_counter_lock to protect the mid counter counter
JIRA: https://issues.redhat.com/browse/RHEL-109507 Conflicts: - Context difference due to missing upstream commit 519be98 ("cifs: Add a tracepoint to track credits involved in R/W requests"). commit 9bd4279 Author: Wang Zhaolong <wangzhaolong@huaweicloud.com> Date: Mon Aug 4 21:40:04 2025 +0800 smb: client: add mid_counter_lock to protect the mid counter counter This is step 2/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. Add a dedicated mid_counter_lock to protect current_mid counter, separating it from mid_queue_lock which protects pending_mid_q operations. This reduces lock contention and prepares for finer- grained locking in subsequent patches. Changes: - Add TCP_Server_Info->mid_counter_lock spinlock - Rename CurrentMid to current_mid for consistency - Use mid_counter_lock to protect current_mid access - Update locking documentation in cifsglob.h This separation allows mid allocation to proceed without blocking queue operations, improving performance under heavy load. Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Acked-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Steve French <stfrench@microsoft.com> Signed-off-by: Paulo Alcantara <paalcant@redhat.com>
1 parent 0fb8b27 commit 9e6368d

File tree

5 files changed

+38
-35
lines changed

5 files changed

+38
-35
lines changed

‎fs/smb/client/cifsglob.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,7 @@ struct TCP_Server_Info {
738738
wait_queue_head_t response_q;
739739
wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
740740
spinlock_t mid_queue_lock; /* protect mid queue */
741+
spinlock_t mid_counter_lock;
741742
struct list_head pending_mid_q;
742743
bool noblocksnd; /* use blocking sendmsg */
743744
bool noautotune; /* do not autotune send buf sizes */
@@ -775,7 +776,7 @@ struct TCP_Server_Info {
775776
/* SMB_COM_WRITE_RAW or SMB_COM_READ_RAW. */
776777
unsigned int capabilities; /* selective disabling of caps by smb sess */
777778
int timeAdj; /* Adjust for difference in server time zone in sec */
778-
__u64 CurrentMid; /* multiplex id - rotating counter, protected by GlobalMid_Lock */
779+
__u64 current_mid; /* multiplex id - rotating counter, protected by mid_counter_lock */
779780
char cryptkey[CIFS_CRYPTO_KEY_SIZE]; /* used by ntlm, ntlmv2 etc */
780781
/* 16th byte of RFC1001 workstation name is always null */
781782
char workstation_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
@@ -2065,8 +2066,8 @@ require use of the stronger protocol */
20652066
* GlobalTotalActiveXid
20662067
* TCP_Server_Info->srv_lock (anything in struct not protected by another lock and can change)
20672068
* TCP_Server_Info->mid_queue_lock TCP_Server_Info->pending_mid_q cifs_get_tcp_session
2068-
* ->CurrentMid
20692069
* (any changes in mid_q_entry fields)
2070+
* TCP_Server_Info->mid_counter_lock TCP_Server_Info->current_mid cifs_get_tcp_session
20702071
* TCP_Server_Info->req_lock TCP_Server_Info->in_flight cifs_get_tcp_session
20712072
* ->credits
20722073
* ->echo_credits

‎fs/smb/client/connect.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ static bool cifs_tcp_ses_needs_reconnect(struct TCP_Server_Info *server, int num
358358
}
359359

360360
cifs_dbg(FYI, "Mark tcp session as need reconnect\n");
361-
trace_smb3_reconnect(server->CurrentMid, server->conn_id,
361+
trace_smb3_reconnect(server->current_mid, server->conn_id,
362362
server->hostname);
363363
server->tcpStatus = CifsNeedReconnect;
364364

@@ -1244,7 +1244,7 @@ smb2_add_credits_from_hdr(char *buffer, struct TCP_Server_Info *server)
12441244
spin_unlock(&server->req_lock);
12451245
wake_up(&server->request_q);
12461246

1247-
trace_smb3_hdr_credits(server->CurrentMid,
1247+
trace_smb3_hdr_credits(server->current_mid,
12481248
server->conn_id, server->hostname, scredits,
12491249
le16_to_cpu(shdr->CreditRequest), in_flight);
12501250
cifs_server_dbg(FYI, "%s: added %u credits total=%d\n",
@@ -1825,6 +1825,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
18251825
spin_lock_init(&tcp_ses->req_lock);
18261826
spin_lock_init(&tcp_ses->srv_lock);
18271827
spin_lock_init(&tcp_ses->mid_queue_lock);
1828+
spin_lock_init(&tcp_ses->mid_counter_lock);
18281829
INIT_LIST_HEAD(&tcp_ses->tcp_ses_list);
18291830
INIT_LIST_HEAD(&tcp_ses->smb_ses_list);
18301831
INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request);

‎fs/smb/client/smb1ops.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,9 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
169169
__u16 last_mid, cur_mid;
170170
bool collision, reconnect = false;
171171

172-
spin_lock(&server->mid_queue_lock);
173-
172+
spin_lock(&server->mid_counter_lock);
174173
/* mid is 16 bit only for CIFS/SMB */
175-
cur_mid = (__u16)((server->CurrentMid) & 0xffff);
174+
cur_mid = (__u16)((server->current_mid) & 0xffff);
176175
/* we do not want to loop forever */
177176
last_mid = cur_mid;
178177
cur_mid++;
@@ -198,6 +197,7 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
198197
cur_mid++;
199198

200199
num_mids = 0;
200+
spin_lock(&server->mid_queue_lock);
201201
list_for_each_entry(mid_entry, &server->pending_mid_q, qhead) {
202202
++num_mids;
203203
if (mid_entry->mid == cur_mid &&
@@ -207,6 +207,7 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
207207
break;
208208
}
209209
}
210+
spin_unlock(&server->mid_queue_lock);
210211

211212
/*
212213
* if we have more than 32k mids in the list, then something
@@ -223,12 +224,12 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
223224

224225
if (!collision) {
225226
mid = (__u64)cur_mid;
226-
server->CurrentMid = mid;
227+
server->current_mid = mid;
227228
break;
228229
}
229230
cur_mid++;
230231
}
231-
spin_unlock(&server->mid_queue_lock);
232+
spin_unlock(&server->mid_counter_lock);
232233

233234
if (reconnect) {
234235
cifs_signal_cifsd_for_reconnect(server, false);

‎fs/smb/client/smb2ops.c‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ smb2_add_credits(struct TCP_Server_Info *server,
9090
if (*val > 65000) {
9191
*val = 65000; /* Don't get near 64K credits, avoid srv bugs */
9292
pr_warn_once("server overflowed SMB3 credits\n");
93-
trace_smb3_overflow_credits(server->CurrentMid,
93+
trace_smb3_overflow_credits(server->current_mid,
9494
server->conn_id, server->hostname, *val,
9595
add, server->in_flight);
9696
}
@@ -121,15 +121,15 @@ smb2_add_credits(struct TCP_Server_Info *server,
121121
wake_up(&server->request_q);
122122

123123
if (reconnect_detected) {
124-
trace_smb3_reconnect_detected(server->CurrentMid,
124+
trace_smb3_reconnect_detected(server->current_mid,
125125
server->conn_id, server->hostname, scredits, add, in_flight);
126126

127127
cifs_dbg(FYI, "trying to put %d credits from the old server instance %d\n",
128128
add, instance);
129129
}
130130

131131
if (reconnect_with_invalid_credits) {
132-
trace_smb3_reconnect_with_invalid_credits(server->CurrentMid,
132+
trace_smb3_reconnect_with_invalid_credits(server->current_mid,
133133
server->conn_id, server->hostname, scredits, add, in_flight);
134134
cifs_dbg(FYI, "Negotiate operation when server credits is non-zero. Optype: %d, server credits: %d, credits added: %d\n",
135135
optype, scredits, add);
@@ -161,7 +161,7 @@ smb2_add_credits(struct TCP_Server_Info *server,
161161
break;
162162
}
163163

164-
trace_smb3_add_credits(server->CurrentMid,
164+
trace_smb3_add_credits(server->current_mid,
165165
server->conn_id, server->hostname, scredits, add, in_flight);
166166
cifs_dbg(FYI, "%s: added %u credits total=%d\n", __func__, add, scredits);
167167
}
@@ -188,7 +188,7 @@ smb2_set_credits(struct TCP_Server_Info *server, const int val)
188188
in_flight = server->in_flight;
189189
spin_unlock(&server->req_lock);
190190

191-
trace_smb3_set_credits(server->CurrentMid,
191+
trace_smb3_set_credits(server->current_mid,
192192
server->conn_id, server->hostname, scredits, val, in_flight);
193193
cifs_dbg(FYI, "%s: set %u credits\n", __func__, val);
194194

@@ -273,7 +273,7 @@ smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
273273
in_flight = server->in_flight;
274274
spin_unlock(&server->req_lock);
275275

276-
trace_smb3_wait_credits(server->CurrentMid,
276+
trace_smb3_wait_credits(server->current_mid,
277277
server->conn_id, server->hostname, scredits, -(credits->value), in_flight);
278278
cifs_dbg(FYI, "%s: removed %u credits total=%d\n",
279279
__func__, credits->value, scredits);
@@ -293,7 +293,7 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
293293
return 0;
294294

295295
if (credits->value < new_val) {
296-
trace_smb3_too_many_credits(server->CurrentMid,
296+
trace_smb3_too_many_credits(server->current_mid,
297297
server->conn_id, server->hostname, 0, credits->value - new_val, 0);
298298
cifs_server_dbg(VFS, "request has less credits (%d) than required (%d)",
299299
credits->value, new_val);
@@ -308,7 +308,7 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
308308
in_flight = server->in_flight;
309309
spin_unlock(&server->req_lock);
310310

311-
trace_smb3_reconnect_detected(server->CurrentMid,
311+
trace_smb3_reconnect_detected(server->current_mid,
312312
server->conn_id, server->hostname, scredits,
313313
credits->value - new_val, in_flight);
314314
cifs_server_dbg(VFS, "trying to return %d credits to old session\n",
@@ -322,7 +322,7 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
322322
spin_unlock(&server->req_lock);
323323
wake_up(&server->request_q);
324324

325-
trace_smb3_adj_credits(server->CurrentMid,
325+
trace_smb3_adj_credits(server->current_mid,
326326
server->conn_id, server->hostname, scredits,
327327
credits->value - new_val, in_flight);
328328
cifs_dbg(FYI, "%s: adjust added %u credits total=%d\n",
@@ -338,19 +338,19 @@ smb2_get_next_mid(struct TCP_Server_Info *server)
338338
{
339339
__u64 mid;
340340
/* for SMB2 we need the current value */
341-
spin_lock(&server->mid_queue_lock);
342-
mid = server->CurrentMid++;
343-
spin_unlock(&server->mid_queue_lock);
341+
spin_lock(&server->mid_counter_lock);
342+
mid = server->current_mid++;
343+
spin_unlock(&server->mid_counter_lock);
344344
return mid;
345345
}
346346

347347
static void
348348
smb2_revert_current_mid(struct TCP_Server_Info *server, const unsigned int val)
349349
{
350-
spin_lock(&server->mid_queue_lock);
351-
if (server->CurrentMid >= val)
352-
server->CurrentMid -= val;
353-
spin_unlock(&server->mid_queue_lock);
350+
spin_lock(&server->mid_counter_lock);
351+
if (server->current_mid >= val)
352+
server->current_mid -= val;
353+
spin_unlock(&server->mid_counter_lock);
354354
}
355355

356356
static struct mid_q_entry *
@@ -424,9 +424,9 @@ smb2_negotiate(const unsigned int xid,
424424
{
425425
int rc;
426426

427-
spin_lock(&server->mid_queue_lock);
428-
server->CurrentMid = 0;
429-
spin_unlock(&server->mid_queue_lock);
427+
spin_lock(&server->mid_counter_lock);
428+
server->current_mid = 0;
429+
spin_unlock(&server->mid_counter_lock);
430430
rc = SMB2_negotiate(xid, ses, server);
431431
return rc;
432432
}
@@ -2453,7 +2453,7 @@ smb2_is_status_pending(char *buf, struct TCP_Server_Info *server)
24532453
spin_unlock(&server->req_lock);
24542454
wake_up(&server->request_q);
24552455

2456-
trace_smb3_pend_credits(server->CurrentMid,
2456+
trace_smb3_pend_credits(server->current_mid,
24572457
server->conn_id, server->hostname, scredits,
24582458
le16_to_cpu(shdr->CreditRequest), in_flight);
24592459
cifs_dbg(FYI, "%s: status pending add %u credits total=%d\n",

‎fs/smb/client/transport.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
424424
* socket so the server throws away the partial SMB
425425
*/
426426
cifs_signal_cifsd_for_reconnect(server, false);
427-
trace_smb3_partial_send_reconnect(server->CurrentMid,
427+
trace_smb3_partial_send_reconnect(server->current_mid,
428428
server->conn_id, server->hostname);
429429
}
430430
smbd_done:
@@ -536,7 +536,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
536536
in_flight = server->in_flight;
537537
spin_unlock(&server->req_lock);
538538

539-
trace_smb3_nblk_credits(server->CurrentMid,
539+
trace_smb3_nblk_credits(server->current_mid,
540540
server->conn_id, server->hostname, scredits, -1, in_flight);
541541
cifs_dbg(FYI, "%s: remove %u credits total=%d\n",
542542
__func__, 1, scredits);
@@ -569,7 +569,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
569569
in_flight = server->in_flight;
570570
spin_unlock(&server->req_lock);
571571

572-
trace_smb3_credit_timeout(server->CurrentMid,
572+
trace_smb3_credit_timeout(server->current_mid,
573573
server->conn_id, server->hostname, scredits,
574574
num_credits, in_flight);
575575
cifs_server_dbg(VFS, "wait timed out after %d ms\n",
@@ -612,7 +612,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
612612
spin_unlock(&server->req_lock);
613613

614614
trace_smb3_credit_timeout(
615-
server->CurrentMid,
615+
server->current_mid,
616616
server->conn_id, server->hostname,
617617
scredits, num_credits, in_flight);
618618
cifs_server_dbg(VFS, "wait timed out after %d ms\n",
@@ -642,7 +642,7 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
642642
in_flight = server->in_flight;
643643
spin_unlock(&server->req_lock);
644644

645-
trace_smb3_waitff_credits(server->CurrentMid,
645+
trace_smb3_waitff_credits(server->current_mid,
646646
server->conn_id, server->hostname, scredits,
647647
-(num_credits), in_flight);
648648
cifs_dbg(FYI, "%s: remove %u credits total=%d\n",
@@ -693,7 +693,7 @@ wait_for_compound_request(struct TCP_Server_Info *server, int num,
693693
*/
694694
if (server->in_flight == 0) {
695695
spin_unlock(&server->req_lock);
696-
trace_smb3_insufficient_credits(server->CurrentMid,
696+
trace_smb3_insufficient_credits(server->current_mid,
697697
server->conn_id, server->hostname, scredits,
698698
num, in_flight);
699699
cifs_dbg(FYI, "%s: %d requests in flight, needed %d total=%d\n",

0 commit comments

Comments
 (0)