On Thu, Nov 01, 2018 at 12:40:58PM +0200, Shamir Rabinovitch wrote: > On Wed, Oct 31, 2018 at 06:09:28PM +0000, Mark Bloch wrote: > > > [ ... ] > > >> mlx5_ib_dealloc_xrcd(devr->x1); > > >> - mlx5_ib_destroy_cq(devr->c0); > > >> - mlx5_ib_dealloc_pd(devr->p0); > > >> + ib_destroy_cq(devr->c0); > > >> + ib_dealloc_pd(devr->p0); > > > > > > and has been de-registered by this point. > > > > > > Calling ib_* functions on an unregistered device is not a good idea. > > > > > > Mark? > > > > Yep, I don't like it, any change to the create/destroy functions > > will now need to take into account that:"Don't play with struct ib_device too much > > as mlx5 does some funky stuff before the device is registered" > > > > Seems like a bad idea. > > > > > > > > Jason > > > > > > > Mark > > Reason for this is that follow patches will start use the restrack to > figure if pd was created by user or not and this is used even in drivers > such as mlx5. So not having restrack initialized is bad idea also. I can > initialize the restrack here and skip the calls to the ib_x functions. > > Will this be acceptable ? You will need to take into account that restrack DB is exposed through RDMA nldev to rdmatool, but for now, I don't see a problem with it, we have Steve's extension to return vendor specific data for already known objects. So yes, why not? > > I do think that it's bad idea that we have objects that are partially > initialized and that when accessed by driver, driver need to be > careful not to touch the uninitialized parts.
Attachment:
signature.asc
Description: PGP signature
