On Mon, Jan 14, 2019 at 05:36:52PM +0000, Jason Gunthorpe wrote: > On Sun, Jan 13, 2019 at 10:26:27AM +0200, Leon Romanovsky wrote: > > On Sat, Jan 12, 2019 at 05:05:49PM +0000, Jason Gunthorpe wrote: > > > On Sat, Jan 12, 2019 at 05:06:02PM +0200, Leon Romanovsky wrote: > > > > On Fri, Jan 11, 2019 at 07:31:24PM -0700, Jason Gunthorpe wrote: > > > > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > > > > > > > > > Work must hold a kref on the ib_device otherwise the dev pointer can > > > > > become free before the work runs. > > > > > > > > How can it occur exactly? > > > > > > > > "dev" will be released after mlx5_ib will exist, mlx5_ib exist will > > > > drain advise_mr queue before it. Such drain will ensure that no works > > > > are running. > > > > > > Okay, that is fine for the get_device, it can be removed. > > > > > > Are we OK with the reg_state hunk? > > > > I'm not sure about that > > Maybe current code is not nice as you suggested but IB_DEV_REGISTERED > > check also valid approach. > > It is not. Checking the state without holding the appropriate locks > does nothing useful. > > We need to delete the test or properly hold the registration lock. > > > More general, I don't like how IB/core handles states and IMHO we should > > push our stack to be similar to block layer elevator. Block layer implements > > clear stages and calls to callbacks provided by drivers. It removes the need > > to manage and check any state in those drivers. > > I'm not super familiar with the block layer, but I don't think more > callbacks will be in anyway more clear here. We have a very > complicated lifetime model already.. The idea is to provide very clear device state machine which will ensure that once driver was called, IB/core won't change state without need to annotate driver code with various get/put. Thanks > > Jason
Attachment:
signature.asc
Description: PGP signature
