From: Wanpeng Li <[email protected]>
Inspired by commit 262de4102c7bb8 (kvm: exit halt polling on need_resched()
as well), due to PPC implements an arch specific halt polling logic, we should
add the need_resched() checking there as well.
Cc: Paul Mackerras <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/powerpc/kvm/book3s_hv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 28a80d2..6199397 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3936,7 +3936,8 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
break;
}
cur = ktime_get();
- } while (single_task_running() && ktime_before(cur, stop));
+ } while (single_task_running() && !need_resched() &&
+ ktime_before(cur, stop));
spin_lock(&vc->lock);
vc->vcore_state = VCORE_INACTIVE;
--
2.7.4
From: Wanpeng Li <[email protected]>
Commit 66570e966dd9 (kvm: x86: only provide PV features if enabled in guest's
CPUID) avoids to access pv tlb shootdown host side logic when this pv feature
is not exposed to guest, however, kvm_steal_time.preempted not only leveraged
by pv tlb shootdown logic but also mitigate the lock holder preemption issue.
From guest point of view, vCPU is always preempted since we lose the reset of
kvm_steal_time.preempted before vmentry if pv tlb shootdown feature is not
exposed. This patch fixes it by clearing kvm_steal_time.preempted before
vmentry.
Fixes: 66570e966dd9 (kvm: x86: only provide PV features if enabled in guest's CPUID)
Cc: [email protected]
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/kvm/x86.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c0244a6..c38e990 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3105,7 +3105,8 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
st->preempted & KVM_VCPU_FLUSH_TLB);
if (xchg(&st->preempted, 0) & KVM_VCPU_FLUSH_TLB)
kvm_vcpu_flush_tlb_guest(vcpu);
- }
+ } else
+ st->preempted = 0;
vcpu->arch.st.preempted = 0;
--
2.7.4
On Sat, May 08, 2021, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> Commit 66570e966dd9 (kvm: x86: only provide PV features if enabled in guest's
> CPUID) avoids to access pv tlb shootdown host side logic when this pv feature
> is not exposed to guest, however, kvm_steal_time.preempted not only leveraged
> by pv tlb shootdown logic but also mitigate the lock holder preemption issue.
> From guest point of view, vCPU is always preempted since we lose the reset of
> kvm_steal_time.preempted before vmentry if pv tlb shootdown feature is not
> exposed. This patch fixes it by clearing kvm_steal_time.preempted before
> vmentry.
>
> Fixes: 66570e966dd9 (kvm: x86: only provide PV features if enabled in guest's CPUID)
> Cc: [email protected]
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> arch/x86/kvm/x86.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c0244a6..c38e990 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3105,7 +3105,8 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
> st->preempted & KVM_VCPU_FLUSH_TLB);
> if (xchg(&st->preempted, 0) & KVM_VCPU_FLUSH_TLB)
> kvm_vcpu_flush_tlb_guest(vcpu);
> - }
> + } else
> + st->preempted = 0;
Curly braces needed since the if-statment needs 'em. Other than that,
Reviewed-by: Sean Christopherson <[email protected]>
>
> vcpu->arch.st.preempted = 0;
>
> --
> 2.7.4
>
On Tue, 11 May 2021 at 08:18, Sean Christopherson <[email protected]> wrote:
>
> On Sat, May 08, 2021, Wanpeng Li wrote:
> > From: Wanpeng Li <[email protected]>
> >
> > Commit 66570e966dd9 (kvm: x86: only provide PV features if enabled in guest's
> > CPUID) avoids to access pv tlb shootdown host side logic when this pv feature
> > is not exposed to guest, however, kvm_steal_time.preempted not only leveraged
> > by pv tlb shootdown logic but also mitigate the lock holder preemption issue.
> > From guest point of view, vCPU is always preempted since we lose the reset of
> > kvm_steal_time.preempted before vmentry if pv tlb shootdown feature is not
> > exposed. This patch fixes it by clearing kvm_steal_time.preempted before
> > vmentry.
> >
> > Fixes: 66570e966dd9 (kvm: x86: only provide PV features if enabled in guest's CPUID)
> > Cc: [email protected]
> > Signed-off-by: Wanpeng Li <[email protected]>
> > ---
> > arch/x86/kvm/x86.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index c0244a6..c38e990 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -3105,7 +3105,8 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
> > st->preempted & KVM_VCPU_FLUSH_TLB);
> > if (xchg(&st->preempted, 0) & KVM_VCPU_FLUSH_TLB)
> > kvm_vcpu_flush_tlb_guest(vcpu);
> > - }
> > + } else
> > + st->preempted = 0;
>
> Curly braces needed since the if-statment needs 'em. Other than that,
Will send out a new version after 1-2 get reviewed. :)
>
> Reviewed-by: Sean Christopherson <[email protected]>
>
> >
> > vcpu->arch.st.preempted = 0;
> >
> > --
> > 2.7.4
> >
Cc more guys,
On Sat, 8 May 2021 at 17:32, Wanpeng Li <[email protected]> wrote:
>
> From: Wanpeng Li <[email protected]>
>
> Inspired by commit 262de4102c7bb8 (kvm: exit halt polling on need_resched()
> as well), due to PPC implements an arch specific halt polling logic, we should
> add the need_resched() checking there as well.
>
Update the patch description:
Inspired by commit 262de4102c7bb8 (kvm: exit halt polling on need_resched()
as well), CFS_BANDWIDTH throttling will use resched_task() when there is just
one task to get the task to block. It was likely allowing VMs to overrun their
quota when halt polling. Due to PPC implements an arch specific halt polling
logic, we should add the need_resched() checking there as well.
> Cc: Paul Mackerras <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> arch/powerpc/kvm/book3s_hv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 28a80d2..6199397 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3936,7 +3936,8 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
> break;
> }
> cur = ktime_get();
> - } while (single_task_running() && ktime_before(cur, stop));
> + } while (single_task_running() && !need_resched() &&
> + ktime_before(cur, stop));
>
> spin_lock(&vc->lock);
> vc->vcore_state = VCORE_INACTIVE;
> --
> 2.7.4
>