This is a continuation of Wanpeng's series[1] to fix tick-based CPU time
accounting on x86, with my cleanups[2] bolted on top. The core premise of
Wanpeng's patches are preserved, but they are heavily stripped down.
Specifically, only the "guest exit" paths are split, and no code is
consolidated. The intent is to do as little as possible in the three
patches that need to be backported. Keeping those changes as small as
possible also meant that my cleanups did not need to unwind much
refactoring.
On x86, tested CONFIG_VIRT_CPU_ACCOUNTING_GEN =y and =n, and with
CONFIG_DEBUG_ENTRY=y && CONFIG_VALIDATE_STACKS=y. Compile tested arm64,
MIPS, PPC, and s390, the latter with CONFIG_DEBUG_ENTRY=y for giggles.
One last note: I elected to use vtime_account_guest_exit() in the x86 code
instead of open coding these equivalents:
if (vtime_accounting_enabled_this_cpu())
vtime_guest_exit(current);
...
if (!vtime_accounting_enabled_this_cpu())
current->flags &= ~PF_VCPU;
With CONFIG_VIRT_CPU_ACCOUNTING_GEN=n, this is a complete non-issue, but
for the =y case it means context_tracking_enabled_this_cpu() is being
checked back-to-back. The redundant checks bug me, but open coding the
gory details in x86 or providing funky variants in vtime.h felt worse.
Delta from Wanpeng's v2:
- s/context_guest/context_tracking_guest, purely to match the existing
functions. I have no strong opinion either way.
- Split only the "exit" functions.
- Partially open code vcpu_account_guest_exit() and
__vtime_account_guest_exit() in x86 to avoid churn when segueing into
my cleanups (see above).
[1] https://lkml.kernel.org/r/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
Sean Christopherson (6):
sched/vtime: Move vtime accounting external declarations above inlines
sched/vtime: Move guest enter/exit vtime accounting to vtime.h
context_tracking: Consolidate guest enter/exit wrappers
context_tracking: KVM: Move guest enter/exit wrappers to KVM's domain
KVM: x86: Consolidate guest enter/exit logic to common helpers
KVM: Move instrumentation-safe annotations for enter/exit to x86 code
Wanpeng Li (3):
context_tracking: Move guest exit context tracking to separate helpers
context_tracking: Move guest exit vtime accounting to separate helpers
KVM: x86: Defer tick-based accounting 'til after IRQ handling
arch/x86/kvm/svm/svm.c | 39 +--------
arch/x86/kvm/vmx/vmx.c | 39 +--------
arch/x86/kvm/x86.c | 8 ++
arch/x86/kvm/x86.h | 52 ++++++++++++
include/linux/context_tracking.h | 92 ++++-----------------
include/linux/kvm_host.h | 38 +++++++++
include/linux/vtime.h | 138 +++++++++++++++++++------------
7 files changed, 204 insertions(+), 202 deletions(-)
--
2.31.1.368.gbe11c130af-goog
From: Wanpeng Li <[email protected]>
Provide separate context tracking helpers for guest exit, the standalone
helpers will be called separately by KVM x86 in later patches to fix
tick-based accounting.
Suggested-by: Thomas Gleixner <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Michael Tokarev <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
Co-developed-by: Sean Christopherson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
include/linux/context_tracking.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index bceb06498521..200d30cb3a82 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -131,10 +131,15 @@ static __always_inline void guest_enter_irqoff(void)
}
}
-static __always_inline void guest_exit_irqoff(void)
+static __always_inline void context_tracking_guest_exit_irqoff(void)
{
if (context_tracking_enabled())
__context_tracking_exit(CONTEXT_GUEST);
+}
+
+static __always_inline void guest_exit_irqoff(void)
+{
+ context_tracking_guest_exit_irqoff();
instrumentation_begin();
if (vtime_accounting_enabled_this_cpu())
@@ -159,6 +164,8 @@ static __always_inline void guest_enter_irqoff(void)
instrumentation_end();
}
+static __always_inline void context_tracking_guest_exit_irqoff(void) { }
+
static __always_inline void guest_exit_irqoff(void)
{
instrumentation_begin();
--
2.31.1.368.gbe11c130af-goog
From: Wanpeng Li <[email protected]>
Provide separate vtime accounting functions for guest exit instead of
open coding the logic within the context tracking code. This will allow
KVM x86 to handle vtime accounting slightly differently when using
tick-based accounting.
Suggested-by: Thomas Gleixner <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Michael Tokarev <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
Co-developed-by: Sean Christopherson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
include/linux/context_tracking.h | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index 200d30cb3a82..7cf03a8e5708 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -137,15 +137,20 @@ static __always_inline void context_tracking_guest_exit_irqoff(void)
__context_tracking_exit(CONTEXT_GUEST);
}
-static __always_inline void guest_exit_irqoff(void)
+static __always_inline void vtime_account_guest_exit(void)
{
- context_tracking_guest_exit_irqoff();
-
- instrumentation_begin();
if (vtime_accounting_enabled_this_cpu())
vtime_guest_exit(current);
else
current->flags &= ~PF_VCPU;
+}
+
+static __always_inline void guest_exit_irqoff(void)
+{
+ context_tracking_guest_exit_irqoff();
+
+ instrumentation_begin();
+ vtime_account_guest_exit();
instrumentation_end();
}
@@ -166,12 +171,17 @@ static __always_inline void guest_enter_irqoff(void)
static __always_inline void context_tracking_guest_exit_irqoff(void) { }
-static __always_inline void guest_exit_irqoff(void)
+static __always_inline void vtime_account_guest_exit(void)
{
- instrumentation_begin();
- /* Flush the guest cputime we spent on the guest */
vtime_account_kernel(current);
current->flags &= ~PF_VCPU;
+}
+
+static __always_inline void guest_exit_irqoff(void)
+{
+ instrumentation_begin();
+ /* Flush the guest cputime we spent on the guest */
+ vtime_account_guest_exit();
instrumentation_end();
}
#endif /* CONFIG_VIRT_CPU_ACCOUNTING_GEN */
--
2.31.1.368.gbe11c130af-goog
On 16.04.21 00:20, Sean Christopherson wrote:
> From: Wanpeng Li <[email protected]>
>
> Provide separate context tracking helpers for guest exit, the standalone
> helpers will be called separately by KVM x86 in later patches to fix
> tick-based accounting.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Michael Tokarev <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> Co-developed-by: Sean Christopherson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
> ---
> include/linux/context_tracking.h | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> index bceb06498521..200d30cb3a82 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -131,10 +131,15 @@ static __always_inline void guest_enter_irqoff(void)
> }
> }
>
> -static __always_inline void guest_exit_irqoff(void)
> +static __always_inline void context_tracking_guest_exit_irqoff(void)
> {
> if (context_tracking_enabled())
> __context_tracking_exit(CONTEXT_GUEST);
> +}
> +
> +static __always_inline void guest_exit_irqoff(void)
> +{
> + context_tracking_guest_exit_irqoff();
>
> instrumentation_begin();
> if (vtime_accounting_enabled_this_cpu())
> @@ -159,6 +164,8 @@ static __always_inline void guest_enter_irqoff(void)
> instrumentation_end();
> }
>
> +static __always_inline void context_tracking_guest_exit_irqoff(void) { }
> +
> static __always_inline void guest_exit_irqoff(void)
> {
> instrumentation_begin();
>
On 16.04.21 00:20, Sean Christopherson wrote:
> From: Wanpeng Li <[email protected]>
>
> Provide separate vtime accounting functions for guest exit instead of
> open coding the logic within the context tracking code. This will allow
> KVM x86 to handle vtime accounting slightly differently when using
> tick-based accounting.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Michael Tokarev <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> Co-developed-by: Sean Christopherson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
> ---
> include/linux/context_tracking.h | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> index 200d30cb3a82..7cf03a8e5708 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -137,15 +137,20 @@ static __always_inline void context_tracking_guest_exit_irqoff(void)
> __context_tracking_exit(CONTEXT_GUEST);
> }
>
> -static __always_inline void guest_exit_irqoff(void)
> +static __always_inline void vtime_account_guest_exit(void)
> {
> - context_tracking_guest_exit_irqoff();
> -
> - instrumentation_begin();
> if (vtime_accounting_enabled_this_cpu())
> vtime_guest_exit(current);
> else
> current->flags &= ~PF_VCPU;
> +}
> +
> +static __always_inline void guest_exit_irqoff(void)
> +{
> + context_tracking_guest_exit_irqoff();
> +
> + instrumentation_begin();
> + vtime_account_guest_exit();
> instrumentation_end();
> }
>
> @@ -166,12 +171,17 @@ static __always_inline void guest_enter_irqoff(void)
>
> static __always_inline void context_tracking_guest_exit_irqoff(void) { }
>
> -static __always_inline void guest_exit_irqoff(void)
> +static __always_inline void vtime_account_guest_exit(void)
> {
> - instrumentation_begin();
> - /* Flush the guest cputime we spent on the guest */
> vtime_account_kernel(current);
> current->flags &= ~PF_VCPU;
> +}
> +
> +static __always_inline void guest_exit_irqoff(void)
> +{
> + instrumentation_begin();
> + /* Flush the guest cputime we spent on the guest */
> + vtime_account_guest_exit();
> instrumentation_end();
> }
> #endif /* CONFIG_VIRT_CPU_ACCOUNTING_GEN */
>
On Thu, Apr 15, 2021 at 03:20:57PM -0700, Sean Christopherson wrote:
> This is a continuation of Wanpeng's series[1] to fix tick-based CPU time
> accounting on x86, with my cleanups[2] bolted on top. The core premise of
> Wanpeng's patches are preserved, but they are heavily stripped down.
> Specifically, only the "guest exit" paths are split, and no code is
> consolidated. The intent is to do as little as possible in the three
> patches that need to be backported. Keeping those changes as small as
> possible also meant that my cleanups did not need to unwind much
> refactoring.
>
> On x86, tested CONFIG_VIRT_CPU_ACCOUNTING_GEN =y and =n, and with
> CONFIG_DEBUG_ENTRY=y && CONFIG_VALIDATE_STACKS=y. Compile tested arm64,
> MIPS, PPC, and s390, the latter with CONFIG_DEBUG_ENTRY=y for giggles.
>
> One last note: I elected to use vtime_account_guest_exit() in the x86 code
> instead of open coding these equivalents:
>
> if (vtime_accounting_enabled_this_cpu())
> vtime_guest_exit(current);
> ...
> if (!vtime_accounting_enabled_this_cpu())
> current->flags &= ~PF_VCPU;
>
> With CONFIG_VIRT_CPU_ACCOUNTING_GEN=n, this is a complete non-issue, but
> for the =y case it means context_tracking_enabled_this_cpu() is being
> checked back-to-back. The redundant checks bug me, but open coding the
> gory details in x86 or providing funky variants in vtime.h felt worse.
>
> Delta from Wanpeng's v2:
>
> - s/context_guest/context_tracking_guest, purely to match the existing
> functions. I have no strong opinion either way.
> - Split only the "exit" functions.
> - Partially open code vcpu_account_guest_exit() and
> __vtime_account_guest_exit() in x86 to avoid churn when segueing into
> my cleanups (see above).
>
> [1] https://lkml.kernel.org/r/[email protected]
> [2] https://lkml.kernel.org/r/[email protected]
>
> Sean Christopherson (6):
> sched/vtime: Move vtime accounting external declarations above inlines
> sched/vtime: Move guest enter/exit vtime accounting to vtime.h
> context_tracking: Consolidate guest enter/exit wrappers
> context_tracking: KVM: Move guest enter/exit wrappers to KVM's domain
> KVM: x86: Consolidate guest enter/exit logic to common helpers
> KVM: Move instrumentation-safe annotations for enter/exit to x86 code
>
> Wanpeng Li (3):
> context_tracking: Move guest exit context tracking to separate helpers
> context_tracking: Move guest exit vtime accounting to separate helpers
> KVM: x86: Defer tick-based accounting 'til after IRQ handling
>
> arch/x86/kvm/svm/svm.c | 39 +--------
> arch/x86/kvm/vmx/vmx.c | 39 +--------
> arch/x86/kvm/x86.c | 8 ++
> arch/x86/kvm/x86.h | 52 ++++++++++++
> include/linux/context_tracking.h | 92 ++++-----------------
> include/linux/kvm_host.h | 38 +++++++++
> include/linux/vtime.h | 138 +++++++++++++++++++------------
> 7 files changed, 204 insertions(+), 202 deletions(-)
Please Cc me on any follow-up of this patchset. I have set up a lot of booby
traps on purpose in this cave and I might be able to remember a few on the way.
Should you meet one of the poisoned arrows, rest assured that you were not the
aimed target though.
Thanks.
On Thu, Apr 15, 2021 at 03:20:58PM -0700, Sean Christopherson wrote:
> From: Wanpeng Li <[email protected]>
>
> Provide separate context tracking helpers for guest exit, the standalone
> helpers will be called separately by KVM x86 in later patches to fix
> tick-based accounting.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Michael Tokarev <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> Co-developed-by: Sean Christopherson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> include/linux/context_tracking.h | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> index bceb06498521..200d30cb3a82 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -131,10 +131,15 @@ static __always_inline void guest_enter_irqoff(void)
> }
> }
>
> -static __always_inline void guest_exit_irqoff(void)
> +static __always_inline void context_tracking_guest_exit_irqoff(void)
You can make this context_tracking_guest_exit() since there is no irq-on
version.
Thanks.