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

[Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]

Dr. Panda, Jeff, and Ishai,

We are trying to get XRC integrated into the next mainstream kernel.

For the kernel submission, I added a destroy_xrc_rcv_qp method (to be
used if the application did not require persistence of the xrc_rcv qp
after the creating process terminated -- per Diego Copernicoff's request).
This did not affect the core API of create/modify/unreg that you have
been using until now.

However, even without the new destroy method (as I suggest below),
having the creating process call unreg is still a bit counterintuitive,
since it calls create, and registration is a side-effect.

Roland is now intensively reviewing the XRC patches, and a made suggestion
to simplify the API which Tziporet and I agree with (see Roland's comments below).

Please comment on this suggestion (which is to have reg_xrc_rcv_qp do create
as well).
This is a minor change, that would require two changes in your current calls:
1. Instead of calling create_xrc_rcv_qp(), as is done currently, MPI would call
   u32 qp_num = 0xFFFFFFFF;
	err = reg_xrc_rcv_qp(xrcd, &qp_num);
   and would have the created qp number returned in qp_num;
   (the qp_init attributes in the old create_xrc_rcv_qp are all ignored except for
   the xrc domain handle anyway)

2. instead of calling reg_xrc_rcv_qp(xrcd, qp_num), you would need to set the
   qp number in a u32 variable, and call reg_xrc_rcv_qp(xrcd, &qp_num).

The other xrc_rcv_qp verbs would work as they work now.

Regarding OFED, this change would not affect OFED 1.5.x ; it would only enter
OFED at 1.6.x.

Please comment.

-Jack

P.S. You can see the submission/discussion of XRC starting at:
	http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg02792.html
On Thursday 06 May 2010 01:40, Roland Dreier wrote:
>  > > I don't really understand the semantics here.  What is supposed to
>  > > happen if I do create/reg/destroy?> What happens if one process does 
>  > > destroy while another process is still registered?
> 
>  > Maybe we can simply assert that the unreg IS the destroy method of the
>  > IB_SPEC, and get rid of the destroy method.
>  > 
>  > The xrc target qp section of the spec was not written with QP persistence
>  > (after the creating process exited) in mind.  That requirement surfaced
>  > at the last minute as a result of testing by the MPI community during the
>  > implementation phase (as far as I know).  Unfortunately, this created
>  > a semantic problem.
> 
> Yes, I think we should try to simplify things here.
> 
> It's very unfortunate to diverge from the API that's been shipped for a
> while now, but I really think we don't want all these different ways of
> saying the same thing, with little difference between create and reg,
> and between destroy and unreg.
> 
> In fact the smallest possible API would be just
> 
>   register_xrc_rcv_qp(xrcd, *qp_num)
> 
> where the user can pass in an invalid qp_num (say, -1 aka ffffffff) and
> have a new QP created, or a valid one to take a new ref on the existing
> rcv QP, and
> 
>   unregister_xrc_rcv_qp(xrcd, qp_num).
> 
> (along these lines, the structure in these patches:
> 
> +struct ib_uverbs_create_xrc_rcv_qp {
> +	__u64 response;
> +	__u64 user_handle;
> +	__u32 xrcd_handle;
> +	__u32 max_send_wr;
> +	__u32 max_recv_wr;
> +	__u32 max_send_sge;
> +	__u32 max_recv_sge;
> +	__u32 max_inline_data;
> +	__u8  sq_sig_all;
> +	__u8  qp_type;
> +	__u8  reserved[6];
> +	__u64 driver_data[0];
> +};
> 
> has many fields we don't need.  Pretty much all the fields after
> xrcd_handle are ignored, except sq_sig_all is used -- and that is highly
> dubious since the rcv QP has no SQ!  So I would propose something like
> just having:
> 
> +struct ib_uverbs_reg_xrc_rcv_qp {
> +	__u64 response;
> +	__u32 xrcd_handle;
> +	__u32 qp_num;
> +	__u64 driver_data[0];
> +};
> 
> where response is used to pass back the qp_num in the create case.
> 
> And then we just have unreg_xrc_rcv_qp and no destroy method (since they
> are synonymous anyway).
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html