On 12/02/2016 10:40 AM, Paolo Bonzini wrote: > > > ----- Original Message ----- >> From: "Christian Borntraeger" <borntraeger@xxxxxxxxxx> >> To: "Roman Kagan" <rkagan@xxxxxxxxxxxxx>, "Radim Krčmář" <rkrcmar@xxxxxxxxxx>, "Paolo Bonzini" <pbonzini@xxxxxxxxxx>, >> kvm@xxxxxxxxxxxxxxx >> Cc: "Denis Lunev" <den@xxxxxxxxxxxxx> >> Sent: Friday, December 2, 2016 10:35:02 AM >> Subject: Re: [PATCH 3/5] kvm: kick vcpu when async_pf is resolved >> >> On 12/02/2016 09:47 AM, Roman Kagan wrote: >>> When async_pf is ready the guest needs to be made aware of it ASAP, >>> because it may be holding off a higher priority task pending the >>> async_pf resolution in favor of a lower priority one. >>> >>> In case async_pf's are harvested in vcpu context (x86) we have to not >>> only wake the vcpu up but kick it into host. >>> >>> While at this, also replace the open-coded vcpu wakeup by the existing >>> helper. >>> >>> Signed-off-by: Roman Kagan <rkagan@xxxxxxxxxxxxx> >>> --- >>> virt/kvm/async_pf.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c >>> index 9cced14..5f0a66c 100644 >>> --- a/virt/kvm/async_pf.c >>> +++ b/virt/kvm/async_pf.c >>> @@ -105,8 +105,11 @@ static void async_pf_execute(struct work_struct *work) >>> * This memory barrier pairs with prepare_to_wait's set_current_state() >>> */ >>> smp_mb(); >>> - if (swait_active(&vcpu->wq)) >>> - swake_up(&vcpu->wq); >>> +#ifdef CONFIG_KVM_ASYNC_PF_SYNC >>> + kvm_vcpu_wake_up(vcpu); >>> +#else >>> + kvm_vcpu_kick(vcpu); >>> +#endif >> >> This will break s390, both functions are disabled for s390. >> On s390 do not want to kick the CPU for a completion. Instead we implement >> the kvm_async_page_present_sync call above and handle completion via an >> "pfault done" interrupt via the normal interrupt delivery. > > Is there any reason (with this patch) to disable kvm_vcpu_wake_up on s390? > It was unused until now, but the patch makes sense as a cleanup. We could enable that. It was some kind of a trigger, that we get a build error when someone enables that for s390 (as it might not be want you want) On the other hand, I dont think that we need a wakeup at all for the SYNC case. (as the interrupt will do that anyway) Maybe something like that - if (swait_active(&vcpu->wq)) - swake_up(&vcpu->wq); +#ifndef CONFIG_KVM_ASYNC_PF_SYNC + kvm_vcpu_kick(vcpu); +#endif would be good enough. (needs more thinking to be sure) Christian -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
