[ofa-general] Re: [PATCH 11/12 V2] mlx4: XRC receive-only QPs

[Roland Dreier]

I have one very specific question:

 > --- a/drivers/infiniband/hw/mlx4/cq.c
 > +++ b/drivers/infiniband/hw/mlx4/cq.c
 > @@ -174,7 +174,7 @@ struct ib_cq *mlx4_ib_create_cq(struct ib_device *ibdev, int entries, int vector
 >  	if (entries < 1 || entries > dev->dev->caps.max_cqes)
 >  		return ERR_PTR(-EINVAL);
 >  
 > -	cq = kmalloc(sizeof *cq, GFP_KERNEL);
 > +	cq = kzalloc(sizeof *cq, GFP_KERNEL);
 >  	if (!cq)
 >  		return ERR_PTR(-ENOMEM);

why is this change made in this patch?

And a more general question: why do these kernel receive-only QPs affect
device-dependent low-level driver code at all.  This whole patch seems
to be just doing bookkeeping that seems like it could be done in generic
code, and it leads to layering problems like:

+	cq = mlx4_ib_create_cq(ibdev, 1, 0, NULL, NULL);
+	if (IS_ERR(cq)) {
+		err = PTR_ERR(cq);
+		goto err_cq;
+	}
+	cq->device        = ibdev;
+	cq->comp_handler  = mlx4_dummy_comp_handler;

ie mlx4 ends up duplicating code from ib_create_cq() for no reason I can
understand.

I'm probably missing something but why can't all or at least most of
this be done in the generic IB code?  I don't see anything related to
device HW interface anywhere in this patch.  Create a new
ib_xrc_kern_recv_qp structure or other helpers if need be I guess.

 - R.