> -----Original Message----- > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- > owner@xxxxxxxxxxxxxxx> On Behalf Of Jason Gunthorpe > Sent: Saturday, January 12, 2019 11:06 AM > To: Leon Romanovsky <leon@xxxxxxxxxx> > Cc: linux-rdma@xxxxxxxxxxxxxxx; Moni Shoua <monis@xxxxxxxxxxxx> > Subject: Re: [PATCH for-rc 2/2] IB/mlx5: Fix how advise_mr() launches async > work > > 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? > > Jason Unrelated to your question, I noticed that advise_mr does uverbs_attr_ptr_get_array_size() for the SGEs. I think this is incorrect. It should have been mr handles whose ownership can be check against the PID or ucontext that registered those MRs. With ib_sge, now one user can start page faulting MRs of another process of another user? How is this protected? Uverbs shouldn't be deriving uverbs handle from mr key of ib_sge. Who enqueues work to dev->advise_mr_wq? schedule_work() in mlx5_ib_advise_mr_prefetch() does to global workqueue. It should be done to advise_mr_wq(). This will give chance to flush the wq when IB device is unregistered by the core.
