Re: [RFC v3 4/5] KVM: add ioregionfd context

Elena Afanasova
2021-03-17T10:48:15Z

On Thu, 2021-03-11 at 11:04 +0800, Jason Wang wrote:
>
> On 2021/3/11 12:41 上午, Elena Afanasova wrote:
> > On Wed, 2021-03-10 at 15:11 +0100, Paolo Bonzini wrote:
> > > On 10/03/21 14:20, Elena Afanasova wrote:
> > > > On Tue, 2021-03-09 at 09:01 +0100, Paolo Bonzini wrote:
> > > > > On 09/03/21 08:54, Jason Wang wrote:
> > > > > > > + return;
> > > > > > > +
> > > > > > > + spin_lock(&ctx->wq.lock);
> > > > > > > + wait_event_interruptible_exclusive_locked(ctx->wq,
> > > > > > > !ctx-
> > > > > > > > busy);
> > > > > >
> > > > > > Any reason that a simple mutex_lock_interruptible() can't
> > > > > > work
> > > > > > here?
> > > > >
> > > > > Or alternatively why can't the callers just take the
> > > > > spinlock.
> > > > >
> > > >
> > > > I'm not sure I understand your question. Do you mean why locked
> > > > version
> > > > of wait_event() is used?
> > >
> > > No, I mean why do you need to use ctx->busy and wait_event,
> > > instead
> > > of
> > > operating directly on the spinlock or on a mutex.
> > >
> >
> > When ioregionfd communication is interrupted by a signal
> > ioctl(KVM_RUN)
> > has to return to userspace. I'm not sure it's ok to do that with
> > the
> > spinlock/mutex being held.
>
>
> So you don't answer my question. Why you can't use
> mutex_lock_interruptible() here?
>
> It looks can do exactly what you want here.
>
> Thanks
>
I think mutex could work here. I used it for the first implementation.
But is it ok to hold a mutex on kernel->user transitions? Is it correct
pattern in this case?
If ioctl returns to userspace and then ioregionfd is deleted or vcpu fd
is closed, with a mutex held it will be necessary to unlock it. But I
think it’s a bit clearer to use wake_up in e.g. kvm_vcpu_release
instead of mutex_unlock. Paolo, could you please also comment on this?

>
>