Groups > linux.kernel > #1588645 > unrolled thread

Re: tip.today - scheduler bam boom crash (cpu hotplug)

Started by	Wanpeng Li <kernellwp@gmail.com>
First post	2017-02-27 13:40 +0100
Last post	2017-02-27 16:10 +0100
Articles	18 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

  Re: tip.today - scheduler bam boom crash (cpu hotplug) Wanpeng Li <kernellwp@gmail.com> - 2017-02-27 13:40 +0100
    Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-27 13:50 +0100
      Re: tip.today - scheduler bam boom crash (cpu hotplug) Paolo Bonzini <pbonzini@redhat.com> - 2017-02-27 14:00 +0100
        Re: tip.today - scheduler bam boom crash (cpu hotplug) Paolo Bonzini <pbonzini@redhat.com> - 2017-02-27 16:40 +0100
          Re: tip.today - scheduler bam boom crash (cpu hotplug) Paolo Bonzini <pbonzini@redhat.com> - 2017-02-27 17:30 +0100
            Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-27 17:40 +0100
              Re: tip.today - scheduler bam boom crash (cpu hotplug) Paolo Bonzini <pbonzini@redhat.com> - 2017-02-27 18:30 +0100
                Re: tip.today - scheduler bam boom crash (cpu hotplug) Thomas Gleixner <tglx@linutronix.de> - 2017-02-27 19:10 +0100
                  Re: tip.today - scheduler bam boom crash (cpu hotplug) Paolo Bonzini <pbonzini@redhat.com> - 2017-02-27 20:10 +0100
                    Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-28 00:00 +0100
            Re: tip.today - scheduler bam boom crash (cpu hotplug) Wanpeng Li <kernellwp@gmail.com> - 2017-02-28 03:00 +0100
              Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-28 09:10 +0100
                Re: tip.today - scheduler bam boom crash (cpu hotplug) Wanpeng Li <kernellwp@gmail.com> - 2017-02-28 09:20 +0100
          Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-27 17:50 +0100
        Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-27 23:50 +0100
      Re: tip.today - scheduler bam boom crash (cpu hotplug) Wanpeng Li <kernellwp@gmail.com> - 2017-02-27 15:00 +0100
        Re: tip.today - scheduler bam boom crash (cpu hotplug) Peter Zijlstra <peterz@infradead.org> - 2017-02-27 15:50 +0100
    Re: tip.today - scheduler bam boom crash (cpu hotplug) Paolo Bonzini <pbonzini@redhat.com> - 2017-02-27 16:10 +0100

#1588645 — Re: tip.today - scheduler bam boom crash (cpu hotplug)

From	Wanpeng Li <kernellwp@gmail.com>
Date	2017-02-27 13:40 +0100
Subject	Re: tip.today - scheduler bam boom crash (cpu hotplug)
Message-ID	<tfsKS-5zx-19@gated-at.bofh.it>

