[ofa-general] Re: wmb missing in libmthca?

[Roland Dreier]

 > Roland, I see this in kernel:
 > 
 >                 ((struct mthca_next_seg *) prev_wqe)->nda_op =
 >                         cpu_to_be32((ind << qp->rq.wqe_shift) | 1);
 >                 wmb();
 >                 ((struct mthca_next_seg *) prev_wqe)->ee_nds =
 >                         cpu_to_be32(MTHCA_NEXT_DBD | size);
 > 
 > but userspace does not have wmb here.
 > Is it needed?

It does seem that way -- otherwise the hardware might read prev_wqe
and see the ee_nds field as set before the nda_op field has the right
variable.  Does this look right to you as a libmthca fix?

diff --git a/src/qp.c b/src/qp.c
index 2d03d49..2ea9dc0 100644
--- a/src/qp.c
+++ b/src/qp.c
@@ -292,7 +292,10 @@ int mthca_tavor_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 			htonl(((ind << qp->sq.wqe_shift) +
 			       qp->send_wqe_offset) |
 			      mthca_opcode[wr->opcode]);
-
+		/*
+		 * Make sure that nda_op is written before setting ee_nds.
+		 */
+		wmb();
 		((struct mthca_next_seg *) prev_wqe)->ee_nds =
 			htonl((size0 ? 0 : MTHCA_NEXT_DBD) | size |
 			((wr->send_flags & IBV_SEND_FENCE) ?