On Wed, Jul 17, 2019 at 10:25:25PM +0300, Shamir Rabinovitch wrote:
> On Wed, Jul 17, 2019 at 08:53:54AM -0300, Jason Gunthorpe wrote:
> > On Tue, Jul 16, 2019 at 09:11:43PM +0300, Shamir Rabinovitch wrote:
> > > From: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> > >
> > > ufile (&ucontext) with the process who own them must not be released
> > > when there are other ufile (&ucontext) that depens at them.
> >
> > We already have a kref, why do we need more? Especially wrongly done
> > refcounts with atomics?
>
> Yes. Will fix in v2.
>
> >
> > Trying to sequence the destroy of the ucontext seems inherently wrong
> > to me. If the driver has to link the PD/MR to data in the ucontext it
> > can't support sharing.
>
> The issue we try to solve here is this:
>
> [process 1] [process 2]
> - alloc mr & point mr to -
> context 1
> - share context -
> - - import mr
> - exit -
> - - exit
> - -- ufile_destroy_ucontext
> - --- driver dereg_mr is called
> - ---- ib_umem_release on umem from
> previously destroyed context 1
Like I said, drivers that require the creating ucontext as part of the
PD and MR cannot support sharing.
> > > + int wait;
> > > +
> > > + if (ufile->parent) {
> > > + pr_debug("%s: release parent ufile. ufile %p parent %p\n",
> > > + __func__, ufile, ufile->parent);
> > > + if (atomic_dec_and_test(&ufile->parent->refcount))
> > > + complete(&ufile->parent->context_released);
> > > + }
> > > +
> > > + if (!atomic_dec_and_test(&ufile->refcount)) {
> > > +wait:
> > > + wait = wait_for_completion_interruptible_timeout(
> > > + &ufile->context_released, 3*HZ);
> > > + if (wait == -ERESTARTSYS) {
> > > + WARN_ONCE(1,
> > > + "signal while waiting for context release! ufile %p\n",
> > > + ufile);
> >
> > ????
> >
> > Jason
>
> I copied the behaviour I saw in the rest of the kernel as for what to do
> when wait_for_completion_interruptible_timeout exit due to interrupt.
It doesn't really make sense here, we can't block release() like this
Jason