Cc Paolo,
2017-01-19 21:36 GMT+08:00 Peter Zijlstra <peterz@infradead.org>:
> On Thu, Jan 19, 2017 at 11:19:24AM +0100, Peter Zijlstra wrote:
>
>> I'll go have a prod. Thanks!
>
> This seems to cure it for me.
>
> ---
> Subject: sched/clock: Fix hotplug issue
>
> Mike reported that he could trigger the WARN_ON_ONCE() in
> set_sched_clock_stable() using hotplug.
>
> This exposed a fundamental problem with the interface, we should never
> mark the TSC stable if we ever find it to be unstable. Therefore
> set_sched_clock_stable() is a broken interface.
>
> The reason it existed is that not having it is a pain, it means all
> relevant architecture code needs to call clear_sched_clock_stable()
> where appropriate.
>
> Of the three architectures that select HAVE_UNSTABLE_SCHED_CLOCK ia64
> and parisc are trivial in that they never called
> set_sched_clock_stable(), so add an unconditional call to
> clear_sched_clock_stable() to them.
>
> For x86 the story is a lot more involved, and what this patch tries to
> do is ensure we preserve the status quo. So even is Cyrix or Transmeta
> have usable TSC they never called set_sched_clock_stable() so they now
> get an explicit mark unstable.
>
> XXX: what about Xen ?
>
> Fixes: 9881b024b7d7 ("sched/clock: Delay switching sched_clock to stable")
> Reported-by: Mike Galbraith <efault@gmx.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/ia64/kernel/setup.c        |  2 ++
>  arch/parisc/kernel/setup.c      |  2 ++
>  arch/x86/kernel/cpu/amd.c       |  6 ++++--
>  arch/x86/kernel/cpu/centaur.c   |  2 ++
>  arch/x86/kernel/cpu/common.c    |  3 +++
>  arch/x86/kernel/cpu/cyrix.c     |  2 ++
>  arch/x86/kernel/cpu/intel.c     |  6 ++++--
>  arch/x86/kernel/cpu/transmeta.c |  3 +++
>  arch/x86/kernel/kvmclock.c      |  2 +-
>  include/linux/sched.h           |  1 -
>  kernel/sched/clock.c            | 29 ++++++++---------------------
>  11 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 7ec7acc..c483ece 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -619,6 +619,8 @@ setup_arch (char **cmdline_p)
>         check_sal_cache_flush();
>  #endif
>         paging_init();
> +
> +       clear_sched_clock_stable();
>  }
>
>  /*
> diff --git a/arch/parisc/kernel/setup.c b/arch/parisc/kernel/setup.c
> index 2e66a88..068ed36 100644
> --- a/arch/parisc/kernel/setup.c
> +++ b/arch/parisc/kernel/setup.c
> @@ -36,6 +36,7 @@
>  #undef PCI_DEBUG
>  #include <linux/proc_fs.h>
>  #include <linux/export.h>
> +#include <linux/sched.h>
>
>  #include <asm/processor.h>
>  #include <asm/sections.h>
> @@ -176,6 +177,7 @@ void __init setup_arch(char **cmdline_p)
>         conswitchp = &dummy_con;        /* we use do_take_over_console() later ! */
>  #endif
>
> +       clear_sched_clock_stable();
>  }
>
>  /*
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 1d31672..80e657e 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -541,8 +541,10 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>         if (c->x86_power & (1 << 8)) {
>                 set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>                 set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> -               if (!check_tsc_unstable())
> -                       set_sched_clock_stable();
> +               if (check_tsc_unstable())
> +                       clear_sched_clock_stable();
> +       } else {
> +               clear_sched_clock_stable();
>         }
>
>         /* Bit 12 of 8000_0007 edx is accumulated power mechanism. */
> diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
> index 1661d8e..c4bab8c 100644
> --- a/arch/x86/kernel/cpu/centaur.c
> +++ b/arch/x86/kernel/cpu/centaur.c
> @@ -104,6 +104,8 @@ static void early_init_centaur(struct cpuinfo_x86 *c)
>  #ifdef CONFIG_X86_64
>         set_cpu_cap(c, X86_FEATURE_SYSENTER32);
>  #endif
> +
> +       clear_sched_clock_stable();
>  }
>
>  static void init_centaur(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 9bab7a8..0bdb1ab 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -83,6 +83,7 @@ static void default_init(struct cpuinfo_x86 *c)
>                         strcpy(c->x86_model_id, "386");
>         }
>  #endif
> +       clear_sched_clock_stable();
>  }
>
>  static const struct cpu_dev default_cpu = {
> @@ -1055,6 +1056,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>          */
>         if (this_cpu->c_init)
>                 this_cpu->c_init(c);
> +       else
> +               clear_sched_clock_stable();
>
>         /* Disable the PN if appropriate */
>         squash_the_stupid_serial_number(c);
> diff --git a/arch/x86/kernel/cpu/cyrix.c b/arch/x86/kernel/cpu/cyrix.c
> index bd9dcd6..47416f9 100644
> --- a/arch/x86/kernel/cpu/cyrix.c
> +++ b/arch/x86/kernel/cpu/cyrix.c
> @@ -9,6 +9,7 @@
>  #include <asm/pci-direct.h>
>  #include <asm/tsc.h>
>  #include <asm/cpufeature.h>
> +#include <linux/sched.h>
>
>  #include "cpu.h"
>
> @@ -183,6 +184,7 @@ static void early_init_cyrix(struct cpuinfo_x86 *c)
>                 set_cpu_cap(c, X86_FEATURE_CYRIX_ARR);
>                 break;
>         }
> +       clear_sched_clock_stable();
>  }
>
>  static void init_cyrix(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 203f860..026c728 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -119,8 +119,10 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>         if (c->x86_power & (1 << 8)) {
>                 set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>                 set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> -               if (!check_tsc_unstable())
> -                       set_sched_clock_stable();
> +               if (check_tsc_unstable())
> +                       clear_sched_clock_stable();
> +       } else {
> +               clear_sched_clock_stable();
>         }
>
>         /* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
> diff --git a/arch/x86/kernel/cpu/transmeta.c b/arch/x86/kernel/cpu/transmeta.c
> index 3417856..c1ea5b9 100644
> --- a/arch/x86/kernel/cpu/transmeta.c
> +++ b/arch/x86/kernel/cpu/transmeta.c
> @@ -1,4 +1,5 @@
>  #include <linux/kernel.h>
> +#include <linux/sched.h>
>  #include <linux/mm.h>
>  #include <asm/cpufeature.h>
>  #include <asm/msr.h>
> @@ -14,6 +15,8 @@ static void early_init_transmeta(struct cpuinfo_x86 *c)
>                 if (xlvl >= 0x80860001)
>                         c->x86_capability[CPUID_8086_0001_EDX] = cpuid_edx(0x80860001);
>         }
> +
> +       clear_sched_clock_stable();
>  }
>
>  static void init_transmeta(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 2a5cafd..542710b 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -107,12 +107,12 @@ static inline void kvm_sched_clock_init(bool stable)
>  {
>         if (!stable) {
>                 pv_time_ops.sched_clock = kvm_clock_read;
> +               clear_sched_clock_stable();
>                 return;
>         }
>
>         kvm_sched_clock_offset = kvm_clock_read();
>         pv_time_ops.sched_clock = kvm_sched_clock_read;
> -       set_sched_clock_stable();

This results in sched clock always unstable for kvm guest since there
is no invariant tsc cpuid bit exposed for kvm guest currently. The
blockage happened for several reasons:

1) Migration: to host with different TSC frequency.
2) Savevm: It is not safe to use the TSC for wall clock timer services.

How about something like below(untested):

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index bae6ea6..a61c477 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -115,6 +115,7 @@ static inline void kvm_sched_clock_init(bool stable)

     kvm_sched_clock_offset = kvm_clock_read();
     pv_time_ops.sched_clock = kvm_sched_clock_read;
+    hypervisor_sched_clock_stable();

     printk(KERN_INFO "kvm-clock: using sched offset of %llu cycles\n",
             kvm_sched_clock_offset);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 451e241..38c6edb 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2499,6 +2499,10 @@ static inline void clear_sched_clock_stable(void)
 {
 }

+static inline void hypervisor_sched_clock_stable(void)
+{
+}
+
 static inline void sched_clock_idle_sleep_event(void)
 {
 }
@@ -2526,6 +2530,7 @@ extern void sched_clock_init_late(void);
  */
 extern int sched_clock_stable(void);
 extern void clear_sched_clock_stable(void);
+extern void hypervisor_sched_clock_stable(void);

 extern void sched_clock_tick(void);
 extern void sched_clock_idle_sleep_event(void);
diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
index ad64efe..a46639e 100644
--- a/kernel/sched/clock.c
+++ b/kernel/sched/clock.c
@@ -77,9 +77,15 @@ EXPORT_SYMBOL_GPL(sched_clock);

 __read_mostly int sched_clock_running;

+enum {
+    SCHED_CLOCK_INIT = 0,
+    HYPERVISOR_SCHED_CLOCK_STABLE,
+    SCHED_CLOCK_INIT_LATE
+};
+
 void sched_clock_init(void)
 {
-    sched_clock_running = 1;
+    sched_clock_running = SCHED_CLOCK_INIT;
 }

 #ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK
@@ -170,13 +176,14 @@ void clear_sched_clock_stable(void)

     smp_mb(); /* matches sched_clock_init_late() */

-    if (sched_clock_running == 2)
+    if (sched_clock_running == SCHED_CLOCK_INIT_LATE)
         schedule_work(&sched_clock_work);
 }

 void sched_clock_init_late(void)
 {
-    sched_clock_running = 2;
+    if (sched_clock_running == SCHED_CLOCK_INIT)
+        sched_clock_running = SCHED_CLOCK_INIT_LATE;
     /*
      * Ensure that it is impossible to not do a static_key update.
      *
@@ -186,8 +193,15 @@ void sched_clock_init_late(void)
      */
     smp_mb(); /* matches {set,clear}_sched_clock_stable() */

-    if (__sched_clock_stable_early)
+    if (__sched_clock_stable_early ||
+        sched_clock_running == HYPERVISOR_SCHED_CLOCK_STABLE) {
         __set_sched_clock_stable();
+    }
+}
+
+void hypervisor_sched_clock_stable()
+{
+    sched_clock_running = HYPERVISOR_SCHED_CLOCK_STABLE;
 }

 /*

Regards,
Wanpeng Li


>
>         printk(KERN_INFO "kvm-clock: using sched offset of %llu cycles\n",
>                         kvm_sched_clock_offset);
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 68b35dd..8a9bbb6 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2522,7 +2522,6 @@ extern void sched_clock_init_late(void);
>   * is reliable after all:
>   */
>  extern int sched_clock_stable(void);
> -extern void set_sched_clock_stable(void);
>  extern void clear_sched_clock_stable(void);
>
>  extern void sched_clock_tick(void);
> diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
> index 7713b2b..ad64efe 100644
> --- a/kernel/sched/clock.c
> +++ b/kernel/sched/clock.c
> @@ -83,8 +83,15 @@ void sched_clock_init(void)
>  }
>
>  #ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK
> +/*
> + * We must start with !__sched_clock_stable because the unstable -> stable
> + * transition is accurate, while the stable -> unstable transition is not.
> + *
> + * Similarly we start with __sched_clock_stable_early, thereby assuming we
> + * will become stable, such that there's only a single 1 -> 0 transition.
> + */
>  static DEFINE_STATIC_KEY_FALSE(__sched_clock_stable);
> -static int __sched_clock_stable_early;
> +static int __sched_clock_stable_early = 1;
>
>  /*
>   * We want: ktime_get_ns() + gtod_offset == sched_clock() + raw_offset
> @@ -132,24 +139,6 @@ static void __set_sched_clock_stable(void)
>         tick_dep_clear(TICK_DEP_BIT_CLOCK_UNSTABLE);
>  }
>
> -void set_sched_clock_stable(void)
> -{
> -       __sched_clock_stable_early = 1;
> -
> -       smp_mb(); /* matches sched_clock_init_late() */
> -
> -       /*
> -        * This really should only be called early (before
> -        * sched_clock_init_late()) when guestimating our sched_clock() is
> -        * solid.
> -        *
> -        * After that we test stability and we can negate our guess using
> -        * clear_sched_clock_stable, possibly from a watchdog.
> -        */
> -       if (WARN_ON_ONCE(sched_clock_running == 2))
> -               __set_sched_clock_stable();
> -}
> -
>  static void __clear_sched_clock_stable(struct work_struct *work)
>  {
>         struct sched_clock_data *scd = this_scd();
> @@ -199,8 +188,6 @@ void sched_clock_init_late(void)
>
>         if (__sched_clock_stable_early)
>                 __set_sched_clock_stable();
> -       else
> -               __clear_sched_clock_stable(NULL);
>  }
>
>  /*

[toc] | [next] | [standalone]

#1588646

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-27 13:50 +0100
Message-ID	<tfsUx-5CV-1@gated-at.bofh.it>
In reply to	#1588645

On Mon, Feb 27, 2017 at 08:30:11PM +0800, Wanpeng Li wrote:

> > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> > index 2a5cafd..542710b 100644
> > --- a/arch/x86/kernel/kvmclock.c
> > +++ b/arch/x86/kernel/kvmclock.c
> > @@ -107,12 +107,12 @@ static inline void kvm_sched_clock_init(bool stable)
> >  {
> >         if (!stable) {
> >                 pv_time_ops.sched_clock = kvm_clock_read;
> > +               clear_sched_clock_stable();
> >                 return;
> >         }
> >
> >         kvm_sched_clock_offset = kvm_clock_read();
> >         pv_time_ops.sched_clock = kvm_sched_clock_read;
> > -       set_sched_clock_stable();
> 
> This results in sched clock always unstable for kvm guest since there
> is no invariant tsc cpuid bit exposed for kvm guest currently. 

What the heck is KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
PVCLOCK_TSC_STABLE_BIT about then?

> How about something like below(untested):

I have no clue what that tries to do.

[toc] | [prev] | [next] | [standalone]

#1588655

From	Paolo Bonzini <pbonzini@redhat.com>
Date	2017-02-27 14:00 +0100
Message-ID	<tft4e-5G3-1@gated-at.bofh.it>
In reply to	#1588646


On 27/02/2017 13:43, Peter Zijlstra wrote:
> On Mon, Feb 27, 2017 at 08:30:11PM +0800, Wanpeng Li wrote:
> 
>>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>>> index 2a5cafd..542710b 100644
>>> --- a/arch/x86/kernel/kvmclock.c
>>> +++ b/arch/x86/kernel/kvmclock.c
>>> @@ -107,12 +107,12 @@ static inline void kvm_sched_clock_init(bool stable)
>>>  {
>>>         if (!stable) {
>>>                 pv_time_ops.sched_clock = kvm_clock_read;
>>> +               clear_sched_clock_stable();
>>>                 return;
>>>         }
>>>
>>>         kvm_sched_clock_offset = kvm_clock_read();
>>>         pv_time_ops.sched_clock = kvm_sched_clock_read;
>>> -       set_sched_clock_stable();
>>
>> This results in sched clock always unstable for kvm guest since there
>> is no invariant tsc cpuid bit exposed for kvm guest currently. 
> 
> What the heck is KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
> PVCLOCK_TSC_STABLE_BIT about then?

It checks that all the bugs in the host have been ironed out, and that
the host itself supports invtsc.

Paolo

[toc] | [prev] | [next] | [standalone]

#1588766

From	Paolo Bonzini <pbonzini@redhat.com>
Date	2017-02-27 16:40 +0100
Message-ID	<tfvz5-7vd-43@gated-at.bofh.it>
In reply to	#1588655


On 27/02/2017 14:04, Peter Zijlstra wrote:
>>>> This results in sched clock always unstable for kvm guest since there
>>>> is no invariant tsc cpuid bit exposed for kvm guest currently. 
>>> What the heck is KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
>>> PVCLOCK_TSC_STABLE_BIT about then?
>> It checks that all the bugs in the host have been ironed out, and that
>> the host itself supports invtsc.
> But what does it mean if that is not so? That is, will kvm_clock_read()
> still be stable even if !stable?

If kvmclock is !stable, nobody should have set that the sched clock to
stable, to begin with.

However, if kvmclock is stable, we know that the sched clock is stable.

Paolo

[toc] | [prev] | [next] | [standalone]

#1588795

From	Paolo Bonzini <pbonzini@redhat.com>
Date	2017-02-27 17:30 +0100
Message-ID	<tfwls-86h-15@gated-at.bofh.it>
In reply to	#1588766


On 27/02/2017 16:59, Peter Zijlstra wrote:
> OK, so if !KVM_FEATURE_CLOCKSOURCE_STABLE_BIT nothing is stable, but if
> it is set, TSC might still not be stable, but kvm_clock_read() is.
> 
>> However, if kvmclock is stable, we know that the sched clock is stable.
> Right, so the problem is that we only ever want to allow marking
> unstable -- once its found unstable, for whatever reason, we should
> never allow going stable. The corollary of this proposition is that we
> must start out assuming it will become stable. And to avoid actually
> using unstable TSC we do a 3 state bringup:
> 
>  1) sched_clock_running = 0, __stable_early = 1, __stable = 0
>  2) sched_clock_running = 1 (__stable is effective, iow, we run unstable)
>  3) sched_clock_running = 2 (__stable <- __stable_early)
> 
> 2) happens 'early' but is 'safe'.
> 3) happens 'late', after we've brought up SMP and probed TSC
> 
> Between there, we should have detected the most common TSC wreckage and
> made sure to not then switch to 'stable' at 3.
> 
> Now the problem appears to be that we assume sched_clock will use RDTSC
> (native_sched_clock) while sched_clock is a paravirt op.
> 
> Now, I've not yet figured out the ordering between when we set
> pv_time_ops.sched_clock and when we do the 'normal' TSC init stuff.

I think the ordering is fine:

- pv_time_ops.sched_clock is set here:

	start_kernel (init/main.c line 509)
	  setup_arch
	    kvmclock_init
	      kvm_sched_clock_init

- TSC can be declared unstable only after this:

	start_kernel (init/main.c line 628)
	  late_time_init
	    tsc_init

So by the time the tsc_cs_mark_unstable or mark_tsc_unstable can call
clear_sched_clock_stable, pv_time_ops.sched_clock has been set.

> But it appears to me, we should not be calling
> clear_sched_clock_stable() on TSC bits when we don't end up using
> native_sched_clock().

Yes, this makes sense.

Paolo

[toc] | [prev] | [next] | [standalone]

#1588800

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-27 17:40 +0100
Message-ID	<tfwv8-8at-13@gated-at.bofh.it>
In reply to	#1588795

On Mon, Feb 27, 2017 at 05:11:34PM +0100, Paolo Bonzini wrote:
> On 27/02/2017 16:59, Peter Zijlstra wrote:

> > Now, I've not yet figured out the ordering between when we set
> > pv_time_ops.sched_clock and when we do the 'normal' TSC init stuff.
> 
> I think the ordering is fine:
> 
> - pv_time_ops.sched_clock is set here:
> 
> 	start_kernel (init/main.c line 509)
> 	  setup_arch
> 	    kvmclock_init
> 	      kvm_sched_clock_init
> 
> - TSC can be declared unstable only after this:
> 
> 	start_kernel (init/main.c line 628)
> 	  late_time_init
> 	    tsc_init
> 
> So by the time the tsc_cs_mark_unstable or mark_tsc_unstable can call
> clear_sched_clock_stable, pv_time_ops.sched_clock has been set.
> 
> > But it appears to me, we should not be calling
> > clear_sched_clock_stable() on TSC bits when we don't end up using
> > native_sched_clock().
> 
> Yes, this makes sense.

Something like the below then (completely untested for now) has a chance
of working. It does however change behaviour a little.

I'm trying to debug something else; after that I'll give this a little
more consideration.

---
 arch/x86/kernel/cpu/amd.c       |  4 +---
 arch/x86/kernel/cpu/centaur.c   |  2 +-
 arch/x86/kernel/cpu/common.c    |  4 ++--
 arch/x86/kernel/cpu/cyrix.c     |  2 +-
 arch/x86/kernel/cpu/intel.c     |  4 +---
 arch/x86/kernel/cpu/transmeta.c |  2 +-
 arch/x86/kernel/tsc.c           | 33 +++++++++++++++++++++------------
 7 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 4e95b2e0d95f..bc3bbb6a8ab0 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -555,10 +555,8 @@ static void early_init_amd(struct cpuinfo_x86 *c)
 	if (c->x86_power & (1 << 8)) {
 		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
 		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
-		if (check_tsc_unstable())
-			clear_sched_clock_stable();
 	} else {
-		clear_sched_clock_stable();
+		mark_tsc_unstable("not invariant");
 	}
 
 	/* Bit 12 of 8000_0007 edx is accumulated power mechanism. */
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index 2c234a6d94c4..0fdff183aa30 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -105,7 +105,7 @@ static void early_init_centaur(struct cpuinfo_x86 *c)
 	set_cpu_cap(c, X86_FEATURE_SYSENTER32);
 #endif
 
