Messages in this thread | | | Date | Thu, 26 Jan 2023 11:57:41 +0100 | Subject | Re: [PATCH v2 clocksource 6/7] clocksource: Verify HPET and PMTMR when TSC unverified | From | Daniel Lezcano <> |
| |
Hi Thomas,
are you ok with this patch ? Shall I pick it ?
Thanks
-- Daniel
On 25/01/2023 01:27, Paul E. McKenney wrote: > On systems with two or fewer sockets, when the boot CPU has CONSTANT_TSC, > NONSTOP_TSC, and TSC_ADJUST, clocksource watchdog verification of the > TSC is disabled. This works well much of the time, but there is the > occasional production-level system that meets all of these criteria, but > which still has a TSC that skews significantly from atomic-clock time. > This is usually attributed to a firmware or hardware fault. Yes, the > various NTP daemons do express their opinions of userspace-to-atomic-clock > time skew, but they put them in various places, depending on the daemon > and distro in question. It would therefore be good for the kernel to > have some clue that there is a problem. > > The old behavior of marking the TSC unstable is a non-starter because a > great many workloads simply cannot tolerate the overheads and latencies > of the various non-TSC clocksources. In addition, NTP-corrected systems > sometimes can tolerate significant kernel-space time skew as long as > the userspace time sources are within epsilon of atomic-clock time. > > Therefore, when watchdog verification of TSC is disabled, enable it for > HPET and PMTMR (AKA ACPI PM timer). This provides the needed in-kernel > time-skew diagnostic without degrading the system's performance. > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org> > Cc: Waiman Long <longman@redhat.com> > Cc: <x86@kernel.org> > Tested-by: Feng Tang <feng.tang@intel.com> > --- > arch/x86/include/asm/time.h | 1 + > arch/x86/kernel/hpet.c | 2 ++ > arch/x86/kernel/tsc.c | 5 +++++ > drivers/clocksource/acpi_pm.c | 6 ++++-- > 4 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/time.h b/arch/x86/include/asm/time.h > index 8ac563abb567b..a53961c64a567 100644 > --- a/arch/x86/include/asm/time.h > +++ b/arch/x86/include/asm/time.h > @@ -8,6 +8,7 @@ > extern void hpet_time_init(void); > extern void time_init(void); > extern bool pit_timer_init(void); > +extern bool tsc_clocksource_watchdog_disabled(void); > > extern struct clock_event_device *global_clock_event; > > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c > index 71f336425e58a..c8eb1ac5125ab 100644 > --- a/arch/x86/kernel/hpet.c > +++ b/arch/x86/kernel/hpet.c > @@ -1091,6 +1091,8 @@ int __init hpet_enable(void) > if (!hpet_counting()) > goto out_nohpet; > > + if (tsc_clocksource_watchdog_disabled()) > + clocksource_hpet.flags |= CLOCK_SOURCE_MUST_VERIFY; > clocksource_register_hz(&clocksource_hpet, (u32)hpet_freq); > > if (id & HPET_ID_LEGSUP) { > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > index a78e73da4a74b..af3782fb6200c 100644 > --- a/arch/x86/kernel/tsc.c > +++ b/arch/x86/kernel/tsc.c > @@ -1186,6 +1186,11 @@ static void __init tsc_disable_clocksource_watchdog(void) > clocksource_tsc.flags &= ~CLOCK_SOURCE_MUST_VERIFY; > } > > +bool tsc_clocksource_watchdog_disabled(void) > +{ > + return !(clocksource_tsc.flags & CLOCK_SOURCE_MUST_VERIFY); > +} > + > static void __init check_system_tsc_reliable(void) > { > #if defined(CONFIG_MGEODEGX1) || defined(CONFIG_MGEODE_LX) || defined(CONFIG_X86_GENERIC) > diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c > index 279ddff81ab49..82338773602ca 100644 > --- a/drivers/clocksource/acpi_pm.c > +++ b/drivers/clocksource/acpi_pm.c > @@ -23,6 +23,7 @@ > #include <linux/pci.h> > #include <linux/delay.h> > #include <asm/io.h> > +#include <asm/time.h> > > /* > * The I/O port the PMTMR resides at. > @@ -210,8 +211,9 @@ static int __init init_acpi_pm_clocksource(void) > return -ENODEV; > } > > - return clocksource_register_hz(&clocksource_acpi_pm, > - PMTMR_TICKS_PER_SEC); > + if (tsc_clocksource_watchdog_disabled()) > + clocksource_acpi_pm.flags |= CLOCK_SOURCE_MUST_VERIFY; > + return clocksource_register_hz(&clocksource_acpi_pm, PMTMR_TICKS_PER_SEC); > } > > /* We use fs_initcall because we want the PCI fixups to have run
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |