Skip to content

Commit 659743b

Browse files
shlomopongratzJames Bottomley
shlomopongratz
authored and
James Bottomley
committed
[SCSI] libiscsi: Reduce locking contention in fast path
Replace the session lock with two locks, a forward lock and a backwards lock named frwd_lock and back_lock respectively. The forward lock protects resources that change while sending a request to the target, such as cmdsn, queued_cmdsn, and allocating task from the commands' pool with kfifo_out. The backward lock protects resources that change while processing a response or in error path, such as cmdsn_exp, cmdsn_max, and returning tasks to the commands' pool with kfifo_in. Under a steady state fast-path situation, that is when one or more processes/threads submit IO to an iscsi device and a single kernel upcall (e.g softirq) is dealing with processing of responses without errors, this patch eliminates the contention between the queuecommand()/request response/scsi_done() flows associated with iscsi sessions. Between the forward and the backward locks exists a strict locking hierarchy. The mutual exclusion zone protected by the forward lock can enclose the mutual exclusion zone protected by the backward lock but not vice versa. For example, in iscsi_conn_teardown or in iscsi_xmit_data when there is a failure and __iscsi_put_task is called, the backward lock is taken while the forward lock is still taken. On the other hand, if in the RX path a nop is to be sent, for example in iscsi_handle_reject or __iscsi_complete_pdu than the forward lock is released and the backward lock is taken for the duration of iscsi_send_nopout, later the backward lock is released and the forward lock is retaken. libiscsi_tcp uses two kernel fifos the r2t pool and the r2t queue. The insertion and deletion from these queues didn't corespond to the assumption taken by the new forward/backwards session locking paradigm. That is, in iscsi_tcp_clenup_task which belongs to the RX (backwards) path, r2t is taken out from r2t queue and inserted to the r2t pool. In iscsi_tcp_get_curr_r2t which belong to the TX (forward) path, r2t is also inserted to the r2t pool and another r2t is pulled from r2t queue. Only in iscsi_tcp_r2t_rsp which is called in the RX path but can requeue to the TX path, r2t is taken from the r2t pool and inserted to the r2t queue. In order to cope with this situation, two spin locks were added, pool2queue and queue2pool. The former protects extracting from the r2t pool and inserting to the r2t queue, and the later protects the extracing from the r2t queue and inserting to the r2t pool. Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com> [minor fix up to apply cleanly and compile fix] Signed-off-by: Mike Christie <michaelc@cs.wisc.edu> Signed-off-by: James Bottomley <JBottomley@Parallels.com>
1 parent 5d0fddd commit 659743b

File tree

9 files changed

+206
-161
lines changed

9 files changed

+206
-161
lines changed

‎drivers/scsi/be2iscsi/be_main.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -233,20 +233,20 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc)
233233
cls_session = starget_to_session(scsi_target(sc->device));
234234
session = cls_session->dd_data;
235235

236-
spin_lock_bh(&session->lock);
236+
spin_lock_bh(&session->frwd_lock);
237237
if (!aborted_task || !aborted_task->sc) {
238238
/* we raced */
239-
spin_unlock_bh(&session->lock);
239+
spin_unlock_bh(&session->frwd_lock);
240240
return SUCCESS;
241241
}
242242

243243
aborted_io_task = aborted_task->dd_data;
244244
if (!aborted_io_task->scsi_cmnd) {
245245
/* raced or invalid command */
246-
spin_unlock_bh(&session->lock);
246+
spin_unlock_bh(&session->frwd_lock);
247247
return SUCCESS;
248248
}
249-
spin_unlock_bh(&session->lock);
249+
spin_unlock_bh(&session->frwd_lock);
250250
/* Invalidate WRB Posted for this Task */
251251
AMAP_SET_BITS(struct amap_iscsi_wrb, invld,
252252
aborted_io_task->pwrb_handle->pwrb,
@@ -311,9 +311,9 @@ static int beiscsi_eh_device_reset(struct scsi_cmnd *sc)
311311
/* invalidate iocbs */
312312
cls_session = starget_to_session(scsi_target(sc->device));
313313
session = cls_session->dd_data;
314-
spin_lock_bh(&session->lock);
314+
spin_lock_bh(&session->frwd_lock);
315315
if (!session->leadconn || session->state != ISCSI_STATE_LOGGED_IN) {
316-
spin_unlock_bh(&session->lock);
316+
spin_unlock_bh(&session->frwd_lock);
317317
return FAILED;
318318
}
319319
conn = session->leadconn;
@@ -342,7 +342,7 @@ static int beiscsi_eh_device_reset(struct scsi_cmnd *sc)
342342
num_invalidate++;
343343
inv_tbl++;
344344
}
345-
spin_unlock_bh(&session->lock);
345+
spin_unlock_bh(&session->frwd_lock);
346346
inv_tbl = phba->inv_tbl;
347347