-	clear_sched_clock_stable();
+	mark_tsc_unstable("not invariant");
 }
 
 static void init_centaur(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index f07005e6f461..2b7ff648ea25 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -86,7 +86,7 @@ static void default_init(struct cpuinfo_x86 *c)
 			strcpy(c->x86_model_id, "386");
 	}
 #endif
-	clear_sched_clock_stable();
+	mark_tsc_unstable("not invariant");
 }
 
 static const struct cpu_dev default_cpu = {
@@ -1076,7 +1076,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	if (this_cpu->c_init)
 		this_cpu->c_init(c);
 	else
-		clear_sched_clock_stable();
+		mark_tsc_unstable("not invariant");
 
 	/* Disable the PN if appropriate */
 	squash_the_stupid_serial_number(c);
diff --git a/arch/x86/kernel/cpu/cyrix.c b/arch/x86/kernel/cpu/cyrix.c
index 47416f959a48..35057d67e864 100644
--- a/arch/x86/kernel/cpu/cyrix.c
+++ b/arch/x86/kernel/cpu/cyrix.c
@@ -184,7 +184,7 @@ static void early_init_cyrix(struct cpuinfo_x86 *c)
 		set_cpu_cap(c, X86_FEATURE_CYRIX_ARR);
 		break;
 	}
