> -----Original Message----- > From: Leon Romanovsky [mailto:leon@xxxxxxxxxx] > Sent: Wednesday, March 21, 2018 12:30 AM > To: Jason Gunthorpe <jgg@xxxxxxxx> > Cc: Parav Pandit <parav@xxxxxxxxxxxx>; Doug Ledford > <dledford@xxxxxxxxxx>; 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:30:08PM -0600, Jason Gunthorpe wrote: > > On Wed, Mar 21, 2018 at 12:11:59AM +0000, Parav Pandit wrote: > > > > > > > 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. > > > > Yes, that seems to be likely. Someone will ultimately have to add new > > methods and revise mlx5. It really shouldn't be creating resources > > prior to registering.. > > There is main difference between Steve's patches and UMR flows. Steve is > exposing extra information for standard QPs which are created by ib_core. > > UMR QPs are something different, they are created by mlx5_ib and ib_core UMR QP is of type IB_QPT_RESERVED1 defined by ib_core. > doesn't aware of them, for example their type is something different from > known to rdmatool and ib_core. > > Mark and me discussed the solution to the current situation and proposed > current patch. We are all agree that UMR flow needs to be revisited and ideal > solution will be create symmetrical create/destroy UMR flows. > However we don't have clear vision how to do it in -rc6. > > This is why it is safe for now to skip UMR completely from restrack. restack_clean() in dealloc_device() eliminates this issue if for-rc is the only concern from the response it seems. > > > > > > > At least that way we still protect the ULPs on other non-broken drivers. All provider 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. > > > > It is more the ULPs that worry me. ULPs leaving dangling resources > > after client unregister seems kind of likely. Given that ib_dealloc_device() still catches those ULP errors. Most drivers call ib_dealloc_device() soon after ib_unregister_device(). > > > > > > Maybe restrack_init should be moved to the register as well in this patch? > > > > > That alleast avoids the confusing code. > > I may admit that names are confusing, but the place is right, ib_alloc_device is > responsible to initialize various structures and this is exactly what restrack_init > does. > > The rdma_restrack_clean() will be renamed to something like > rdma_restrack_check_leaks() in near future, because I see a need to rewrite the > printed output from that function to make debug more sane. > > Jason -- 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