348348
nonemb_cmd.va = pci_alloc_consistent(phba->ctrl.pdev,
@@ -1185,9 +1185,9 @@ beiscsi_process_async_pdu(struct beiscsi_conn *beiscsi_conn,
11851185
return 1;
11861186
}
11871187

1188-
spin_lock_bh(&session->lock);
1188+
spin_lock_bh(&session->back_lock);
11891189
__iscsi_complete_pdu(conn, (struct iscsi_hdr *)ppdu, pbuffer, buf_len);
1190-
spin_unlock_bh(&session->lock);
1190+
spin_unlock_bh(&session->back_lock);
11911191
return 0;
11921192
}
11931193

@@ -1606,7 +1606,7 @@ static void hwi_complete_cmd(struct beiscsi_conn *beiscsi_conn,
16061606
pwrb = pwrb_handle->pwrb;
16071607
type = ((struct beiscsi_io_task *)task->dd_data)->wrb_type;
16081608

1609-
spin_lock_bh(&session->lock);
1609+
spin_lock_bh(&session->back_lock);
16101610
switch (type) {
16111611
case HWH_TYPE_IO:
16121612
case HWH_TYPE_IO_RD:
@@ -1645,7 +1645,7 @@ static void hwi_complete_cmd(struct beiscsi_conn *beiscsi_conn,
16451645
break;
16461646
}
16471647

1648-
spin_unlock_bh(&session->lock);
1648+
spin_unlock_bh(&session->back_lock);
16491649
}
16501650

16511651
static struct list_head *hwi_get_async_busy_list(struct hwi_async_pdu_context
@@ -4693,9 +4693,9 @@ beiscsi_offload_connection(struct beiscsi_conn *beiscsi_conn,
46934693
* login/startup related tasks.
46944694
*/
46954695
beiscsi_conn->login_in_progress = 0;
4696-
spin_lock_bh(&session->lock);
4696+
spin_lock_bh(&session->back_lock);
46974697
beiscsi_cleanup_task(task);
4698-
spin_unlock_bh(&session->lock);
4698+
spin_unlock_bh(&session->back_lock);
46994699

47004700
pwrb_handle = alloc_wrb_handle(phba, beiscsi_conn->beiscsi_conn_cid);
47014701

‎drivers/scsi/bnx2i/bnx2i_hwi.c

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,7 +1361,7 @@ int bnx2i_process_scsi_cmd_resp(struct iscsi_session *session,
13611361
u32 datalen = 0;
13621362

13631363
resp_cqe = (struct bnx2i_cmd_response *)cqe;
1364-
spin_lock_bh(&session->lock);
1364+
spin_lock_bh(&session->back_lock);
13651365
task = iscsi_itt_to_task(conn,
13661366
resp_cqe->itt & ISCSI_CMD_RESPONSE_INDEX);
13671367
if (!task)
@@ -1432,7 +1432,7 @@ int bnx2i_process_scsi_cmd_resp(struct iscsi_session *session,
14321432
__iscsi_complete_pdu(conn, (struct iscsi_hdr *)hdr,
14331433
conn->data, datalen);
14341434
fail:
1435-
spin_unlock_bh(&session->lock);
1435+
spin_unlock_bh(&session->back_lock);
14361436
return 0;
14371437
}
14381438

@@ -1457,7 +1457,7 @@ static int bnx2i_process_login_resp(struct iscsi_session *session,
14571457
int pad_len;
14581458

14591459
login = (struct bnx2i_login_response *) cqe;
1460-
spin_lock(&session->lock);
1460+
spin_lock(&session->back_lock);
14611461
task = iscsi_itt_to_task(conn,
14621462
login->itt & ISCSI_LOGIN_RESPONSE_INDEX);
14631463
if (!task)
@@ -1500,7 +1500,7 @@ static int bnx2i_process_login_resp(struct iscsi_session *session,
15001500
bnx2i_conn->gen_pdu.resp_buf,
15011501
bnx2i_conn->gen_pdu.resp_wr_ptr - bnx2i_conn->gen_pdu.resp_buf);
15021502
done:
1503-
spin_unlock(&session->lock);
1503+
spin_unlock(&session->back_lock);
15041504
return 0;
15051505
}
15061506

@@ -1525,7 +1525,7 @@ static int bnx2i_process_text_resp(struct iscsi_session *session,
15251525
int pad_len;
15261526

15271527
text = (struct bnx2i_text_response *) cqe;
1528-
spin_lock(&session->lock);
1528+
spin_lock(&session->back_lock);
15291529
task = iscsi_itt_to_task(conn, text->itt & ISCSI_LOGIN_RESPONSE_INDEX);
15301530
if (!task)
15311531
goto done;
@@ -1561,7 +1561,7 @@ static int bnx2i_process_text_resp(struct iscsi_session *session,
15611561
bnx2i_conn->gen_pdu.resp_wr_ptr -
15621562
bnx2i_conn->gen_pdu.resp_buf);
15631563
done:
1564-
spin_unlock(&session->lock);
1564+
spin_unlock(&session->back_lock);
15651565
return 0;
15661566
}
15671567

@@ -1584,7 +1584,7 @@ static int bnx2i_process_tmf_resp(struct iscsi_session *session,
15841584
struct iscsi_tm_rsp *resp_hdr;
15851585

15861586
tmf_cqe = (struct bnx2i_tmf_response *)cqe;
1587-
spin_lock(&session->lock);
1587+
spin_lock(&session->back_lock);
15881588
task = iscsi_itt_to_task(conn,
15891589
tmf_cqe->itt & ISCSI_TMF_RESPONSE_INDEX);
15901590
if (!task)
@@ -1600,7 +1600,7 @@ static int bnx2i_process_tmf_resp(struct iscsi_session *session,
16001600

16011601
__iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr, NULL, 0);
16021602
done:
1603-
spin_unlock(&session->lock);
1603+
spin_unlock(&session->back_lock);
16041604
return 0;
16051605
}
16061606

@@ -1623,7 +1623,7 @@ static int bnx2i_process_logout_resp(struct iscsi_session *session,
16231623
struct iscsi_logout_rsp *resp_hdr;
16241624

16251625
logout = (struct bnx2i_logout_response *) cqe;
1626-
spin_lock(&session->lock);
1626+
spin_lock(&session->back_lock);
16271627
task = iscsi_itt_to_task(conn,
16281628
logout->itt & ISCSI_LOGOUT_RESPONSE_INDEX);
16291629
if (!task)
@@ -1647,7 +1647,7 @@ static int bnx2i_process_logout_resp(struct iscsi_session *session,
16471647

16481648
bnx2i_conn->ep->state = EP_STATE_LOGOUT_RESP_RCVD;
16491649
done:
1650-
spin_unlock(&session->lock);
1650+
spin_unlock(&session->back_lock);
16511651
return 0;
16521652
}
16531653

@@ -1668,12 +1668,12 @@ static void bnx2i_process_nopin_local_cmpl(struct iscsi_session *session,
16681668
struct iscsi_task *task;
16691669

16701670
nop_in = (struct bnx2i_nop_in_msg *)cqe;
1671-
spin_lock(&session->lock);
1671+
spin_lock(&session->back_lock);
16721672
task = iscsi_itt_to_task(conn,
16731673
nop_in->itt & ISCSI_NOP_IN_MSG_INDEX);
16741674
if (task)
16751675
__iscsi_put_task(task);
1676-
spin_unlock(&session->lock);
1676+
spin_unlock(&session->back_lock);
16771677
}
16781678

16791679
/**
@@ -1712,7 +1712,7 @@ static int bnx2i_process_nopin_mesg(struct iscsi_session *session,
17121712

17131713
nop_in = (struct bnx2i_nop_in_msg *)cqe;
17141714

1715-
spin_lock(&session->lock);
1715+
spin_lock(&session->back_lock);
17161716
hdr = (struct iscsi_nopin *)&bnx2i_conn->gen_pdu.resp_hdr;
17171717
memset(hdr, 0, sizeof(struct iscsi_hdr));
17181718
hdr->opcode = nop_in->op_code;
@@ -1738,7 +1738,7 @@ static int bnx2i_process_nopin_mesg(struct iscsi_session *session,
17381738
}
17391739
done:
17401740
__iscsi_complete_pdu(conn, (struct iscsi_hdr *)hdr, NULL, 0);
1741-
spin_unlock(&session->lock);
1741+
spin_unlock(&session->back_lock);
17421742

17431743
return tgt_async_nop;
17441744
}
@@ -1771,7 +1771,7 @@ static void bnx2i_process_async_mesg(struct iscsi_session *session,
17711771
return;
17721772
}
17731773

1774-
spin_lock(&session->lock);
1774+
spin_lock(&session->back_lock);
17751775
resp_hdr = (struct iscsi_async *) &bnx2i_conn->gen_pdu.resp_hdr;
17761776
memset(resp_hdr, 0, sizeof(struct iscsi_hdr));
17771777
resp_hdr->opcode = async_cqe->op_code;
@@ -1790,7 +1790,7 @@ static void bnx2i_process_async_mesg(struct iscsi_session *session,
17901790

17911791
__iscsi_complete_pdu(bnx2i_conn->cls_conn->dd_data,
17921792
(struct iscsi_hdr *)resp_hdr, NULL, 0);
1793-
spin_unlock(&session->lock);
1793+
spin_unlock(&session->back_lock);
17941794
}
17951795

17961796

@@ -1817,7 +1817,7 @@ static void bnx2i_process_reject_mesg(struct iscsi_session *session,
18171817
} else
18181818
bnx2i_unsol_pdu_adjust_rq(bnx2i_conn);
18191819

1820-
spin_lock(&session->lock);
1820+
spin_lock(&session->back_lock);
18211821
hdr = (struct iscsi_reject *) &bnx2i_conn->gen_pdu.resp_hdr;
18221822
memset(hdr, 0, sizeof(struct iscsi_hdr));
18231823
hdr->opcode = reject->op_code;
@@ -1828,7 +1828,7 @@ static void bnx2i_process_reject_mesg(struct iscsi_session *session,
18281828
hdr->ffffffff = cpu_to_be32(RESERVED_ITT);
18291829
__iscsi_complete_pdu(conn, (struct iscsi_hdr *)hdr, conn->data,
18301830
reject->data_length);
1831-
spin_unlock(&session->lock);
1831+
spin_unlock(&session->back_lock);
18321832
}
18331833

18341834
/**
@@ -1848,13 +1848,13 @@ static void bnx2i_process_cmd_cleanup_resp(struct iscsi_session *session,
18481848
struct iscsi_task *task;
18491849

18501850
cmd_clean_rsp = (struct bnx2i_cleanup_response *)cqe;
1851-
spin_lock(&session->lock);
1851+
spin_lock(&session->back_lock);
18521852
task = iscsi_itt_to_task(conn,
18531853
cmd_clean_rsp->itt & ISCSI_CLEANUP_RESPONSE_INDEX);
18541854
if (!task)
18551855
printk(KERN_ALERT "bnx2i: cmd clean ITT %x not active\n",
18561856
cmd_clean_rsp->itt & ISCSI_CLEANUP_RESPONSE_INDEX);
1857-
spin_unlock(&session->lock);
1857+
spin_unlock(&session->back_lock);
18581858
complete(&bnx2i_conn->cmd_cleanup_cmpl);
18591859
}
18601860

@@ -1921,11 +1921,11 @@ static int bnx2i_queue_scsi_cmd_resp(struct iscsi_session *session,
19211921
int rc = 0;
19221922
int cpu;
19231923

1924-
spin_lock(&session->lock);
1924+
spin_lock(&session->back_lock);
19251925
task = iscsi_itt_to_task(bnx2i_conn->cls_conn->dd_data,
19261926
cqe->itt & ISCSI_CMD_RESPONSE_INDEX);
19271927
if (!task || !task->sc) {
1928-
spin_unlock(&session->lock);
1928+
spin_unlock(&session->back_lock);
19291929
return -EINVAL;
19301930
}
19311931
sc = task->sc;
@@ -1935,7 +1935,7 @@ static int bnx2i_queue_scsi_cmd_resp(struct iscsi_session *session,
19351935
else
19361936
cpu = sc->request->cpu;
19371937

1938-
spin_unlock(&session->lock);
1938+
spin_unlock(&session->back_lock);
19391939

19401940
p = &per_cpu(bnx2i_percpu, cpu);
19411941
spin_lock(&p->p_work_lock);

‎drivers/scsi/bnx2i/bnx2i_iscsi.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,10 +1169,10 @@ static void bnx2i_cleanup_task(struct iscsi_task *task)
11691169
if (task->state == ISCSI_TASK_ABRT_TMF) {
11701170
bnx2i_send_cmd_cleanup_req(hba, task->dd_data);
11711171

1172-
spin_unlock_bh(&conn->session->lock);
1172+
spin_unlock_bh(&conn->session->back_lock);
11731173
wait_for_completion_timeout(&bnx2i_conn->cmd_cleanup_cmpl,
11741174
msecs_to_jiffies(ISCSI_CMD_CLEANUP_TIMEOUT));
1175-
spin_lock_bh(&conn->session->lock);
1175+
spin_lock_bh(&conn->session->back_lock);
11761176
}
11771177
bnx2i_iscsi_unmap_sg_list(task->dd_data);
11781178
}
@@ -2059,7 +2059,7 @@ int bnx2i_hw_ep_disconnect(struct bnx2i_endpoint *bnx2i_ep)
20592059
goto out;
20602060

20612061
if (session) {
2062-
spin_lock_bh(&session->lock);
2062+
spin_lock_bh(&session->frwd_lock);
20632063
if (bnx2i_ep->state != EP_STATE_TCP_FIN_RCVD) {
20642064
if (session->state == ISCSI_STATE_LOGGING_OUT) {
20652065
if (bnx2i_ep->state == EP_STATE_LOGOUT_SENT) {
@@ -2075,7 +2075,7 @@ int bnx2i_hw_ep_disconnect(struct bnx2i_endpoint *bnx2i_ep)
20752075
} else
20762076
close = 1;
20772077

2078-
spin_unlock_bh(&session->lock);
2078+
spin_unlock_bh(&session->frwd_lock);
20792079
}
20802080

20812081
bnx2i_ep->state = EP_STATE_DISCONN_START;

‎drivers/scsi/iscsi_tcp.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -593,9 +593,9 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)
593593
iscsi_sw_tcp_conn_restore_callbacks(conn);
594594
sock_put(sock->sk);
595595

596-
spin_lock_bh(&session->lock);
596+
spin_lock_bh(&session->frwd_lock);
597597
tcp_sw_conn->sock = NULL;
598-
spin_unlock_bh(&session->lock);
598+
spin_unlock_bh(&session->frwd_lock);
599599
sockfd_put(sock);
600600
}
601601

@@ -663,10 +663,10 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
663663
if (err)
664664
goto free_socket;
665665

666-
spin_lock_bh(&session->lock);
666+
spin_lock_bh(&session->frwd_lock);
667667
/* bind iSCSI connection and socket */
668668
tcp_sw_conn->sock = sock;
669-
spin_unlock_bh(&session->lock);
669+
spin_unlock_bh(&session->frwd_lock);
670670

671671
/* setup Socket parameters */
672672
sk = sock->sk;
@@ -726,14 +726,14 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
726726
switch(param) {
727727
case ISCSI_PARAM_CONN_PORT:
728728
case ISCSI_PARAM_CONN_ADDRESS:
729-
spin_lock_bh(&conn->session->lock);
729+
spin_lock_bh(&conn->session->frwd_lock);
730730
if (!tcp_sw_conn || !tcp_sw_conn->sock) {
731-
spin_unlock_bh(&conn->session->lock);
731+
spin_unlock_bh(&conn->session->frwd_lock);
732732
return -ENOTCONN;
733733
}
734734
rc = kernel_getpeername(tcp_sw_conn->sock,
735735
(struct sockaddr *)&addr, &len);
736-
spin_unlock_bh(&conn->session->lock);
736+
spin_unlock_bh(&conn->session->frwd_lock);
737737
if (rc)
738738
return rc;
739739

@@ -759,23 +759,23 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host *shost,
759759

760760
switch (param) {
761761
case ISCSI_HOST_PARAM_IPADDRESS:
762-
spin_lock_bh(&session->lock);
762+
spin_lock_bh(&session->frwd_lock);
763763
conn = session->leadconn;
764764
if (!conn) {
765-
spin_unlock_bh(&session->lock);
765+
spin_unlock_bh(&session->frwd_lock);
766766
return -ENOTCONN;
767767
}
768768
tcp_conn = conn->dd_data;
769769

770770
tcp_sw_conn = tcp_conn->dd_data;
771771
if (!tcp_sw_conn->sock) {
772-
spin_unlock_bh(&session->lock);
772+
spin_unlock_bh(&session->frwd_lock);
773773
return -ENOTCONN;
774774
}
775775

776776
rc = kernel_getsockname(tcp_sw_conn->sock,
777777
(struct sockaddr *)&addr, &len);
778-
spin_unlock_bh(&session->lock);
778+
spin_unlock_bh(&session->frwd_lock);
779779
if (rc)
780780
return rc;
781781

0 commit comments

Comments
 (0)