-	clear_sched_clock_stable();
+	mark_tsc_unstable("not invariant");
 }
 
 static void init_cyrix(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 017ecd3bb553..e0e192e43a4c 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -161,10 +161,8 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 	if (c->x86_power & (1 << 8)) {
 		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
 		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
-		if (check_tsc_unstable())
-			clear_sched_clock_stable();
 	} else {
-		clear_sched_clock_stable();
+		mark_tsc_unstable("not invariant");
 	}
 
 	/* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
diff --git a/arch/x86/kernel/cpu/transmeta.c b/arch/x86/kernel/cpu/transmeta.c
index c1ea5b999839..fa243ffd1c84 100644
--- a/arch/x86/kernel/cpu/transmeta.c
+++ b/arch/x86/kernel/cpu/transmeta.c
@@ -16,7 +16,7 @@ static void early_init_transmeta(struct cpuinfo_x86 *c)
 			c->x86_capability[CPUID_8086_0001_EDX] = cpuid_edx(0x80860001);
 	}
 
-	clear_sched_clock_stable();
+	mark_tsc_unstable("not invariant");
 }
 
 static void init_transmeta(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 2724dc82f992..bf6627aff54d 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -326,9 +326,16 @@ unsigned long long sched_clock(void)
 {
 	return paravirt_sched_clock();
 }
+
+static inline bool using_native_sched_clock(void)
+{
+	return pv_time_ops.sched_clock == native_sched_clock;
+}
 #else
 unsigned long long
 sched_clock(void) __attribute__((alias("native_sched_clock")));
+
+static inline bool using_native_sched_clock(void) { return true; }
 #endif
 
 int check_tsc_unstable(void)
@@ -1112,7 +1119,8 @@ static void tsc_cs_mark_unstable(struct clocksource *cs)
 	if (tsc_unstable)
 		return;
 	tsc_unstable = 1;
-	clear_sched_clock_stable();
+	if (using_native_sched_clock())
+		clear_sched_clock_stable();
 	disable_sched_clock_irqtime();
 	pr_info("Marking TSC unstable due to clocksource watchdog\n");
 }
@@ -1134,18 +1142,19 @@ static struct clocksource clocksource_tsc = {
 
 void mark_tsc_unstable(char *reason)
 {
-	if (!tsc_unstable) {
-		tsc_unstable = 1;
+	if (tsc_unstable)
+		return;
+	tsc_unstable = 1;
+	if (using_native_sched_clock())
 		clear_sched_clock_stable();
-		disable_sched_clock_irqtime();
-		pr_info("Marking TSC unstable due to %s\n", reason);
-		/* Change only the rating, when not registered */
-		if (clocksource_tsc.mult)
-			clocksource_mark_unstable(&clocksource_tsc);
-		else {
-			clocksource_tsc.flags |= CLOCK_SOURCE_UNSTABLE;
-			clocksource_tsc.rating = 0;
-		}
+	disable_sched_clock_irqtime();
+	pr_info("Marking TSC unstable due to %s\n", reason);
+	/* Change only the rating, when not registered */
+	if (clocksource_tsc.mult)
+		clocksource_mark_unstable(&clocksource_tsc);
+	else {
+		clocksource_tsc.flags |= CLOCK_SOURCE_UNSTABLE;
+		clocksource_tsc.rating = 0;
 	}
 }

[toc] | [prev] | [next] | [standalone]

#1588833

From	Paolo Bonzini <pbonzini@redhat.com>
Date	2017-02-27 18:30 +0100
Message-ID	<tfxhv-jQ-9@gated-at.bofh.it>
In reply to	#1588800


