在 2021/3/17 下午6:46, Elena Afanasova 写道:
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?
I may miss something but the semantic is the same for the following of
the two?
A:
spin_lock(&ctx->wq.lock);
wait_event_interruptible_exclusive_locked(ctx->wq, !ctx->busy);
ctx->busy = true;
spin_unlock(&ctx->wq.lock);
B:
mutex_lock_interruptible(&ctx->lock);
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.
It's something nature and anything different with your current code that
uses spinlock + waitqueue? And we can know whehter or not we're
interrupt by a singal (I notice you don't check the return value of
wait_event_interruptible_exclusive_locked() and set ctx->busy to true
unconditonally which is probably wrong.
But I
think it’s a bit clearer to use wake_up in e.g. kvm_vcpu_release
instead of mutex_unlock.
I am not sure I undersatnd here (I dont' this how it is doen in this patch).
Thanks
Paolo, could you please also comment on this?
