Hi Jack, Thanks for your note and the suggested changes. I will discuss this with my team members and get back to you with our thoughts next week.
Thanks, DK On Sat, 8 May 2010, Jack Morgenstein wrote: > 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/[email protected]/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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
