On Wednesday 02 July 2008 18:04, Jack Morgenstein wrote:
> On Tuesday 24 June 2008 00:53, Roland Dreier wrote:
> >  > @@ -769,6 +775,7 @@ struct ib_ucontext {
> >  >          struct list_head        srq_list;
> >  >          struct list_head        ah_list;
> >  >          struct list_head        xrc_domain_list;
> >  > +        struct list_head        xrc_reg_qp_list;
> >  >          int                     closing;
> >  >  };
> > 
> > Wouldn't it be cleaner to keep the like of recv QPs per xrcd?  Then you
> > wouldn't have to do stuff like:
> > 
> >  > +        list_for_each_entry(tmp, &file->ucontext->xrc_reg_qp_list, list)
> >  > +                if (cmd.xrcd_handle == tmp->domain_handle) {
> > 
> > instead you could just do a list_empty() test.
> > 
> Currently, there is no per-process object for xrc domains. I needed a list
> of qp's registered ***per-process***, so that I could prevent closing a 
> domain (for that process)
> while that process still had registered QPs on that domain.  This was 
> especially true if
> the current process was the last one using the domain -- so that the domain 
> itself would be deleted
> when close_domain was invoked.
> 
> With the check in place, if the user tried to close the domain while there 
> were QPs registered,
> he gets an error.  If the user does not close the domain, and does not 
> unregister all the QPs
> for that process, this gets done in ib_uverbs_cleanup_ucontext() (in the 
> proper order), so there
> are no resource leaks.
> 
> I think that here we have "6 of one, half a dozen of the other":
> If I register QPs per xrcd, I either have to create and manage an xrcd
> user-space specific object (e.g., struct ib_uxrcd_object) -- instead of the 
> generic ib_uobject,
> or I need to save the process ID per QP in an xrcd list of QPs -- and then I 
> would need to
> check for equality of PIDs, rather than domain handles.
> 
> I think the way things are now is relatively simpler.

On second thought, I'll look at this again for a day or two.  It is a 
medium-complexity code change, which
needs to be thoroughly tested out.

I'll need to define an ib_uxrcd_object, and place the xrc_reg_qp_list in that 
uobject.  When cleaning up the
process, I'll just need to unregister all the xrc_rcv_qp's in a loop within the 
xrc domain cleanup loop
(i.e.   list_for_each_entry_safe(uobj, tmp, &context->xrc_domain_list, list) {
                struct ib_xrcd *xrcd = uobj->object;
===> xrc_rcv_qp list cleanup here, in a "for each" loop
                idr_remove_uobj(&ib_uverbs_xrc_domain_idr, uobj);
                ib_uverbs_dealloc_xrcd(file->device->ib_dev, xrcd);
                kfree(uobj);
        }
        mutex_unlock(&file->device->ib_dev->xrcd_table_mutex);
)

When registering/unregistering xrc_rcv_qp's, I need to manage the qp list 
within the ib_uxrcd_object.

I do agree that this would be logically cleaner, since this way the receive 
qp's per process xrc domain
are contained in the process' xrc domain object.

- Jack
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to