On 27/02/2017 17:36, Peter Zijlstra wrote:
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 4e95b2e0d95f..bc3bbb6a8ab0 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -555,10 +555,8 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>  	if (c->x86_power & (1 << 8)) {
>  		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>  		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> -		if (check_tsc_unstable())
> -			clear_sched_clock_stable();
>  	} else {
> -		clear_sched_clock_stable();
> +		mark_tsc_unstable("not invariant");
>  	}
>  
>  	/* Bit 12 of 8000_0007 edx is accumulated power mechanism. */
> diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
> index 2c234a6d94c4..0fdff183aa30 100644
> --- a/arch/x86/kernel/cpu/centaur.c
> +++ b/arch/x86/kernel/cpu/centaur.c
> @@ -105,7 +105,7 @@ static void early_init_centaur(struct cpuinfo_x86 *c)
>  	set_cpu_cap(c, X86_FEATURE_SYSENTER32);
>  #endif
>  
> -	clear_sched_clock_stable();
> +	mark_tsc_unstable("not invariant");
>  }
>  
>  static void init_centaur(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index f07005e6f461..2b7ff648ea25 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -86,7 +86,7 @@ static void default_init(struct cpuinfo_x86 *c)
>  			strcpy(c->x86_model_id, "386");
>  	}
>  #endif
> -	clear_sched_clock_stable();
> +	mark_tsc_unstable("not invariant");
>  }
>  
>  static const struct cpu_dev default_cpu = {
> @@ -1076,7 +1076,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  	if (this_cpu->c_init)
>  		this_cpu->c_init(c);
>  	else
> -		clear_sched_clock_stable();
> +		mark_tsc_unstable("not invariant");
>  
>  	/* Disable the PN if appropriate */
>  	squash_the_stupid_serial_number(c);
> diff --git a/arch/x86/kernel/cpu/cyrix.c b/arch/x86/kernel/cpu/cyrix.c
> index 47416f959a48..35057d67e864 100644
> --- a/arch/x86/kernel/cpu/cyrix.c
> +++ b/arch/x86/kernel/cpu/cyrix.c
> @@ -184,7 +184,7 @@ static void early_init_cyrix(struct cpuinfo_x86 *c)
>  		set_cpu_cap(c, X86_FEATURE_CYRIX_ARR);
>  		break;
>  	}
> -	clear_sched_clock_stable();
> +	mark_tsc_unstable("not invariant");
>  }
>  
>  static void init_cyrix(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 017ecd3bb553..e0e192e43a4c 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -161,10 +161,8 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>  	if (c->x86_power & (1 << 8)) {
>  		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>  		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> -		if (check_tsc_unstable())
> -			clear_sched_clock_stable();
>  	} else {
> -		clear_sched_clock_stable();
> +		mark_tsc_unstable("not invariant");
>  	}
>  
>  	/* Penwell and Cloverview have the TSC which doesn't sleep on S3 */

Doh, these are called _before_ kvmclock_init.  But perhaps they can all
be replaced by something like this:

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 2724dc82f992..3080b6877190 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1398,6 +1398,9 @@ void __init tsc_init(void)

 	use_tsc_delay();

+	if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+		mark_tsc_unstable("not invariant");
+
 	if (unsynchronized_tsc())
 		mark_tsc_unstable("TSCs unsynchronized");

The rest seems nice.

Paolo

[toc] | [prev] | [next] | [standalone]

#1588844

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-02-27 19:10 +0100
Message-ID	<tfxUe-Oq-11@gated-at.bofh.it>
In reply to	#1588833

On Mon, 27 Feb 2017, Paolo Bonzini wrote:
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 2724dc82f992..3080b6877190 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1398,6 +1398,9 @@ void __init tsc_init(void)
> 
>  	use_tsc_delay();
> 
> +	if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
> +		mark_tsc_unstable("not invariant");

Errm, no. 

That makes TSC unusable for systems which do not go into C/P states in
which the TSC stops. There is a world outside KVM ....

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1588876

From	Paolo Bonzini <pbonzini@redhat.com>
Date	2017-02-27 20:10 +0100
Message-ID	<tfyQh-1uW-11@gated-at.bofh.it>
In reply to	#1588844


----- Original Message -----
> From: "Thomas Gleixner" <tglx@linutronix.de>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Peter Zijlstra" <peterz@infradead.org>, "Wanpeng Li" <kernellwp@gmail.com>, "Mike Galbraith" <efault@gmx.de>,
> "LKML" <linux-kernel@vger.kernel.org>, "Ingo Molnar" <mingo@elte.hu>, "Borislav Petkov" <bp@alien8.de>
> Sent: Monday, February 27, 2017 6:40:46 PM
> Subject: Re: tip.today - scheduler bam boom crash (cpu hotplug)
> 
> On Mon, 27 Feb 2017, Paolo Bonzini wrote:
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index 2724dc82f992..3080b6877190 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> > @@ -1398,6 +1398,9 @@ void __init tsc_init(void)
> > 
> >  	use_tsc_delay();
> > 
> > +	if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
> > +		mark_tsc_unstable("not invariant");
> 
> Errm, no.
> 
> That makes TSC unusable for systems which do not go into C/P states in
> which the TSC stops. There is a world outside KVM ....

Actually I was surprised too by Peter's patch, as it was adding
mark_tsc_unstable pretty much everywhere that didn't have nonstop TSC.
But hopefully it would still be okay to call clear_sched_clock_stable
in tsc_init, in the same way.

Paolo

[toc] | [prev] | [next] | [standalone]

#1589020

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-28 00:00 +0100
Message-ID	<tfCqS-3Po-9@gated-at.bofh.it>
In reply to	#1588876

On Mon, Feb 27, 2017 at 02:06:27PM -0500, Paolo Bonzini wrote:
> > > +	if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
> > > +		mark_tsc_unstable("not invariant");
> > 
> > Errm, no.
> > 
> > That makes TSC unusable for systems which do not go into C/P states in
> > which the TSC stops. There is a world outside KVM ....
> 
> Actually I was surprised too by Peter's patch, as it was adding
> mark_tsc_unstable pretty much everywhere that didn't have nonstop TSC.
> But hopefully it would still be okay to call clear_sched_clock_stable
> in tsc_init, in the same way.

I was preserving the sched_clock behaviour. That said, maybe we can
simply remove all those extra checks and only rely on the current
mark_tsc_unstable() calls.

I'll just have to sit down and consider the various cases..

[toc] | [prev] | [next] | [standalone]

#1589105

From	Wanpeng Li <kernellwp@gmail.com>
Date	2017-02-28 03:00 +0100
Message-ID	<tfFf3-5LI-3@gated-at.bofh.it>
In reply to	#1588795

2017-02-28 0:11 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 27/02/2017 16:59, Peter Zijlstra wrote:
>> OK, so if !KVM_FEATURE_CLOCKSOURCE_STABLE_BIT nothing is stable, but if
>> it is set, TSC might still not be stable, but kvm_clock_read() is.
>>
>>> However, if kvmclock is stable, we know that the sched clock is stable.
>> Right, so the problem is that we only ever want to allow marking
>> unstable -- once its found unstable, for whatever reason, we should
>> never allow going stable. The corollary of this proposition is that we
>> must start out assuming it will become stable. And to avoid actually
>> using unstable TSC we do a 3 state bringup:
>>
>>  1) sched_clock_running = 0, __stable_early = 1, __stable = 0
>>  2) sched_clock_running = 1 (__stable is effective, iow, we run unstable)
>>  3) sched_clock_running = 2 (__stable <- __stable_early)
>>
>> 2) happens 'early' but is 'safe'.
>> 3) happens 'late', after we've brought up SMP and probed TSC
>>
>> Between there, we should have detected the most common TSC wreckage and
>> made sure to not then switch to 'stable' at 3.
>>
>> Now the problem appears to be that we assume sched_clock will use RDTSC
>> (native_sched_clock) while sched_clock is a paravirt op.
>>
>> Now, I've not yet figured out the ordering between when we set
>> pv_time_ops.sched_clock and when we do the 'normal' TSC init stuff.
>
> I think the ordering is fine:
>
> - pv_time_ops.sched_clock is set here:
>
>         start_kernel (init/main.c line 509)
>           setup_arch
>             kvmclock_init
>               kvm_sched_clock_init
>
> - TSC can be declared unstable only after this:
>
>         start_kernel (init/main.c line 628)
>           late_time_init
>             tsc_init
>
> So by the time the tsc_cs_mark_unstable or mark_tsc_unstable can call
> clear_sched_clock_stable, pv_time_ops.sched_clock has been set.
>
>> But it appears to me, we should not be calling
>> clear_sched_clock_stable() on TSC bits when we don't end up using
>> native_sched_clock().
>
> Yes, this makes sense.

How about something like below, we delay sched clock stable check to
tsc_init() if we run in VM, it calls clear_sched_clock_stable() if
there is no invariant tsc bit when we end up using
native_sched_clock(), otherwise, kvmclock will determine if it is
stable depends on KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
PVCLOCK_TSC_STABLE_BIT :

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 4e95b2e..ed8eda4 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -557,7 +557,7 @@ static void early_init_amd(struct cpuinfo_x86 *c)
         set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
         if (check_tsc_unstable())
             clear_sched_clock_stable();
-    } else {
+    } else if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
         clear_sched_clock_stable();
     }

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 017ecd3..1927f5f 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -163,7 +163,7 @@ static void early_init_intel(struct cpuinfo_x86 *c)
         set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
         if (check_tsc_unstable())
             clear_sched_clock_stable();
-    } else {
+    } else if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
         clear_sched_clock_stable();
     }

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 2724dc8..68149f5 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -326,9 +326,16 @@ unsigned long long sched_clock(void)
 {
     return paravirt_sched_clock();
 }
+
+static inline bool using_native_sched_clock(void)
+{
+    return pv_time_ops.sched_clock == native_sched_clock;
+}
 #else
 unsigned long long
 sched_clock(void) __attribute__((alias("native_sched_clock")));
+
+static inline bool using_native_sched_clock(void) { return true; }
 #endif

 int check_tsc_unstable(void)
@@ -1398,6 +1405,11 @@ void __init tsc_init(void)

     use_tsc_delay();

+    if (using_native_sched_clock() &&
+        boot_cpu_has(X86_FEATURE_HYPERVISOR) &&
+        !boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+        clear_sched_clock_stable();
+
     if (unsynchronized_tsc())
         mark_tsc_unstable("TSCs unsynchronized");

Regards,
Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1589262

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-28 09:10 +0100
Message-ID	<tfL18-1He-19@gated-at.bofh.it>
In reply to	#1589105

On Tue, Feb 28, 2017 at 09:51:07AM +0800, Wanpeng Li wrote:
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 4e95b2e..ed8eda4 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -557,7 +557,7 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>          set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
>          if (check_tsc_unstable())
>              clear_sched_clock_stable();
> -    } else {
> +    } else if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
>          clear_sched_clock_stable();
>      }

