diff mbox

[resend] rxe: fix broken receive queue draining

Message ID 1498555178-12106-1-git-send-email-sagi@grimberg.me (mailing list archive)
State Accepted
Headers show

Commit Message

Sagi Grimberg June 27, 2017, 9:19 a.m. UTC
From: Vijay Immanuel <vijayi@attalasystems.com>

If we modified the qp to ERROR state, and
drained the recieve queue, post_recv must
trigger the responder task to complete
the drain work request.

Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Signed-off-by: Vijay Immanuel <vijayi@attalasystems.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/infiniband/sw/rxe/rxe_resp.c  | 3 +++
 drivers/infiniband/sw/rxe/rxe_verbs.c | 3 +++
 2 files changed, 6 insertions(+)

Comments

Leon Romanovsky June 28, 2017, 10:07 a.m. UTC | #1
On Tue, Jun 27, 2017 at 12:19:38PM +0300, Sagi Grimberg wrote:
> From: Vijay Immanuel <vijayi@attalasystems.com>
>
> If we modified the qp to ERROR state, and
> drained the recieve queue, post_recv must
> trigger the responder task to complete
> the drain work request.
>
> Cc: Bart Van Assche <bart.vanassche@sandisk.com>
> Signed-off-by: Vijay Immanuel <vijayi@attalasystems.com>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c  | 3 +++
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 3 +++
>  2 files changed, 6 insertions(+)
>
It looks OK for me.

Moni ?

> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 23039768f541..f66b48c114ac 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -1217,6 +1217,9 @@ void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
>  		kfree_skb(skb);
>  	}
>
> +	if (notify)
> +		return;
> +
>  	while (!qp->srq && qp->rq.queue && queue_head(qp->rq.queue))
>  		advance_consumer(qp->rq.queue);
>  }
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 83d709e74dfb..94af4d67988b 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -919,6 +919,9 @@ static int rxe_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
>
>  	spin_unlock_irqrestore(&rq->producer_lock, flags);
>
> +	if (qp->resp.state == QP_STATE_ERROR)
> +		rxe_run_task(&qp->resp.task, 1);
> +
>  err1:
>  	return err;
>  }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bart Van Assche July 11, 2017, 10:42 p.m. UTC | #2
On Tue, 2017-06-27 at 12:19 +0300, Sagi Grimberg wrote:
> From: Vijay Immanuel <vijayi@attalasystems.com>
> 
> If we modified the qp to ERROR state, and
> drained the recieve queue, post_recv must
              ^^^^^^^
              receive?
> trigger the responder task to complete
> the drain work request.
> 
> Cc: Bart Van Assche <bart.vanassche@sandisk.com>
> Signed-off-by: Vijay Immanuel <vijayi@attalasystems.com>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c  | 3 +++
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 23039768f541..f66b48c114ac 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -1217,6 +1217,9 @@ void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
>  		kfree_skb(skb);
>  	}
>  
> +	if (notify)
> +		return;
> +
>  	while (!qp->srq && qp->rq.queue && queue_head(qp->rq.queue))
>  		advance_consumer(qp->rq.queue);

Since an "if (...) return" statement is added, please consider moving the
qp->srq test out of the while-loop and into the if-expression. Anyway:

Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford July 22, 2017, 6:14 p.m. UTC | #3
On 6/27/2017 5:19 AM, Sagi Grimberg wrote:
> From: Vijay Immanuel <vijayi@attalasystems.com>
> 
> If we modified the qp to ERROR state, and
> drained the recieve queue, post_recv must
> trigger the responder task to complete
> the drain work request.
> 
> Cc: Bart Van Assche <bart.vanassche@sandisk.com>
> Signed-off-by: Vijay Immanuel <vijayi@attalasystems.com>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>

This was accepted into 4.13-rc, thanks.
diff mbox

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 23039768f541..f66b48c114ac 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1217,6 +1217,9 @@  void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
 		kfree_skb(skb);
 	}
 
+	if (notify)
+		return;
+
 	while (!qp->srq && qp->rq.queue && queue_head(qp->rq.queue))
 		advance_consumer(qp->rq.queue);
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 83d709e74dfb..94af4d67988b 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -919,6 +919,9 @@  static int rxe_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
 
 	spin_unlock_irqrestore(&rq->producer_lock, flags);
 
+	if (qp->resp.state == QP_STATE_ERROR)
+		rxe_run_task(&qp->resp.task, 1);
+
 err1:
 	return err;
 }