On 12/02/2016 12:34 PM, Roman Kagan wrote: > 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? Yes, please do you patch with the swake_up and I will come up with a followup. -- 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