That's wrong, you can have HYPERVISOR and still use
native_sched_clock() (lguest does that for one).


I suspect we can do something like the below. Since we fixed the
clocksource watchdog to mark TSC unstable, and we're already fairly
careful with using TSC for timekeeping anyway.

---
 arch/x86/kernel/cpu/amd.c       |  4 ----
 arch/x86/kernel/cpu/centaur.c   |  2 --
 arch/x86/kernel/cpu/common.c    |  3 ---
 arch/x86/kernel/cpu/cyrix.c     |  1 -
 arch/x86/kernel/cpu/intel.c     |  4 ----
 arch/x86/kernel/cpu/transmeta.c |  2 --
 arch/x86/kernel/tsc.c           | 35 +++++++++++++++++++++++------------
 7 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 35a5d5dca2fa..c36140d788fe 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -556,10 +556,6 @@ static void early_init_amd(struct cpuinfo_x86 *c)
 	if (c->x86_power & (1 << 8)) {
 		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
 		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
-		if (check_tsc_unstable())
-			clear_sched_clock_stable();
-	} else {
-		clear_sched_clock_stable();
 	}
 
 	/* Bit 12 of 8000_0007 edx is accumulated power mechanism. */
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index 106bd3318121..44207b71fee1 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -105,8 +105,6 @@ static void early_init_centaur(struct cpuinfo_x86 *c)
 #ifdef CONFIG_X86_64
 	set_cpu_cap(c, X86_FEATURE_SYSENTER32);
 #endif
-
-	clear_sched_clock_stable();
 }
 
 static void init_centaur(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index c188ae5a5d9f..0209907a63b1 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -88,7 +88,6 @@ static void default_init(struct cpuinfo_x86 *c)
 			strcpy(c->x86_model_id, "386");
 	}
 #endif
-	clear_sched_clock_stable();
 }
 
 static const struct cpu_dev default_cpu = {
@@ -1077,8 +1076,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	 */
 	if (this_cpu->c_init)
 		this_cpu->c_init(c);
-	else
-		clear_sched_clock_stable();
 
 	/* Disable the PN if appropriate */
 	squash_the_stupid_serial_number(c);
diff --git a/arch/x86/kernel/cpu/cyrix.c b/arch/x86/kernel/cpu/cyrix.c
index 0a3bc19de017..a70fd61095f8 100644
--- a/arch/x86/kernel/cpu/cyrix.c
+++ b/arch/x86/kernel/cpu/cyrix.c
@@ -185,7 +185,6 @@ static void early_init_cyrix(struct cpuinfo_x86 *c)
 		set_cpu_cap(c, X86_FEATURE_CYRIX_ARR);
 		break;
 	}
-	clear_sched_clock_stable();
 }
 
 static void init_cyrix(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fe0a615a051b..063197771b8d 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -162,10 +162,6 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 	if (c->x86_power & (1 << 8)) {
 		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
 		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
-		if (check_tsc_unstable())
-			clear_sched_clock_stable();
-	} else {
-		clear_sched_clock_stable();
 	}
 
 	/* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
diff --git a/arch/x86/kernel/cpu/transmeta.c b/arch/x86/kernel/cpu/transmeta.c
index 8457b4978668..d77d07ab310b 100644
--- a/arch/x86/kernel/cpu/transmeta.c
+++ b/arch/x86/kernel/cpu/transmeta.c
@@ -16,8 +16,6 @@ static void early_init_transmeta(struct cpuinfo_x86 *c)
 		if (xlvl >= 0x80860001)
 			c->x86_capability[CPUID_8086_0001_EDX] = cpuid_edx(0x80860001);
 	}
-
-	clear_sched_clock_stable();
 }
 
 static void init_transmeta(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 46bcda4cb1c2..5ca0f52e1ba1 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -327,9 +327,16 @@ unsigned long long sched_clock(void)
 {
 	return paravirt_sched_clock();
 }
+
+static inline bool using_native_sched_clock(void)
+{
+	return pv_time_ops.sched_clock == native_sched_clock;
+}
 #else
 unsigned long long
 sched_clock(void) __attribute__((alias("native_sched_clock")));
+
+static inline bool using_native_sched_clock(void) { return true; }
 #endif
 
 int check_tsc_unstable(void)
@@ -1112,8 +1119,10 @@ static void tsc_cs_mark_unstable(struct clocksource *cs)
 {
 	if (tsc_unstable)
 		return;
+
 	tsc_unstable = 1;
-	clear_sched_clock_stable();
+	if (using_native_sched_clock())
+		clear_sched_clock_stable();
 	disable_sched_clock_irqtime();
 	pr_info("Marking TSC unstable due to clocksource watchdog\n");
 }
@@ -1135,18 +1144,20 @@ static struct clocksource clocksource_tsc = {
 
 void mark_tsc_unstable(char *reason)
 {
-	if (!tsc_unstable) {
-		tsc_unstable = 1;
+	if (tsc_unstable)
+		return;
+
+	tsc_unstable = 1;
+	if (using_native_sched_clock())
 		clear_sched_clock_stable();
-		disable_sched_clock_irqtime();
-		pr_info("Marking TSC unstable due to %s\n", reason);
-		/* Change only the rating, when not registered */
-		if (clocksource_tsc.mult)
-			clocksource_mark_unstable(&clocksource_tsc);
-		else {
-			clocksource_tsc.flags |= CLOCK_SOURCE_UNSTABLE;
-			clocksource_tsc.rating = 0;
-		}
+	disable_sched_clock_irqtime();
+	pr_info("Marking TSC unstable due to %s\n", reason);
+	/* Change only the rating, when not registered */
+	if (clocksource_tsc.mult)
+		clocksource_mark_unstable(&clocksource_tsc);
+	else {
+		clocksource_tsc.flags |= CLOCK_SOURCE_UNSTABLE;
+		clocksource_tsc.rating = 0;
 	}
 }

[toc] | [prev] | [next] | [standalone]

#1589271

From	Wanpeng Li <kernellwp@gmail.com>
Date	2017-02-28 09:20 +0100
Message-ID	<tfLaO-1Kw-11@gated-at.bofh.it>
In reply to	#1589262

2017-02-28 16:08 GMT+08:00 Peter Zijlstra <peterz@infradead.org>:
> On Tue, Feb 28, 2017 at 09:51:07AM +0800, Wanpeng Li wrote:
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index 4e95b2e..ed8eda4 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -557,7 +557,7 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>>          set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
>>          if (check_tsc_unstable())
>>              clear_sched_clock_stable();
>> -    } else {
>> +    } else if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
>>          clear_sched_clock_stable();
>>      }
>
> That's wrong, you can have HYPERVISOR and still use
> native_sched_clock() (lguest does that for one).

My posting delay to check this in tsc_init().

Regards,
Wanpeng Li

