[PATCH 0/5] qedr: QP and CQ fixes for 4.10

[PATCH 0/5] qedr: QP and CQ fixes for 4.10

[Ram Amrani]

From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

This series contains QP and CQ related fixes for 4.10.

Built against origin/k.o/for-4.10, 5cc8fab.

Ram Amrani (5):
  qedr: return success when changing from QP state X to X
  qedr: return maximum inline data capability on QP query
  qedr: remove CQ spinlock from RDMA CM completion handlers
  qedr: avoid printing error messages for flushed CQEs if QP in error
    state
  qedr: fail QP state transition from error to reset if SQ/RQ are not
    empty

 drivers/infiniband/hw/qedr/qedr_cm.c | 10 ++--------
 drivers/infiniband/hw/qedr/verbs.c   | 19 ++++++++++++++-----
 2 files changed, 16 insertions(+), 13 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] qedr: return success when changing from QP state X to X

[Ram Amrani]

From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/qedr/verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 57c8de2..af6074d 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -1657,7 +1657,7 @@ static int qedr_update_qp_state(struct qedr_dev *dev,
 	int status = 0;
 
 	if (new_state == qp->state)
-		return 1;
+		return 0;
 
 	switch (qp->state) {
 	case QED_ROCE_QP_STATE_RESET:
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] qedr: return maximum inline data capability on QP query

[Ram Amrani]

From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/qedr/verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index af6074d..e843501 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -2028,7 +2028,7 @@ int qedr_query_qp(struct ib_qp *ibqp,
 	qp_attr->cap.max_recv_wr = qp->rq.max_wr;
 	qp_attr->cap.max_send_sge = qp->sq.max_sges;
 	qp_attr->cap.max_recv_sge = qp->rq.max_sges;
-	qp_attr->cap.max_inline_data = qp->max_inline_data;
+	qp_attr->cap.max_inline_data = ROCE_REQ_MAX_INLINE_DATA_SIZE;
 	qp_init_attr->cap = qp_attr->cap;
 
 	memcpy(&qp_attr->ah_attr.grh.dgid.raw[0], &params.dgid.bytes[0],
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] qedr: remove CQ spinlock from RDMA CM completion handlers

[Ram Amrani]

From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

There is only a single event queue that triggers the completion
events for the RDMA CM and it is being processed serially. This means
that inherently there can no parallelism of CQ completion handler.
Hence the lock is redundant.

Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/qedr/qedr_cm.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/qedr_cm.c b/drivers/infiniband/hw/qedr/qedr_cm.c
index 63890eb..00361f3 100644
--- a/drivers/infiniband/hw/qedr/qedr_cm.c
+++ b/drivers/infiniband/hw/qedr/qedr_cm.c
@@ -87,11 +87,8 @@ void qedr_ll2_tx_cb(void *_qdev, struct qed_roce_ll2_packet *pkt)
 	qedr_inc_sw_gsi_cons(&qp->sq);
 	spin_unlock_irqrestore(&qp->q_lock, flags);
 
-	if (cq->ibcq.comp_handler) {
-		spin_lock_irqsave(&cq->comp_handler_lock, flags);
+	if (cq->ibcq.comp_handler)
 		(*cq->ibcq.comp_handler) (&cq->ibcq, cq->ibcq.cq_context);
-		spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
-	}
 }
 
 void qedr_ll2_rx_cb(void *_dev, struct qed_roce_ll2_packet *pkt,
@@ -113,11 +110,8 @@ void qedr_ll2_rx_cb(void *_dev, struct qed_roce_ll2_packet *pkt,
 
 	spin_unlock_irqrestore(&qp->q_lock, flags);
 
-	if (cq->ibcq.comp_handler) {
-		spin_lock_irqsave(&cq->comp_handler_lock, flags);
+	if (cq->ibcq.comp_handler)
 		(*cq->ibcq.comp_handler) (&cq->ibcq, cq->ibcq.cq_context);
-		spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
-	}
 }
 
 static void qedr_destroy_gsi_cq(struct qedr_dev *dev,
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] qedr: remove CQ spinlock from RDMA CM completion handlers

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]

On Mon, Jan 23, 2017 at 05:26:25PM +0200, Ram Amrani wrote:
> From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>
> There is only a single event queue that triggers the completion
> events for the RDMA CM and it is being processed serially. This means
> that inherently there can no parallelism of CQ completion handler.
> Hence the lock is redundant.
>
> Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/hw/qedr/qedr_cm.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)

Don't you need to remove this lock from struct qedr_cq too?

