[PATCH] IB/mlx4: fix post_recv wq overflow check

[PATCH] IB/mlx4: fix post_recv wq overflow check

[Or Gerlitz]

the post recv flow should check wq overflow using the recv and not the send cq

Signed-off-by: Or Gerlitz <ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org>

diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 989555c..2a97c96 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -1752,7 +1752,7 @@ int mlx4_ib_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
 	ind = qp->rq.head & (qp->rq.wqe_cnt - 1);

 	for (nreq = 0; wr; ++nreq, wr = wr->next) {
-		if (mlx4_wq_overflow(&qp->rq, nreq, qp->ibqp.send_cq)) {
+		if (mlx4_wq_overflow(&qp->rq, nreq, qp->ibqp.recv_cq)) {
 			err = -ENOMEM;
 			*bad_wr = wr;
 			goto out;
--
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] IB/mlx4: fix post_recv wq overflow check

[Roland Dreier]

thanks, applied.
--
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] IB/mlx4: fix post_recv wq overflow check

[Or Gerlitz]

Roland Dreier wrote:
> thanks, applied.

With this not being a regression, I see that it went into your for-next branch and as such I assume will be available by 2.6.34. Are you fine with the patch going into the -stable series? 

Or.
--
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] IB/mlx4: fix post_recv wq overflow check

[Roland Dreier]

 > With this not being a regression, I see that it went into your
 > for-next branch and as such I assume will be available by 2.6.34. Are
 > you fine with the patch going into the -stable series?

Actually I was planning on sending it for 2.6.33, since it's so small
and obvious and we're reasonable early in the cycle.  Not sure about
-stable though -- has this been hit in practice?

 - R.
--
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] IB/mlx4: fix post_recv wq overflow check

[Or Gerlitz]

Roland Dreier wrote:
> Actually I was planning on sending it for 2.6.33, since it's so small and obvious and we're reasonable early in the cycle.  Not sure about -stable though -- has this been hit in practice?
I agree that it should go into 2.6.33, since its so small there's no 
reason to wait for 2.6.34. As for the being hit question: note that 
without there is both bug in the overflow check and creation of extra 
contention between the post recv and poll send cq flows, for ULPs that 
have their send cq different from the recv cq, e.g IPoIB, I came a cross 
this bug when reviewing the mlx4 posting code when during some profiling.

I wonder if the overflow check could be removed all together and be left 
to the ULP (kernel is trusted environment...) is there any risk in doing 
so? this way the WR posting code will not experience contention with the 
poll WC code on the CQ lock.

Or.
--
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] IB/mlx4: fix post_recv wq overflow check

[Roland Dreier]

 > I wonder if the overflow check could be removed all together and be
 > left to the ULP (kernel is trusted environment...) is there any risk
 > in doing so? this way the WR posting code will not experience
 > contention with the poll WC code on the CQ lock.

We could do that I guess if it's a really big performance gain.  I do
think it is quite common to see this WQ overflow check trigger, even for
kernel code, and so we would have to be sure the performance gain is
worth making these bugs much harder to find and fix.

 - R.
--
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] IB/mlx4: fix post_recv wq overflow check

[Or Gerlitz]

Roland Dreier wrote:
> I do think it is quite common to see this WQ overflow check trigger, even for kernel code
mmm, why is that common? typically there's a higher layer to which the 
IB ULP advertises some sort of maximal number of credits (e.g in the 
SCSI case, iser and srp specify the maximal number of commands in the 
scsi host template) or the ULP informs a higher layer that no more sends 
can be done (e.g IPoIB calling netif_stop_queue once it sense that the 
QP filled, etc).

Or.
--
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] IB/mlx4: fix post_recv wq overflow check

[Roland Dreier]

 > mmm, why is that common? typically there's a higher layer to which the
 > IB ULP advertises some sort of maximal number of credits (e.g in the
 > SCSI case, iser and srp specify the maximal number of commands in the
 > scsi host template) or the ULP informs a higher layer that no more
 > sends can be done (e.g IPoIB calling netif_stop_queue once it sense
 > that the QP filled, etc).

bugs in the ULP.  And it seems to be a common kind of bug.

In other words this check catches common bugs and makes them a gazillion
times easier to find and fix.  So unless the performance impact is
extreme, I'm inclined to leave it.

 - R.
--
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] IB/mlx4: fix post_recv wq overflow check

[Or Gerlitz]

Roland Dreier wrote:
> In other words this check catches common bugs and makes them a gazillion times easier to find and fix.  So unless the performance impact is extreme, I'm inclined to leave it
okay, lets leave this like that for unless someone comes with 
performance data that shows this is really a bottleneck.

Or.
--
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