On Fri, Dec 02, 2016 at 11:00:00AM +0100, Christian Borntraeger wrote: > 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) As my knowlege of s390 approaches zero so that only your thinking can be applied here ;) , how about me redoing this patch leaving the open-coded wakeup as is, and then you submitting another patch on top dropping that wakeup if you find that appropriate? Thanks, Roman. -- 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
