Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1702570

Re: [PATCH v4 09/10] pvclock: add clocksource change notification on changing of tsc stable bit

From Marcelo Tosatti <mtosatti@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH v4 09/10] pvclock: add clocksource change notification on changing of tsc stable bit
Date 2017-08-03 01:40 +0200
Message-ID <uabp7-8mA-5@gated-at.bofh.it> (permalink)
References <ua3hT-2KU-5@gated-at.bofh.it> <ua3hU-2KU-39@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, Aug 02, 2017 at 05:38:09PM +0300, Denis Plotnikov wrote:
> It's needed to notify the KVM guest about critical changes in pvclock
> and make it to update its masterclock.
> 
> This is a part of the work aiming to make kvmclock be a clocksource
> providing valid cycles value for KVM masterclock, another words
> make possible to use KVM masterclock over kvmclock clocksource.
> 
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> ---
>  arch/x86/kernel/pvclock.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)

Please do an analysis similar to the comment which starts at 

"* Assuming a stable TSC across physical CPUS, and a stable TSC
 * across virtual CPUs, the following condition is possible.
 * Each numbered line represents an event visible to both
 * CPUs at the next numbered event."

Describing why its safe to use kvmclock as source for the masterclock
(honestly i haven't gone through the details, but someone should 
before this patch is merged).

For one thing, its only safe to use kvmclock masterclock if
the TSCs are synchronized in the host (so that you can read offset
on vcpu-0 using a TSC that has been initialized on vcpu-1). 
So masterclock in the L1 guest is necessary. Do you enforce that?

Also L2 guest TSCs must be synchronized. Where is that enforced?

Also why its safe to use non-TSC-clocksource (clock_read, tsc_read) + 
tsc offsets from that point.



> 
> diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
> index bece384..5898f20 100644
> --- a/arch/x86/kernel/pvclock.c
> +++ b/arch/x86/kernel/pvclock.c
> @@ -22,6 +22,7 @@
>  #include <linux/gfp.h>
>  #include <linux/bootmem.h>
>  #include <linux/nmi.h>
> +#include <linux/cs_notifier.h>
>  
>  #include <asm/fixmap.h>
>  #include <asm/pvclock.h>
> @@ -73,6 +74,8 @@ u8 pvclock_read_flags(struct pvclock_vcpu_time_info *src)
>  	return flags & valid_flags;
>  }
>  
> +static atomic_t clocksource_stable = ATOMIC_INIT(0);
> +
>  u64 pvclock_clocksource_read(struct pvclock_vcpu_time_info *src,
>  				u64 *cycles_stamp, u8 *flags_stamp)
>  {
> @@ -102,10 +105,20 @@ u64 pvclock_clocksource_read(struct pvclock_vcpu_time_info *src,
>  		pvclock_touch_watchdogs();
>  	}
>  
> -	if ((valid_flags & PVCLOCK_TSC_STABLE_BIT) &&
> -		(flags & PVCLOCK_TSC_STABLE_BIT))
> -		return ret;
> +	if (likely(valid_flags & PVCLOCK_TSC_STABLE_BIT)) {
> +		bool stable_now = !!(flags & PVCLOCK_TSC_STABLE_BIT);
> +		bool stable_last = (bool) atomic_read(&clocksource_stable);
> +
> +		if (unlikely(stable_now != stable_last)) {
> +			/* send notification once */
> +			if (stable_last == atomic_cmpxchg(
> +				&clocksource_stable, stable_last, stable_now))
> +				clocksource_changes_notify();
> +		}
>  
> +		if (stable_now)
> +			return ret;
> +	}
>  	/*
>  	 * Assumption here is that last_value, a global accumulator, always goes
>  	 * forward. If we are less than that, we should not be much smaller.
> -- 
> 2.7.4

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v4 00/10] make L2's kvm-clock stable, get rid of pvclock_gtod_copy in KVM Denis Plotnikov <dplotnikov@virtuozzo.com> - 2017-08-02 17:00 +0200
  [PATCH v4 10/10] kvmclock: implement the extended reading function Denis Plotnikov <dplotnikov@virtuozzo.com> - 2017-08-02 17:00 +0200
  [PATCH v4 07/10] KVM: x86: remove not used pvclock_gtod_copy Denis Plotnikov <dplotnikov@virtuozzo.com> - 2017-08-02 17:00 +0200
    Re: [PATCH v4 07/10] KVM: x86: remove not used pvclock_gtod_copy Marcelo Tosatti <mtosatti@redhat.com> - 2017-08-03 01:30 +0200
      Re: [PATCH v4 07/10] KVM: x86: remove not used pvclock_gtod_copy Paolo Bonzini <pbonzini@redhat.com> - 2017-08-03 14:40 +0200
  [PATCH v4 09/10] pvclock: add clocksource change notification on changing of tsc stable bit Denis Plotnikov <dplotnikov@virtuozzo.com> - 2017-08-02 17:00 +0200
    Re: [PATCH v4 09/10] pvclock: add clocksource change notification on  changing of tsc stable bit Marcelo Tosatti <mtosatti@redhat.com> - 2017-08-03 01:40 +0200
  [PATCH v4 06/10] timekeeper: add clocksource change notifier Denis Plotnikov <dplotnikov@virtuozzo.com> - 2017-08-02 17:00 +0200
  Re: [PATCH v4 00/10] make L2's kvm-clock stable, get rid of  pvclock_gtod_copy in KVM Paolo Bonzini <pbonzini@redhat.com> - 2017-08-02 18:20 +0200
  Re: [PATCH v4 00/10] make L2's kvm-clock stable, get rid of  pvclock_gtod_copy in KVM John Stultz <john.stultz@linaro.org> - 2017-08-02 18:50 +0200
    Re: [PATCH v4 00/10] make L2's kvm-clock stable, get rid of  pvclock_gtod_copy in KVM Paolo Bonzini <pbonzini@redhat.com> - 2017-08-02 19:20 +0200

csiph-web