>
>
> I suspect we can do something like the below. Since we fixed the
> clocksource watchdog to mark TSC unstable, and we're already fairly
> careful with using TSC for timekeeping anyway.
>
> ---
>  arch/x86/kernel/cpu/amd.c       |  4 ----
>  arch/x86/kernel/cpu/centaur.c   |  2 --
>  arch/x86/kernel/cpu/common.c    |  3 ---
>  arch/x86/kernel/cpu/cyrix.c     |  1 -
>  arch/x86/kernel/cpu/intel.c     |  4 ----
>  arch/x86/kernel/cpu/transmeta.c |  2 --
>  arch/x86/kernel/tsc.c           | 35 +++++++++++++++++++++++------------
>  7 files changed, 23 insertions(+), 28 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 35a5d5dca2fa..c36140d788fe 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -556,10 +556,6 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>         if (c->x86_power & (1 << 8)) {
>                 set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>                 set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> -               if (check_tsc_unstable())
> -                       clear_sched_clock_stable();
> -       } else {
> -               clear_sched_clock_stable();
>         }
>
>         /* Bit 12 of 8000_0007 edx is accumulated power mechanism. */
> diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
> index 106bd3318121..44207b71fee1 100644
> --- a/arch/x86/kernel/cpu/centaur.c
> +++ b/arch/x86/kernel/cpu/centaur.c
> @@ -105,8 +105,6 @@ static void early_init_centaur(struct cpuinfo_x86 *c)
>  #ifdef CONFIG_X86_64
>         set_cpu_cap(c, X86_FEATURE_SYSENTER32);
>  #endif
> -
> -       clear_sched_clock_stable();
>  }
>
>  static void init_centaur(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index c188ae5a5d9f..0209907a63b1 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -88,7 +88,6 @@ static void default_init(struct cpuinfo_x86 *c)
>                         strcpy(c->x86_model_id, "386");
>         }
>  #endif
> -       clear_sched_clock_stable();
>  }
>
>  static const struct cpu_dev default_cpu = {
> @@ -1077,8 +1076,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>          */
>         if (this_cpu->c_init)
>                 this_cpu->c_init(c);
> -       else
> -               clear_sched_clock_stable();
>
>         /* Disable the PN if appropriate */
>         squash_the_stupid_serial_number(c);
> diff --git a/arch/x86/kernel/cpu/cyrix.c b/arch/x86/kernel/cpu/cyrix.c
> index 0a3bc19de017..a70fd61095f8 100644
> --- a/arch/x86/kernel/cpu/cyrix.c
> +++ b/arch/x86/kernel/cpu/cyrix.c
> @@ -185,7 +185,6 @@ static void early_init_cyrix(struct cpuinfo_x86 *c)
>                 set_cpu_cap(c, X86_FEATURE_CYRIX_ARR);
>                 break;
>         }
> -       clear_sched_clock_stable();
>  }
>
>  static void init_cyrix(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index fe0a615a051b..063197771b8d 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -162,10 +162,6 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>         if (c->x86_power & (1 << 8)) {
>                 set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>                 set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> -               if (check_tsc_unstable())
> -                       clear_sched_clock_stable();
> -       } else {
> -               clear_sched_clock_stable();
>         }
>
>         /* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
> diff --git a/arch/x86/kernel/cpu/transmeta.c b/arch/x86/kernel/cpu/transmeta.c
> index 8457b4978668..d77d07ab310b 100644
> --- a/arch/x86/kernel/cpu/transmeta.c
> +++ b/arch/x86/kernel/cpu/transmeta.c
> @@ -16,8 +16,6 @@ static void early_init_transmeta(struct cpuinfo_x86 *c)
>                 if (xlvl >= 0x80860001)
>                         c->x86_capability[CPUID_8086_0001_EDX] = cpuid_edx(0x80860001);
>         }
> -
> -       clear_sched_clock_stable();
>  }
>
>  static void init_transmeta(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 46bcda4cb1c2..5ca0f52e1ba1 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -327,9 +327,16 @@ unsigned long long sched_clock(void)
>  {
>         return paravirt_sched_clock();
>  }
> +
> +static inline bool using_native_sched_clock(void)
> +{
> +       return pv_time_ops.sched_clock == native_sched_clock;
> +}
>  #else
>  unsigned long long
>  sched_clock(void) __attribute__((alias("native_sched_clock")));
> +
> +static inline bool using_native_sched_clock(void) { return true; }
>  #endif
>
>  int check_tsc_unstable(void)
> @@ -1112,8 +1119,10 @@ static void tsc_cs_mark_unstable(struct clocksource *cs)
>  {
>         if (tsc_unstable)
>                 return;
> +
>         tsc_unstable = 1;
> -       clear_sched_clock_stable();
> +       if (using_native_sched_clock())
> +               clear_sched_clock_stable();
>         disable_sched_clock_irqtime();
>         pr_info("Marking TSC unstable due to clocksource watchdog\n");
>  }
> @@ -1135,18 +1144,20 @@ static struct clocksource clocksource_tsc = {
>
>  void mark_tsc_unstable(char *reason)
>  {
> -       if (!tsc_unstable) {
> -               tsc_unstable = 1;
> +       if (tsc_unstable)
> +               return;
> +
> +       tsc_unstable = 1;
> +       if (using_native_sched_clock())
>                 clear_sched_clock_stable();
> -               disable_sched_clock_irqtime();
> -               pr_info("Marking TSC unstable due to %s\n", reason);
> -               /* Change only the rating, when not registered */
> -               if (clocksource_tsc.mult)
> -                       clocksource_mark_unstable(&clocksource_tsc);
> -               else {
> -                       clocksource_tsc.flags |= CLOCK_SOURCE_UNSTABLE;
> -                       clocksource_tsc.rating = 0;
> -               }
> +       disable_sched_clock_irqtime();
> +       pr_info("Marking TSC unstable due to %s\n", reason);
> +       /* Change only the rating, when not registered */
> +       if (clocksource_tsc.mult)
> +               clocksource_mark_unstable(&clocksource_tsc);
> +       else {
> +               clocksource_tsc.flags |= CLOCK_SOURCE_UNSTABLE;
> +               clocksource_tsc.rating = 0;
>         }
>  }
>

[toc] | [prev] | [next] | [standalone]

#1588804

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-27 17:50 +0100
Message-ID	<tfwls-86h-17@gated-at.bofh.it>
In reply to	#1588766

On Mon, Feb 27, 2017 at 04:27:32PM +0100, Paolo Bonzini wrote:
> 
> 
> On 27/02/2017 14:04, Peter Zijlstra wrote:
> >>>> This results in sched clock always unstable for kvm guest since there
> >>>> is no invariant tsc cpuid bit exposed for kvm guest currently. 
> >>> What the heck is KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
> >>> PVCLOCK_TSC_STABLE_BIT about then?
> >> It checks that all the bugs in the host have been ironed out, and that
> >> the host itself supports invtsc.
> > But what does it mean if that is not so? That is, will kvm_clock_read()
> > still be stable even if !stable?
> 
> If kvmclock is !stable, nobody should have set that the sched clock to
> stable, to begin with.

OK, so if !KVM_FEATURE_CLOCKSOURCE_STABLE_BIT nothing is stable, but if
it is set, TSC might still not be stable, but kvm_clock_read() is.

> However, if kvmclock is stable, we know that the sched clock is stable.

Right, so the problem is that we only ever want to allow marking
unstable -- once its found unstable, for whatever reason, we should
never allow going stable. The corollary of this proposition is that we
must start out assuming it will become stable. And to avoid actually
using unstable TSC we do a 3 state bringup:

 1) sched_clock_running = 0, __stable_early = 1, __stable = 0
 2) sched_clock_running = 1 (__stable is effective, iow, we run unstable)
 3) sched_clock_running = 2 (__stable <- __stable_early)

2) happens 'early' but is 'safe'.
3) happens 'late', after we've brought up SMP and probed TSC

Between there, we should have detected the most common TSC wreckage and
made sure to not then switch to 'stable' at 3.

Now the problem appears to be that we assume sched_clock will use RDTSC
(native_sched_clock) while sched_clock is a paravirt op.

Now, I've not yet figured out the ordering between when we set
pv_time_ops.sched_clock and when we do the 'normal' TSC init stuff.

But it appears to me, we should not be calling
clear_sched_clock_stable() on TSC bits when we don't end up using
native_sched_clock().

[toc] | [prev] | [next] | [standalone]

#1589015

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-27 23:50 +0100
Message-ID	<tfvz5-7vd-45@gated-at.bofh.it>
In reply to	#1588655

