Skip to content

Commit b5f3fe2

Browse files
Guoqing Jiangjgunthorpe
Guoqing Jiang
authored and
jgunthorpe
committed
RDMA/rxe: Convert spin_{lock_bh,unlock_bh} to spin_{lock_irqsave,unlock_irqrestore}
We need to call spin_lock_irqsave()/spin_unlock_irqrestore() for state_lock in rxe, otherwsie the callchain: ib_post_send_mad -> spin_lock_irqsave -> ib_post_send -> rxe_post_send -> spin_lock_bh -> spin_unlock_bh -> spin_unlock_irqrestore Causes below traces during run block nvmeof-mp/001 test due to mismatched spinlock nesting: WARNING: CPU: 0 PID: 94794 at kernel/softirq.c:376 __local_bh_enable_ip+0xc2/0x140 [ ... ] CPU: 0 PID: 94794 Comm: kworker/u4:1 Tainted: G E 6.4.0-rc1 #9 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014 Workqueue: rdma_cm cma_work_handler [rdma_cm] RIP: 0010:__local_bh_enable_ip+0xc2/0x140 Code: 48 85 c0 74 72 5b 41 5c 5d 31 c0 89 c2 89 c1 89 c6 89 c7 41 89 c0 e9 bd 0e 11 01 65 8b 05 f2 65 72 48 85 c0 0f 85 76 ff ff ff <0f> 0b e9 6f ff ff ff e8 d2 39 1c 00 eb 80 4c 89 e7 e8 68 ad 0a 00 RSP: 0018:ffffb7cf818539f0 EFLAGS: 00010046 RAX: 0000000000000000 RBX: 0000000000000201 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000201 RDI: ffffffffc0f25f79 RBP: ffffb7cf81853a00 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffc0f25f79 R13: ffff8db1f0fa6000 R14: ffff8db2c63ff000 R15: 00000000000000e8 FS: 0000000000000000(0000) GS:ffff8db33bc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000559758db0f20 CR3: 0000000105124000 CR4: 00000000003506f0 Call Trace: <TASK> _raw_spin_unlock_bh+0x31/0x40 rxe_post_send+0x59/0x8b0 [rdma_rxe] ib_send_mad+0x26b/0x470 [ib_core] ib_post_send_mad+0x150/0xb40 [ib_core] ? cm_form_tid+0x5b/0x90 [ib_cm] ib_send_cm_req+0x7c8/0xb70 [ib_cm] rdma_connect_locked+0x433/0x940 [rdma_cm] nvme_rdma_cm_handler+0x5d7/0x9c0 [nvme_rdma] cma_cm_event_handler+0x4f/0x170 [rdma_cm] cma_work_handler+0x6a/0xe0 [rdma_cm] process_one_work+0x2a9/0x580 worker_thread+0x52/0x3f0 ? __pfx_worker_thread+0x10/0x10 kthread+0x109/0x140 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x2c/0x50 </TASK> raw_local_irq_restore() called with IRQs enabled WARNING: CPU: 0 PID: 94794 at kernel/locking/irqflag-debug.c:10 warn_bogus_irq_restore+0x37/0x60 [ ... ] CPU: 0 PID: 94794 Comm: kworker/u4:1 Tainted: G W E 6.4.0-rc1 #9 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014 Workqueue: rdma_cm cma_work_handler [rdma_cm] RIP: 0010:warn_bogus_irq_restore+0x37/0x60 Code: fb 01 77 36 83 e3 01 74 0e 48 8b 5d f8 c9 31 f6 89 f7 e9 ac ea 01 00 48 c7 c7 e0 52 33 b9 c6 05 bb 1c 69 01 01 e8 39 24 f0 fe <0f> 0b 48 8b 5d f8 c9 31 f6 89 f7 e9 89 ea 01 00 0f b6 f3 48 c7 c7 RSP: 0018:ffffb7cf81853a58 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: ffffb7cf81853a60 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: ffff8db2cfb1a9e8 R13: ffff8db2cfb1a9d8 R14: ffff8db2c63ff000 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff8db33bc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000559758db0f20 CR3: 0000000105124000 CR4: 00000000003506f0 Call Trace: <TASK> _raw_spin_unlock_irqrestore+0x91/0xa0 ib_send_mad+0x1e3/0x470 [ib_core] ib_post_send_mad+0x150/0xb40 [ib_core] ? cm_form_tid+0x5b/0x90 [ib_cm] ib_send_cm_req+0x7c8/0xb70 [ib_cm] rdma_connect_locked+0x433/0x940 [rdma_cm] nvme_rdma_cm_handler+0x5d7/0x9c0 [nvme_rdma] cma_cm_event_handler+0x4f/0x170 [rdma_cm] cma_work_handler+0x6a/0xe0 [rdma_cm] process_one_work+0x2a9/0x580 worker_thread+0x52/0x3f0 ? __pfx_worker_thread+0x10/0x10 kthread+0x109/0x140 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x2c/0x50 </TASK> Fixes: f605f26 ("RDMA/rxe: Protect QP state with qp->state_lock") Link: https://lore.kernel.org/r/20230510035056.881196-1-guoqing.jiang@linux.dev Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
1 parent 17eabd6 commit b5f3fe2

