> -----Original Message----- > From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > Sent: Tuesday, March 20, 2018 6:59 PM > To: Parav Pandit <parav@xxxxxxxxxxxx> > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Mark Bloch <markb@xxxxxxxxxxxx> > Subject: Re: [PATCH rdma-rc 1/2] RDMA/restrack: Add ability to create non- > traceable restrack objects > > On Tue, Mar 20, 2018 at 11:50:20PM +0000, Parav Pandit wrote: > > > > > > > From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > > > Sent: Tuesday, March 20, 2018 5:37 PM > > > To: Parav Pandit <parav@xxxxxxxxxxxx> > > > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford > > > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; > RDMA > > > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Mark Bloch > > > <markb@xxxxxxxxxxxx> > > > Subject: Re: [PATCH rdma-rc 1/2] RDMA/restrack: Add ability to > > > create non- traceable restrack objects > > > > > > On Tue, Mar 20, 2018 at 07:19:28PM +0000, Parav Pandit wrote: > > > > > > > rdma_restrack_clean() call from ib_unregister_device() is not correct. > > > > rdma_restrack_init() is called in ib_alloc_device(). > > > > The correct cleanup routine of alloc_device() is ib_dealloc_device(). > > > > Therefore rdma_restrack_clean() should be done there. > > > > Ib_unregister_device() is not the right place regardless of what > > > > is done in this > > > patch. > > > > > > Not really.. It is a serious error for IB objects to still exist > > > once the client remove callbacks have completed, after that point > > > the driver is gone and the objects won't work right. > > > > > IB resources being open after it is unregistered is a hack being > > promoted here. > > Well, I won't disagree with that.. But that is a problem with mlx5's design, not a > core problem that would motivate moving the check outside of unregister. > > > Right way to do ib core to have open() and close() callback. > > Probably.. But since we already accepted the patches that cause this bug for this > merge window we may be stuck accepting a hack. > > The hack is to let mlx5 opt out of resource tracking for it's internal objects. There are some patches from Steve that I didn't review deeply, but those patches let some internal data structure of the provider driver to be exposed to user for debugging purpose, which is very useful. Given UMR QP is one such core QP that might need debugging as well in future and some other internal UD QPs that Bodong is working on. Given that it might be useful to debug such internal QP as well. Having them resource tracked is useful instead of opting out. So not tracking them further creates blockers to not able to debug them in future. Once open() and close() are in place, restack_clean() in dealloc_device() would be sufficient too. > > At least that way we still protect the ULPs on other non-broken drivers. Not sure if any other drivers have dangling resources after ib_unregister_device() is called. Otherwise Leon' patch would have notrack() API invoked in other provider drivers too. > > Maybe restrack_init should be moved to the register as well in this patch? That alleast avoids the confusing code. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
