diff mbox series

[for-next] RDMA/rxe: Fix deadlock in rxe_do_local_ops()

Message ID 20220523223251.15350-1-rpearsonhpe@gmail.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series [for-next] RDMA/rxe: Fix deadlock in rxe_do_local_ops() | expand

Commit Message

Bob Pearson May 23, 2022, 10:32 p.m. UTC
When a local operation (invalidate mr, reg mr, bind mw) is finished
there will be no ack packet coming from a responder to cause the
wqe to be completed. This may happen anyway if a subsequent wqe
performs IO. Currently if the wqe is signalled the completer
tasklet is scheduled immediately but not otherwise.

This leads to a deadlock if the next wqe has the fence bit set in
send flags and the operation is not signalled. This patch removes
the condition that the wqe must be signalled in order to schedule
the completer tasklet which is the simplest fix for this deadlock
and is fairly low cost. This is the analog for local operations of
always setting the ackreq bit in all last or only request packets
even if the operation is not signalled.

Reported-by: Jenny Hack <jhack@hpe.com>
Fixes: c1a411268a4b1 ("RDMA/rxe: Move local ops to subroutine")
Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)


base-commit: c5eb0a61238dd6faf37f58c9ce61c9980aaffd7a

Comments

Jason Gunthorpe June 30, 2022, 11:57 p.m. UTC | #1
On Mon, May 23, 2022 at 05:32:52PM -0500, Bob Pearson wrote:
> When a local operation (invalidate mr, reg mr, bind mw) is finished
> there will be no ack packet coming from a responder to cause the
> wqe to be completed. This may happen anyway if a subsequent wqe
> performs IO. Currently if the wqe is signalled the completer
> tasklet is scheduled immediately but not otherwise.
> 
> This leads to a deadlock if the next wqe has the fence bit set in
> send flags and the operation is not signalled. This patch removes
> the condition that the wqe must be signalled in order to schedule
> the completer tasklet which is the simplest fix for this deadlock
> and is fairly low cost. This is the analog for local operations of
> always setting the ackreq bit in all last or only request packets
> even if the operation is not signalled.
> 
> Reported-by: Jenny Hack <jhack@hpe.com>
> Fixes: c1a411268a4b1 ("RDMA/rxe: Move local ops to subroutine")
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_req.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Applied to for-next, thanks

Jason
Pearson, Robert B July 1, 2022, 3:36 p.m. UTC | #2
Thanks Jason.

-----Original Message-----
From: Jason Gunthorpe <jgg@nvidia.com> 
Sent: Thursday, June 30, 2022 6:58 PM
To: Bob Pearson <rpearsonhpe@gmail.com>
Cc: zyjzyj2000@gmail.com; Hack, Jenny (Ft. Collins) <jhack@hpe.com>; Zago, Frank <frank.zago@hpe.com>; linux-rdma@vger.kernel.org
Subject: Re: [PATCH] RDMA/rxe: Fix deadlock in rxe_do_local_ops()

On Mon, May 23, 2022 at 05:32:52PM -0500, Bob Pearson wrote:
> When a local operation (invalidate mr, reg mr, bind mw) is finished 
> there will be no ack packet coming from a responder to cause the wqe 
> to be completed. This may happen anyway if a subsequent wqe performs 
> IO. Currently if the wqe is signalled the completer tasklet is 
> scheduled immediately but not otherwise.
> 
> This leads to a deadlock if the next wqe has the fence bit set in send 
> flags and the operation is not signalled. This patch removes the 
> condition that the wqe must be signalled in order to schedule the 
> completer tasklet which is the simplest fix for this deadlock and is 
> fairly low cost. This is the analog for local operations of always 
> setting the ackreq bit in all last or only request packets even if the 
> operation is not signalled.
> 
> Reported-by: Jenny Hack <jhack@hpe.com>
> Fixes: c1a411268a4b1 ("RDMA/rxe: Move local ops to subroutine")
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_req.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Applied to for-next, thanks

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index ae5fbc79dd5c..ca0b60dd365d 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -586,9 +586,11 @@  static int rxe_do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 	wqe->status = IB_WC_SUCCESS;
 	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
 
-	if ((wqe->wr.send_flags & IB_SEND_SIGNALED) ||
-	    qp->sq_sig_type == IB_SIGNAL_ALL_WR)
-		rxe_run_task(&qp->comp.task, 1);
+	/* There is no ack coming for local work requests
+	 * which can lead to a deadlock. So go ahead and complete
+	 * it now.
+	 */
+	rxe_run_task(&qp->comp.task, 1);
 
 	return 0;
 }