7 files changed

Lines changed: 86 additions & 61 deletions

File tree

‎drivers/infiniband/sw/rxe/rxe_comp.c‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,16 @@ static enum ib_wc_opcode wr_to_wc_opcode(enum ib_wr_opcode opcode)
115115
void retransmit_timer(struct timer_list *t)
116116
{
117117
struct rxe_qp *qp = from_timer(qp, t, retrans_timer);
118+
unsigned long flags;
118119

119120
rxe_dbg_qp(qp, "retransmit timer fired\n");
120121

121-
spin_lock_bh(&qp->state_lock);
122+
spin_lock_irqsave(&qp->state_lock, flags);
122123
if (qp->valid) {
123124
qp->comp.timeout = 1;
124125
rxe_sched_task(&qp->comp.task);
125126
}
126-
spin_unlock_bh(&qp->state_lock);
127+
spin_unlock_irqrestore(&qp->state_lock, flags);
127128
}
128129

129130
void rxe_comp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb)
@@ -481,11 +482,13 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
481482

482483
static void comp_check_sq_drain_done(struct rxe_qp *qp)
483484
{
484-
spin_lock_bh(&qp->state_lock);
485+
unsigned long flags;
486+
487+
spin_lock_irqsave(&qp->state_lock, flags);
485488
if (unlikely(qp_state(qp) == IB_QPS_SQD)) {
486489
if (qp->attr.sq_draining && qp->comp.psn == qp->req.psn) {
487490
qp->attr.sq_draining = 0;
488-
spin_unlock_bh(&qp->state_lock);
491+
spin_unlock_irqrestore(&qp->state_lock, flags);
489492

490493
if (qp->ibqp.event_handler) {
491494
struct ib_event ev;
@@ -499,7 +502,7 @@ static void comp_check_sq_drain_done(struct rxe_qp *qp)
499502
return;
500503
}
501504
}
502-
spin_unlock_bh(&qp->state_lock);
505+
spin_unlock_irqrestore(&qp->state_lock, flags);
503506
}
504507

505508
static inline enum comp_state complete_ack(struct rxe_qp *qp,
@@ -625,13 +628,15 @@ static void free_pkt(struct rxe_pkt_info *pkt)
625628
*/
626629
static void reset_retry_timer(struct rxe_qp *qp)
627630
{
631+
unsigned long flags;
632+
628633
if (qp_type(qp) == IB_QPT_RC && qp->qp_timeout_jiffies) {
629-
spin_lock_bh(&qp->state_lock);
634+
spin_lock_irqsave(&qp->state_lock, flags);
630635
if (qp_state(qp) >= IB_QPS_RTS &&
631636
psn_compare(qp->req.psn, qp->comp.psn) > 0)
632637
mod_timer(&qp->retrans_timer,
633638
jiffies + qp->qp_timeout_jiffies);
634-
spin_unlock_bh(&qp->state_lock);
639+
spin_unlock_irqrestore(&qp->state_lock, flags);
635640
}
636641
}
637642

@@ -643,18 +648,19 @@ int rxe_completer(struct rxe_qp *qp)
643648
struct rxe_pkt_info *pkt = NULL;
644649
enum comp_state state;
645650
int ret;
651+
unsigned long flags;
646652

647-
spin_lock_bh(&qp->state_lock);
653+
spin_lock_irqsave(&qp->state_lock, flags);
648654
if (!qp->valid || qp_state(qp) == IB_QPS_ERR ||
649655
qp_state(qp) == IB_QPS_RESET) {
650656
bool notify = qp->valid && (qp_state(qp) == IB_QPS_ERR);
651657

652658
drain_resp_pkts(qp);
653659
flush_send_queue(qp, notify);
654-
spin_unlock_bh(&qp->state_lock);
660+
spin_unlock_irqrestore(&qp->state_lock, flags);
655661
goto exit;
656662
}
657-
spin_unlock_bh(&qp->state_lock);
663+
spin_unlock_irqrestore(&qp->state_lock, flags);
658664

659665
if (qp->comp.timeout) {
660666
qp->comp.timeout_retry = 1;

‎drivers/infiniband/sw/rxe/rxe_net.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,15 +412,16 @@ int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
412412
int err;
413413
int is_request = pkt->mask & RXE_REQ_MASK;
414414
struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
415+
unsigned long flags;
415416

416-
spin_lock_bh(&qp->state_lock);
417+
spin_lock_irqsave(&qp->state_lock, flags);
417418
if ((is_request && (qp_state(qp) < IB_QPS_RTS)) ||
418419
(!is_request && (qp_state(qp) < IB_QPS_RTR))) {
419-
spin_unlock_bh(&qp->state_lock);
420+
spin_unlock_irqrestore(&qp->state_lock, flags);
420421
rxe_dbg_qp(qp, "Packet dropped. QP is not in ready state\n");
421422
goto drop;
422423
}
423-
spin_unlock_bh(&qp->state_lock);
424+
spin_unlock_irqrestore(&qp->state_lock, flags);
424425

425426
rxe_icrc_generate(skb, pkt);
426427

‎drivers/infiniband/sw/rxe/rxe_qp.c‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ int rxe_qp_from_init(struct rxe_dev *rxe, struct rxe_qp *qp, struct rxe_pd *pd,
300300
struct rxe_cq *rcq = to_rcq(init->recv_cq);
301301
struct rxe_cq *scq = to_rcq(init->send_cq);
302302
struct rxe_srq *srq = init->srq ? to_rsrq(init->srq) : NULL;
303+
unsigned long flags;
303304

304305
rxe_get(pd);
305306
rxe_get(rcq);
@@ -325,10 +326,10 @@ int rxe_qp_from_init(struct rxe_dev *rxe, struct rxe_qp *qp, struct rxe_pd *pd,
325326
if (err)
326327
goto err2;
327328

328-
spin_lock_bh(&qp->state_lock);
329+
spin_lock_irqsave(&qp->state_lock, flags);
329330
qp->attr.qp_state = IB_QPS_RESET;
330331
qp->valid = 1;
331-
spin_unlock_bh(&qp->state_lock);
332+
spin_unlock_irqrestore(&qp->state_lock, flags);
332333

333334
return 0;
334335

@@ -492,24 +493,28 @@ static void rxe_qp_reset(struct rxe_qp *qp)
492493
/* move the qp to the error state */
493494
void rxe_qp_error(struct rxe_qp *qp)
494495
{
495-
spin_lock_bh(&qp->state_lock);
496+
unsigned long flags;
497+
498+
spin_lock_irqsave(&qp->state_lock, flags);
496499
qp->attr.qp_state = IB_QPS_ERR;
497500

498501
/* drain work and packet queues */
499502
rxe_sched_task(&qp->resp.task);
500503
rxe_sched_task(&qp->comp.task);
501504
rxe_sched_task(&qp->req.task);
502-
spin_unlock_bh(&qp->state_lock);
505+
spin_unlock_irqrestore(&qp->state_lock, flags);
503506
}
504507

505508
static void rxe_qp_sqd(struct rxe_qp *qp, struct ib_qp_attr *attr,
506509
int mask)
507510
{
508-
spin_lock_bh(&qp->state_lock);
511+
unsigned long flags;
512+
513+
spin_lock_irqsave(&qp->state_lock, flags);
509514
qp->attr.sq_draining = 1;
510515
rxe_sched_task(&qp->comp.task);
511516
rxe_sched_task(&qp->req.task);
512-
spin_unlock_bh(&qp->state_lock);
517+
spin_unlock_irqrestore(&qp->state_lock, flags);
513518
}
514519

515520
/* caller should hold qp->state_lock */
@@ -555,14 +560,16 @@ int rxe_qp_from_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask,
555560
qp->attr.cur_qp_state = attr->qp_state;
556561

557562
if (mask & IB_QP_STATE) {
558-
spin_lock_bh(&qp->state_lock);
563+
unsigned long flags;
564+
565+
spin_lock_irqsave(&qp->state_lock, flags);
559566
err = __qp_chk_state(qp, attr, mask);
560567
if (!err) {
561568
qp->attr.qp_state = attr->qp_state;
562569
rxe_dbg_qp(qp, "state -> %s\n",
563570
qps2str[attr->qp_state]);
564571
}
565-
spin_unlock_bh(&qp->state_lock);
572+
spin_unlock_irqrestore(&qp->state_lock, flags);
566573

567574
if (err)
568575
return err;
@@ -688,6 +695,8 @@ int rxe_qp_from_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask,
688695
/* called by the query qp verb */
689696
int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask)
690697
{
698+
unsigned long flags;
699+
691700
*attr = qp->attr;
692701

693702
attr->rq_psn = qp->resp.psn;
@@ -708,12 +717,12 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask)
708717
/* Applications that get this state typically spin on it.
709718
* Yield the processor
710719
*/
711-
spin_lock_bh(&qp->state_lock);
720+
spin_lock_irqsave(&qp->state_lock, flags);
712721
if (qp->attr.sq_draining) {
713-
spin_unlock_bh(&qp->state_lock);
722+
spin_unlock_irqrestore(&qp->state_lock, flags);
714723
cond_resched();
715724
} else {
716-
spin_unlock_bh(&qp->state_lock);
725+
spin_unlock_irqrestore(&qp->state_lock, flags);
717726
}
718727

719728
return 0;
@@ -737,10 +746,11 @@ int rxe_qp_chk_destroy(struct rxe_qp *qp)
737746
static void rxe_qp_do_cleanup(struct work_struct *work)
738747
{
739748
struct rxe_qp *qp = container_of(work, typeof(*qp), cleanup_work.work);
749+
unsigned long flags;
740750

741-
spin_lock_bh(&qp->state_lock);
751+
spin_lock_irqsave(&qp->state_lock, flags);
742752
qp->valid = 0;
743-
spin_unlock_bh(&qp->state_lock);
753+
spin_unlock_irqrestore(&qp->state_lock, flags);
744754
qp->qp_timeout_jiffies = 0;
745755

746756
if (qp_type(qp) == IB_QPT_RC) {

‎drivers/infiniband/sw/rxe/rxe_recv.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ static int check_type_state(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
1414
struct rxe_qp *qp)
1515
{
1616
unsigned int pkt_type;
17+
unsigned long flags;
1718

1819
if (unlikely(!qp->valid))
1920
return -EINVAL;
@@ -38,19 +39,19 @@ static int check_type_state(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
3839
return -EINVAL;
3940
}
4041

41-
spin_lock_bh(&qp->state_lock);
42+
spin_lock_irqsave(&qp->state_lock, flags);
4243
if (pkt->mask & RXE_REQ_MASK) {
4344
if (unlikely(qp_state(qp) < IB_QPS_RTR)) {
44-
spin_unlock_bh(&qp->state_lock);
45+
spin_unlock_irqrestore(&qp->state_lock, flags);
4546
return -EINVAL;
4647
}
4748
} else {
4849
if (unlikely(qp_state(qp) < IB_QPS_RTS)) {
49-
spin_unlock_bh(&qp->state_lock);
50+
spin_unlock_irqrestore(&qp->state_lock, flags);
5051
return -EINVAL;
5152
}
5253
}
53-
spin_unlock_bh(&qp->state_lock);
54+
spin_unlock_irqrestore(&qp->state_lock, flags);
5455

5556
return 0;
5657
}

‎drivers/infiniband/sw/rxe/rxe_req.c‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -99,17 +99,18 @@ static void req_retry(struct rxe_qp *qp)
9999
void rnr_nak_timer(struct timer_list *t)
100100
{
101101
struct rxe_qp *qp = from_timer(qp, t, rnr_nak_timer);
102+
unsigned long flags;
102103

103104
rxe_dbg_qp(qp, "nak timer fired\n");
104105

105-
spin_lock_bh(&qp->state_lock);
106+
spin_lock_irqsave(&qp->state_lock, flags);
106107
if (qp->valid) {
107108
/* request a send queue retry */
108109
qp->req.need_retry = 1;
109110
qp->req.wait_for_rnr_timer = 0;
110111
rxe_sched_task(&qp->req.task);
111112
}
112-
spin_unlock_bh(&qp->state_lock);
113+
spin_unlock_irqrestore(&qp->state_lock, flags);
113114
}
114115

115116
static void req_check_sq_drain_done(struct rxe_qp *qp)
@@ -118,8 +119,9 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
118119
unsigned int index;
119120
unsigned int cons;
120121
struct rxe_send_wqe *wqe;
122+
unsigned long flags;
121123

122-
spin_lock_bh(&qp->state_lock);
124+
spin_lock_irqsave(&qp->state_lock, flags);
123125
if (qp_state(qp) == IB_QPS_SQD) {
124126
q = qp->sq.queue;
125127
index = qp->req.wqe_index;
@@ -140,7 +142,7 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
140142
break;
141143

142144
qp->attr.sq_draining = 0;
143-
spin_unlock_bh(&qp->state_lock);
145+
spin_unlock_irqrestore(&qp->state_lock, flags);
144146

145147
if (qp->ibqp.event_handler) {
146148
struct ib_event ev;
@@ -154,7 +156,7 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
154156
return;
155157
} while (0);
156158
}
157-
spin_unlock_bh(&qp->state_lock);
159+
spin_unlock_irqrestore(&qp->state_lock, flags);
158160
}
159161

160162
static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
@@ -173,20 +175,21 @@ static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
173175
static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
174176
{
175177
struct rxe_send_wqe *wqe;
178+
unsigned long flags;
176179

177180
req_check_sq_drain_done(qp);
178181

179182
wqe = __req_next_wqe(qp);
180183
if (wqe == NULL)
181184
return NULL;
182185

183-
spin_lock_bh(&qp->state_lock);
186+
spin_lock_irqsave(&qp->state_lock, flags);
184187
if (unlikely((qp_state(qp) == IB_QPS_SQD) &&
185188
(wqe->state != wqe_state_processing))) {
186-
spin_unlock_bh(&qp->state_lock);
189+
spin_unlock_irqrestore(&qp->state_lock, flags);
187190
return NULL;
188191
}
189-
spin_unlock_bh(&qp->state_lock);
192+
spin_unlock_irqrestore(&qp->state_lock, flags);
190193

191194
wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
192195
return wqe;
@@ -676,16 +679,17 @@ int rxe_requester(struct rxe_qp *qp)
676679
struct rxe_queue *q = qp->sq.queue;
677680
struct rxe_ah *ah;
678681
struct rxe_av *av;
682+
unsigned long flags;
679683

680-
spin_lock_bh(&qp->state_lock);
684+
spin_lock_irqsave(&qp->state_lock, flags);
681685
if (unlikely(!qp->valid)) {
682-
spin_unlock_bh(&qp->state_lock);
686+
spin_unlock_irqrestore(&qp->state_lock, flags);
683687
goto exit;
684688
}
685689

686690
if (unlikely(qp_state(qp) == IB_QPS_ERR)) {
687691
wqe = __req_next_wqe(qp);
688-
spin_unlock_bh(&qp->state_lock);
692+
spin_unlock_irqrestore(&qp->state_lock, flags);
689693
if (wqe)
690694
goto err;
691695
else
@@ -700,10 +704,10 @@ int rxe_requester(struct rxe_qp *qp)
700704
qp->req.wait_psn = 0;
701705
qp->req.need_retry = 0;
702706
qp->req.wait_for_rnr_timer = 0;
703-
spin_unlock_bh(&qp->state_lock);
707+
spin_unlock_irqrestore(&qp->state_lock, flags);
704708
goto exit;
705709
}
706-
spin_unlock_bh(&qp->state_lock);
710+
spin_unlock_irqrestore(&qp->state_lock, flags);
707711

708712
/* we come here if the retransmit timer has fired
709713
* or if the rnr timer has fired. If the retransmit

0 commit comments

Comments
 (0)