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

[Jack Morgenstein]

On Wednesday 16 July 2008 22:12, Roland Dreier wrote
> 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?

Because of the call below to mlx4_ib_create_qp -- I wanted to zero-out the
entire structure to not worry about the ib (core) layer info.
> 
> 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.

In the draft addition to the IB Spec, XRC Target QPs (XRC-RCV qp's here)
do not require CQ's, so we should not require such in the core.

Since in ConnectX we do require every QP to have send and receive CQs, I created,
for each XRC domain, a single CQ for use with XRC RCV qps in that domain, in the
low level driver.

At the time, I thought it best to keep these CQs completely internal to the
low-level driver layer. I will think about having the low-level driver
invoke ib_create_cq() instead of mlx4_ib_create_cq() -- and ditto for PDs.

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

The CQ usage in XRC RCV cq's is HW specific (it is not required by the IB Spec).

- Jack