[ofw] rev 931 of al_pd.c

Fab Tillier ftillier at windows.microsoft.com
Wed Feb 13 09:58:19 PST 2008


Hi Leonid,

I was looking over the code and noticed a change in al_pd.c that you made in revision 931.

What was the motivation for the change?  The original code checked that one of either the send or receive queue depth was non-zero (preventing creating a QP with zero send and zero receive WRs), and checked that one of either the number of send or receive SGEs was non-zero.

Your change changed these to an OR rather than an AND, so that if either queue depth was zero the call would fail (though having zero receive WRs is a valid configuration if you have > 0 send WRs, and vice versa), and then commented the whole block out with a #if 0.

What motivated this change?  Did you run into an issue where the check was failing a valid QP creation check?

Note that the second check (for SGEs) is probably not needed.  I can imagine someone using a QP for signaling events only, and not moving any data (though such use seems silly).

Here's the code change for revision 931:

Index: core/al/al_pd.c
===================================================================
--- core/al/al_pd.c     (revision 930)
+++ core/al/al_pd.c     (revision 931)
@@ -334,17 +334,23 @@
                return IB_INVALID_PD_HANDLE;
        }

-       if( !p_qp_create->rq_depth &&  !p_qp_create->sq_depth )
+#if 0
+       /* seems like no need in these checks */
+       if( !p_qp_create->rq_depth || !p_qp_create->sq_depth )
        {
-               AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_MAX_WRS\n") );
+               AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_MAX_WRS (%d:%d)\n",
+                       p_qp_create->rq_depth, p_qp_create->sq_depth ) );
                return IB_INVALID_MAX_WRS;
        }
-       if( !p_qp_create->sq_sge && !p_qp_create->sq_sge)
+
+       if( !p_qp_create->rq_sge || !p_qp_create->sq_sge)
        {
-               AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_MAX_SGE\n") );
+               AL_PRINT_EXIT( TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("IB_INVALID_MAX_SGE (%d:%d)\n",
+                       p_qp_create->rq_sge, p_qp_create->sq_sge ) );
                return IB_INVALID_MAX_SGE;
+       }
+#endif

-       }
        if (h_pd->obj.p_ci_ca && h_pd->obj.p_ci_ca->p_pnp_attr)
        {
                if ((p_qp_create->rq_depth > h_pd->obj.p_ci_ca->p_pnp_attr->max_wrs) ||
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080213/ec11cb65/attachment.html>


More information about the ofw mailing list