>
> diff --git a/drivers/infiniband/hw/qedr/qedr_cm.c b/drivers/infiniband/hw/qedr/qedr_cm.c
> index 63890eb..00361f3 100644
> --- a/drivers/infiniband/hw/qedr/qedr_cm.c
> +++ b/drivers/infiniband/hw/qedr/qedr_cm.c
> @@ -87,11 +87,8 @@ void qedr_ll2_tx_cb(void *_qdev, struct qed_roce_ll2_packet *pkt)
>  	qedr_inc_sw_gsi_cons(&qp->sq);
>  	spin_unlock_irqrestore(&qp->q_lock, flags);
>
> -	if (cq->ibcq.comp_handler) {
> -		spin_lock_irqsave(&cq->comp_handler_lock, flags);
> +	if (cq->ibcq.comp_handler)
>  		(*cq->ibcq.comp_handler) (&cq->ibcq, cq->ibcq.cq_context);
> -		spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
> -	}
>  }
>
>  void qedr_ll2_rx_cb(void *_dev, struct qed_roce_ll2_packet *pkt,
> @@ -113,11 +110,8 @@ void qedr_ll2_rx_cb(void *_dev, struct qed_roce_ll2_packet *pkt,
>
>  	spin_unlock_irqrestore(&qp->q_lock, flags);
>
> -	if (cq->ibcq.comp_handler) {
> -		spin_lock_irqsave(&cq->comp_handler_lock, flags);
> +	if (cq->ibcq.comp_handler)
>  		(*cq->ibcq.comp_handler) (&cq->ibcq, cq->ibcq.cq_context);
> -		spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
> -	}
>  }
>
>  static void qedr_destroy_gsi_cq(struct qedr_dev *dev,
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 3/5] qedr: remove CQ spinlock from RDMA CM completion handlers

[Amrani, Ram]

> Don't you need to remove this lock from struct qedr_cq too?

The CQ lock has other uses so it cannot be deleted.

Thanks,
Ram

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] qedr: remove CQ spinlock from RDMA CM completion handlers

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On Tue, Jan 24, 2017 at 09:02:55AM +0000, Amrani, Ram wrote:
> > Don't you need to remove this lock from struct qedr_cq too?
>
> The CQ lock has other uses so it cannot be deleted.

What do I miss here?

Your patch removed all usages of this lock.

➜  linux-rdma git:(master) grep -rI comp_handler_lock drivers/infiniband/hw/qedr/*
drivers/infiniband/hw/qedr/qedr_cm.c: spin_lock_irqsave(&cq->comp_handler_lock, flags);
drivers/infiniband/hw/qedr/qedr_cm.c: spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
drivers/infiniband/hw/qedr/qedr_cm.c: spin_lock_irqsave(&cq->comp_handler_lock, flags);
drivers/infiniband/hw/qedr/qedr_cm.c: spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
drivers/infiniband/hw/qedr/qedr.h:	spinlock_t comp_handler_lock;


>
> Thanks,
> Ram
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 3/5] qedr: remove CQ spinlock from RDMA CM completion handlers

[Amrani, Ram]

> On Tue, Jan 24, 2017 at 09:02:55AM +0000, Amrani, Ram wrote:
> > > Don't you need to remove this lock from struct qedr_cq too?
> >
> > The CQ lock has other uses so it cannot be deleted.
> 
> What do I miss here?
> 
> Your patch removed all usages of this lock.
> 
> ➜  linux-rdma git:(master) grep -rI comp_handler_lock drivers/infiniband/hw/qedr/*
> drivers/infiniband/hw/qedr/qedr_cm.c: spin_lock_irqsave(&cq->comp_handler_lock, flags);
> drivers/infiniband/hw/qedr/qedr_cm.c: spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
> drivers/infiniband/hw/qedr/qedr_cm.c: spin_lock_irqsave(&cq->comp_handler_lock, flags);
> drivers/infiniband/hw/qedr/qedr_cm.c: spin_unlock_irqrestore(&cq->comp_handler_lock, flags);
> drivers/infiniband/hw/qedr/qedr.h:	spinlock_t comp_handler_lock;
> 
> 

Yeah, you are right. I see now the other functionality isn't there.
I will re-issue the series.
Thanks!

Ram

[PATCH 4/5] qedr: avoid printing error messages for flushed CQEs if QP in error state

[Ram Amrani]

From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

It is normal to flush CQEs if the QP is in error state. Hence there's no
use in printing a message per CQE to dmesg.

Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/qedr/verbs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index e843501..564080a 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -3234,9 +3234,10 @@ static int qedr_poll_cq_req(struct qedr_dev *dev,
 				  IB_WC_SUCCESS, 0);
 		break;
 	case RDMA_CQE_REQ_STS_WORK_REQUEST_FLUSHED_ERR:
-		DP_ERR(dev,
-		       "Error: POLL CQ with RDMA_CQE_REQ_STS_WORK_REQUEST_FLUSHED_ERR. CQ icid=0x%x, QP icid=0x%x\n",
-		       cq->icid, qp->icid);
+		if (qp->state != QED_ROCE_QP_STATE_ERR)
+			DP_ERR(dev,
+			       "Error: POLL CQ with RDMA_CQE_REQ_STS_WORK_REQUEST_FLUSHED_ERR. CQ icid=0x%x, QP icid=0x%x\n",
+			       cq->icid, qp->icid);
 		cnt = process_req(dev, qp, cq, num_entries, wc, req->sq_cons,
 				  IB_WC_WR_FLUSH_ERR, 1);
 		break;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] qedr: fail QP state transition from error to reset if SQ/RQ are not empty

[Ram Amrani]

From: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/qedr/verbs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 564080a..189f9bd 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -1733,6 +1733,14 @@ static int qedr_update_qp_state(struct qedr_dev *dev,
 		/* ERR->XXX */
 		switch (new_state) {
 		case QED_ROCE_QP_STATE_RESET:
+			if ((qp->rq.prod != qp->rq.cons) ||
+			    (qp->sq.prod != qp->sq.cons)) {
+				DP_NOTICE(dev,
+					  "Error->Reset with rq/sq not empty rq.prod=%x rq.cons=%x sq.prod=%x sq.cons=%x\n",
+					  qp->rq.prod, qp->rq.cons, qp->sq.prod,
+					  qp->sq.cons);
+				status = -EINVAL;
+			}
 			break;
 		default:
 			status = -EINVAL;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html