On Mon, Feb 27, 2017 at 01:50:29PM +0100, Paolo Bonzini wrote:
> 
> 
> On 27/02/2017 13:43, Peter Zijlstra wrote:
> > On Mon, Feb 27, 2017 at 08:30:11PM +0800, Wanpeng Li wrote:
> > 
> >>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> >>> index 2a5cafd..542710b 100644
> >>> --- a/arch/x86/kernel/kvmclock.c
> >>> +++ b/arch/x86/kernel/kvmclock.c
> >>> @@ -107,12 +107,12 @@ static inline void kvm_sched_clock_init(bool stable)
> >>>  {
> >>>         if (!stable) {
> >>>                 pv_time_ops.sched_clock = kvm_clock_read;
> >>> +               clear_sched_clock_stable();
> >>>                 return;
> >>>         }
> >>>
> >>>         kvm_sched_clock_offset = kvm_clock_read();
> >>>         pv_time_ops.sched_clock = kvm_sched_clock_read;
> >>> -       set_sched_clock_stable();
> >>
> >> This results in sched clock always unstable for kvm guest since there
> >> is no invariant tsc cpuid bit exposed for kvm guest currently. 
> > 
> > What the heck is KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
> > PVCLOCK_TSC_STABLE_BIT about then?
> 
> It checks that all the bugs in the host have been ironed out, and that
> the host itself supports invtsc.

But what does it mean if that is not so? That is, will kvm_clock_read()
still be stable even if !stable?

[toc] | [prev] | [next] | [standalone]

#1588669

From	Wanpeng Li <kernellwp@gmail.com>
Date	2017-02-27 15:00 +0100
Message-ID	<tfu0i-6lr-9@gated-at.bofh.it>
In reply to	#1588646

2017-02-27 20:43 GMT+08:00 Peter Zijlstra <peterz@infradead.org>:
> On Mon, Feb 27, 2017 at 08:30:11PM +0800, Wanpeng Li wrote:
>
>> > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>> > index 2a5cafd..542710b 100644
>> > --- a/arch/x86/kernel/kvmclock.c
>> > +++ b/arch/x86/kernel/kvmclock.c
>> > @@ -107,12 +107,12 @@ static inline void kvm_sched_clock_init(bool stable)
>> >  {
>> >         if (!stable) {
>> >                 pv_time_ops.sched_clock = kvm_clock_read;
>> > +               clear_sched_clock_stable();
>> >                 return;
>> >         }
>> >
>> >         kvm_sched_clock_offset = kvm_clock_read();
>> >         pv_time_ops.sched_clock = kvm_sched_clock_read;
>> > -       set_sched_clock_stable();
>>
>> This results in sched clock always unstable for kvm guest since there
>> is no invariant tsc cpuid bit exposed for kvm guest currently.
>
> What the heck is KVM_FEATURE_CLOCKSOURCE_STABLE_BIT /
> PVCLOCK_TSC_STABLE_BIT about then?
>
>> How about something like below(untested):
>
> I have no clue what that tries to do.

The patch tries to mark sched clock stable if
KVM_FEATURE_CLOCKSOURCE_STABLE_BIT / PVCLOCK_TSC_STABLE_BIT is set in
kvm guest even if there is no invariant tsc cpuid bit.

Regards,
Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1588697

From	Peter Zijlstra <peterz@infradead.org>
Date	2017-02-27 15:50 +0100
Message-ID	<tfuMH-6Wf-39@gated-at.bofh.it>
In reply to	#1588669

On Mon, Feb 27, 2017 at 09:48:35PM +0800, Wanpeng Li wrote:
> 2017-02-27 20:43 GMT+08:00 Peter Zijlstra <peterz@infradead.org>:
> > I have no clue what that tries to do.
> 
> The patch tries to mark sched clock stable if
> KVM_FEATURE_CLOCKSOURCE_STABLE_BIT / PVCLOCK_TSC_STABLE_BIT is set in
> kvm guest even if there is no invariant tsc cpuid bit.

I just ripped out the interface to 'set stable' for good reasons. Also,
the branch you add it to doesn't in fact do clear_stable -- so who does?

Also, the sched/clock.c code shouldn't know about hypervisor anything.

So whatever it is you're doing, you're doing it wrong.

[toc] | [prev] | [next] | [standalone]

#1588714

From	Paolo Bonzini <pbonzini@redhat.com>
Date	2017-02-27 16:10 +0100
Message-ID	<tfv62-7iD-15@gated-at.bofh.it>
In reply to	#1588645


On 27/02/2017 13:30, Wanpeng Li wrote:
> This results in sched clock always unstable for kvm guest since there
> is no invariant tsc cpuid bit exposed for kvm guest currently. The
> blockage happened for several reasons:
> 
> 1) Migration: to host with different TSC frequency.
> 2) Savevm: It is not safe to use the TSC for wall clock timer services.

Right, the purpose of kvmclock is basically to ensure a stable clock
without using TSC.  However, I know zilch about kernel/sched/clock.c so
I cannot really understand your patch.

Paolo

> How about something like below(untested):
> 
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index bae6ea6..a61c477 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -115,6 +115,7 @@ static inline void kvm_sched_clock_init(bool stable)
> 
>      kvm_sched_clock_offset = kvm_clock_read();
>      pv_time_ops.sched_clock = kvm_sched_clock_read;
> +    hypervisor_sched_clock_stable();
> 
>      printk(KERN_INFO "kvm-clock: using sched offset of %llu cycles\n",
>              kvm_sched_clock_offset);
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 451e241..38c6edb 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2499,6 +2499,10 @@ static inline void clear_sched_clock_stable(void)
>  {
>  }
> 
> +static inline void hypervisor_sched_clock_stable(void)
> +{
> +}
> +
>  static inline void sched_clock_idle_sleep_event(void)
>  {
>  }
> @@ -2526,6 +2530,7 @@ extern void sched_clock_init_late(void);
>   */
>  extern int sched_clock_stable(void);
>  extern void clear_sched_clock_stable(void);
> +extern void hypervisor_sched_clock_stable(void);
> 
>  extern void sched_clock_tick(void);
>  extern void sched_clock_idle_sleep_event(void);
> diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
> index ad64efe..a46639e 100644
> --- a/kernel/sched/clock.c
> +++ b/kernel/sched/clock.c
> @@ -77,9 +77,15 @@ EXPORT_SYMBOL_GPL(sched_clock);
> 
>  __read_mostly int sched_clock_running;
> 
> +enum {
> +    SCHED_CLOCK_INIT = 0,
> +    HYPERVISOR_SCHED_CLOCK_STABLE,
> +    SCHED_CLOCK_INIT_LATE
> +};
> +
>  void sched_clock_init(void)
>  {
> -    sched_clock_running = 1;
> +    sched_clock_running = SCHED_CLOCK_INIT;
>  }
> 
>  #ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK
> @@ -170,13 +176,14 @@ void clear_sched_clock_stable(void)
> 
>      smp_mb(); /* matches sched_clock_init_late() */
> 
> -    if (sched_clock_running == 2)
> +    if (sched_clock_running == SCHED_CLOCK_INIT_LATE)
>          schedule_work(&sched_clock_work);
>  }
> 
>  void sched_clock_init_late(void)
>  {
> -    sched_clock_running = 2;
> +    if (sched_clock_running == SCHED_CLOCK_INIT)
> +        sched_clock_running = SCHED_CLOCK_INIT_LATE;
>      /*
>       * Ensure that it is impossible to not do a static_key update.
>       *
> @@ -186,8 +193,15 @@ void sched_clock_init_late(void)
>       */
>      smp_mb(); /* matches {set,clear}_sched_clock_stable() */
> 
> -    if (__sched_clock_stable_early)
> +    if (__sched_clock_stable_early ||
> +        sched_clock_running == HYPERVISOR_SCHED_CLOCK_STABLE) {
>          __set_sched_clock_stable();
> +    }
> +}
> +
> +void hypervisor_sched_clock_stable()
> +{
> +    sched_clock_running = HYPERVISOR_SCHED_CLOCK_STABLE;
>  }
> 
>  /*

[toc] | [prev] | [standalone]

csiph-web

Re: tip.today - scheduler bam boom crash (cpu hotplug)

Contents

#1588645 — Re: tip.today - scheduler bam boom crash (cpu hotplug)

#1588646

#1588655

#1588766

#1588795

#1588800

#1588833

#1588844

#1588876

#1589020

#1589105

#1589262

#1589271

#1588804

#1589015

#1588669

#1588697

#1588714