linux-kernel - Re: [PATCH 1/1] xen/time: do not decrease steal time after live migration on xen

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c0a7963d-3466-7a4d-cd61-ec43d6ef696b@oracle.com>
Date:   Thu, 19 Oct 2017 11:10:32 -0400
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Dongli Zhang <dongli.zhang@...cle.com>,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org
Cc:     jgross@...e.com, joao.m.martins@...cle.com
Subject: Re: [PATCH 1/1] xen/time: do not decrease steal time after live
 migration on xen

On 10/19/2017 04:02 AM, Dongli Zhang wrote:
> After guest live migration on xen, steal time in /proc/stat
> (cpustat[CPUTIME_STEAL]) might decrease because steal returned by
> xen_steal_lock() might be less than this_rq()->prev_steal_time which is
> derived from previous return value of xen_steal_clock().
>
> For instance, steal time of each vcpu is 335 before live migration.
>
> cpu  198 0 368 200064 1962 0 0 1340 0 0
> cpu0 38 0 81 50063 492 0 0 335 0 0
> cpu1 65 0 97 49763 634 0 0 335 0 0
> cpu2 38 0 81 50098 462 0 0 335 0 0
> cpu3 56 0 107 50138 374 0 0 335 0 0
>
> After live migration, steal time is reduced to 312.
>
> cpu  200 0 370 200330 1971 0 0 1248 0 0
> cpu0 38 0 82 50123 500 0 0 312 0 0
> cpu1 65 0 97 49832 634 0 0 312 0 0
> cpu2 39 0 82 50167 462 0 0 312 0 0
> cpu3 56 0 107 50207 374 0 0 312 0 0
>
> The code in this patch is borrowed from do_stolen_accounting() which has
> already been removed from linux source code since commit ecb23dc6f2ef
> ("xen: add steal_clock support on x86"). The core idea of both
> do_stolen_accounting() and this patch is to avoid accounting new steal
> clock if it is smaller than previous old steal clock.
>
> Similar and more severe issue would impact prior linux 4.8-4.10 as
> discussed by Michael Las at
> https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest,
> which would overflow steal time and lead to 100% st usage in top command
> for linux 4.8-4.10. A backport of this patch would fix that issue.
>
> References: https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest
> Signed-off-by: Dongli Zhang <dongli.zhang@...cle.com>
> ---
>  drivers/xen/time.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/time.c b/drivers/xen/time.c
> index ac5f23f..2b3a996 100644
> --- a/drivers/xen/time.c
> +++ b/drivers/xen/time.c
> @@ -19,6 +19,8 @@
>  /* runstate info updated by Xen */
>  static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate);
>  
> +static DEFINE_PER_CPU(u64, xen_old_steal);
> +
>  /* return an consistent snapshot of 64-bit time/counter value */
>  static u64 get64(const u64 *p)
>  {
> @@ -83,9 +85,20 @@ bool xen_vcpu_stolen(int vcpu)
>  u64 xen_steal_clock(int cpu)
>  {
>  	struct vcpu_runstate_info state;
> +	u64 xen_new_steal;
> +	s64 steal_delta;
>  
>  	xen_get_runstate_snapshot_cpu(&state, cpu);
> -	return state.time[RUNSTATE_runnable] + state.time[RUNSTATE_offline];
> +	xen_new_steal = state.time[RUNSTATE_runnable]
> +					+ state.time[RUNSTATE_offline];
> +	steal_delta = xen_new_steal - per_cpu(xen_old_steal, cpu);
> +
> +	if (steal_delta < 0)
> +		xen_new_steal = per_cpu(xen_old_steal, cpu);
> +	else
> +		per_cpu(xen_old_steal, cpu) = xen_new_steal;
> +
> +	return xen_new_steal;
>  }
>  
>  void xen_setup_runstate_info(int cpu)

Can we stash state.time[] during suspend and then add stashed values
inside xen_get_runstate_snapshot_cpu()?

This will make xen_steal_clock() simpler.

-boris