[v2] RDMA/qedr: Fix wmb usage in qedr

Commit Message

Kalderon, Michal April 5, 2018, 6:59 a.m. UTC

This patch comes as a result of Sinan Kaya's patch:
https://patchwork.kernel.org/patch/10301863/

wmb usages in qedr driver have either been removed
where they were there only to order DMA accesses,
and replaced with smp_wmb and comments for the places
that the barrier was there for SMP reasons.

Signed-off-by: Michal Kalderon <[e-mail]>
Signed-off-by: Ariel Elior <[e-mail]>
---
Changes from v1: 
	- Modified the smp_wmb usage comment to be clearer
---
 drivers/infiniband/hw/qedr/verbs.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

Jason Gunthorpe April 5, 2018, 5:14 p.m. UTC | #1

On Thu, Apr 05, 2018 at 09:59:29AM +0300, Kalderon, Michal wrote:
> This patch comes as a result of Sinan Kaya's patch:
> https://patchwork.kernel.org/patch/10301863/
> 
> wmb usages in qedr driver have either been removed
> where they were there only to order DMA accesses,
> and replaced with smp_wmb and comments for the places
> that the barrier was there for SMP reasons.
> 
> Signed-off-by: Michal Kalderon <[e-mail]>
> Signed-off-by: Ariel Elior <[e-mail]>
> ---
> Changes from v1: 
> 	- Modified the smp_wmb usage comment to be clearer
> ---
>  drivers/infiniband/hw/qedr/verbs.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)

I added a fixes line and resolved the merge conflicts since Sinan's
patch was already merged. Applied to for-next as it fixes a patch
already accepted.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 875b172..450344f 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -856,8 +856,6 @@  static inline void qedr_init_cq_params(struct qedr_cq *cq,
 
 static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 flags)
 {
-	/* Flush data before signalling doorbell */
-	wmb();
 	cq->db.data.agg_flags = flags;
 	cq->db.data.value = cpu_to_le32(cons);
 	writeq(cq->db.raw, cq->db_addr);
@@ -1869,7 +1867,6 @@  static int qedr_update_qp_state(struct qedr_dev *dev,
 			 */
 
 			if (rdma_protocol_roce(&dev->ibdev, 1)) {
-				wmb();
 				writel(qp->rq.db_data.raw, qp->rq.db);
 				/* Make sure write takes effect */
 				mmiowb();
@@ -3256,7 +3253,14 @@  int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 	 * unchanged). For performance reasons we avoid checking for this
 	 * redundant doorbell.
 	 */
-	wmb();
+	/* qp->wqe_wr_id is accessed during qedr_poll_cq, as
+	 * soon as we give the doorbell, we could get a completion
+	 * for this wr, therefore we need to make sure that the
+	 * memory is updated before giving the doorbell.
+	 * During qedr_poll_cq, rmb is called before accessing the
+	 * cqe. This covers for the smp_rmb as well.
+	 */
+	smp_wmb();
 	writel(qp->sq.db_data.raw, qp->sq.db);
 
 	/* Make sure write sticks */
@@ -3343,8 +3347,14 @@  int qedr_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
 
 		qedr_inc_sw_prod(&qp->rq);
 
-		/* Flush all the writes before signalling doorbell */
-		wmb();
+		/* qp->rqe_wr_id is accessed during qedr_poll_cq, as
+		 * soon as we give the doorbell, we could get a completion
+		 * for this wr, therefore we need to make sure that the
+		 * memory is update before giving the doorbell.
+		 * During qedr_poll_cq, rmb is called before accessing the
+		 * cqe. This covers for the smp_rmb as well.
+		 */
+		smp_wmb();
 
 		qp->rq.db_data.data.value++;