On Wednesday 23 August 2006 23:25, Roland Dreier wrote:
> > 5. Return the send_cq, receive cq and srq handles. ib_query_qp() needs
> them
> > (required by IB Spec). ibv_query_qp() overwrites these values in
> user-space
> > with appropriate user-space values.
>
> > + qp_init_attr->send_cq = ibqp->send_cq;
> > + qp_init_attr->recv_cq = ibqp->recv_cq;
> > + qp_init_attr->srq = ibqp->srq;
>
> I really disagree with this change. It's silly to do this copying
> since the consumer already has the ibqp pointer. And it's especially
> silly to put this in a low-level driver, since there's nothing
> device-specific about it.
>
Note that the same thing is done in user-space (albeit in the verbs layer):
libibverbs/src/cmd.c, lines 678 ff. (procedure ibv_cmd_query_qp() ):
init_attr->send_cq = qp->send_cq;
init_attr->recv_cq = qp->recv_cq;
init_attr->srq = qp->srq;
Either return these values in both kernel and user, or remove them from
the user-space verb.
Regarding putting this in the low level kernel driver, I agree.
A patch for core/verbs.c is given below.
(P.S. IMHO, the correct thing to do is to remove the above lines from cmd.c,
and not return to the user stuff that s/he already has available).
- Jack
-------------
Index: ofed_1_1/drivers/infiniband/core/verbs.c
===================================================================
--- ofed_1_1.orig/drivers/infiniband/core/verbs.c 2006-08-03
14:30:21.000000000 +0300
+++ ofed_1_1/drivers/infiniband/core/verbs.c 2006-08-24 10:03:51.831333000
+0300
@@ -556,9 +556,17 @@ int ib_query_qp(struct ib_qp *qp,
int qp_attr_mask,
struct ib_qp_init_attr *qp_init_attr)
{
- return qp->device->query_qp ?
- qp->device->query_qp(qp, qp_attr, qp_attr_mask, qp_init_attr) :
- -ENOSYS;
+ int rc = -ENOSYS;
+ if (qp->device->query_qp) {
+ rc = qp->device->query_qp(qp, qp_attr,
+ qp_attr_mask, qp_init_attr);
+ if(!rc) {
+ qp_init_attr->recv_cq = qp->recv_cq;
+ qp_init_attr->send_cq = qp->send_cq;
+ qp_init_attr->srq = qp->srq;
+ }
+ }
+ return rc;
}
EXPORT_SYMBOL(ib_query_qp);
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
