Re: [PATCH 2/4] ib_core: implement XRC RCV qp's

Jack Morgenstein
2010-05-05T05:36:28Z

On Thursday 22 April 2010 21:03, Roland Dreier wrote:
> So I'm looking at merging this, and I'm wondering about one thing.
> Seems like it's just a mistake but I want to make sure I understand
> properly:
>
> > @@ -1078,6 +1079,7 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
> > goto err_put;
> > }
> >
> > + attr.create_flags = 0;
> > attr.event_handler = ib_uverbs_qp_event_handler;
>
> This looks redundant, because this function already sets create_flags to
> 0 a few lines later. So I think this line is just a remnant from some
> other patch.
You're correct.

>
> But then ib_uverbs_create_xrc_rcv_qp() doesn't set create_flags before
> the call to device->create_xrc_rcv_qp() -- which maybe is OK, since that
> function is not going to look at create_flags right now, but for the
> future we should probably set it to 0, right?
Can't hurt.

> Also it's not 100% clear to me why the low-level driver needs a special
> create_xrc_rcv_qp method, rather than having uverbs just call create_qp
> with the right parameters.
I did not want to have the verbs layer dictate implementation details to the
low-level driver. It is more correct, in my opinion, to have each low-level
driver decide for itself on implementation. Therefore, the separate method.
However, please note that I re-use the qp_create_common() method in
mlx4_ib_create_xrc_rcv_qp.

> But I haven't looked throught carefully to
> see the differences between eg query_xrc_rcv_qp() vs query_qp() methods.
>
Same comment.
> - R.