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.. Jason
