Re: PATCH v2 clocksource 8/7] clocksource: Enable TSC watchdog checking of HPET and PMTMR only when requested

Paul E. McKenney
2023-02-07T01:08:40Z

On Mon, Feb 06, 2023 at 02:57:55PM -0500, Waiman Long wrote:
> On 2/2/23 23:36, Paul E. McKenney wrote:
> > Unconditionally enabling TSC watchdog checking of the HPET and PMTMR
> > clocksources can degrade latency and performance. Therefore, provide
> > a new "watchdog" option to the tsc= boot parameter that opts into such
> > checking. Note that tsc=watchdog is overridden by a tsc=nowatchdog
> > regardless of their relative positions in the list of boot parameters.
> >
> > Reported-by: Thomas Gleixner <tglx@linutronix.de>
> > Reported-by: Waiman Long <longman@redhat.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 95f0d104c2322..7b4df6d89d3c3 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -6373,6 +6373,12 @@
> > (HPET or PM timer) on systems whose TSC frequency was
> > obtained from HW or FW using either an MSR or CPUID(0x15).
> > Warn if the difference is more than 500 ppm.
> > + [x86] watchdog: Use TSC as the watchdog clocksource with
> > + which to check other HW timers (HPET or PM timer), but
> > + only on systems where TSC has been deemed trustworthy.
> > + This will be suppressed by an earlier tsc=nowatchdog and
> > + can be overridden by a later tsc=nowatchdog. A console
> > + message will flag any such suppression or overriding.
> > tsc_early_khz= [X86] Skip early TSC calibration and use the given
> > value instead. Useful when the early TSC frequency discovery
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index a5371c6d4b64b..306c233c98d84 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> > @@ -294,6 +294,7 @@ __setup("notsc", notsc_setup);
> > static int no_sched_irq_time;
> > static int no_tsc_watchdog;
> > +static int tsc_as_watchdog;
> > static int __init tsc_setup(char *str)
> > {
> > @@ -303,10 +304,22 @@ static int __init tsc_setup(char *str)
> > no_sched_irq_time = 1;
> > if (!strcmp(str, "unstable"))
> > mark_tsc_unstable("boot parameter");
> > - if (!strcmp(str, "nowatchdog"))
> > + if (!strcmp(str, "nowatchdog")) {
> > no_tsc_watchdog = 1;
> > + if (tsc_as_watchdog)
> > + pr_alert("%s: Overriding earlier tsc=watchdog with tsc=nowatchdog\n",
> > + __func__);
> > + tsc_as_watchdog = 0;
> > + }
> > if (!strcmp(str, "recalibrate"))
> > tsc_force_recalibrate = 1;
> > + if (!strcmp(str, "watchdog")) {
> > + if (no_tsc_watchdog)
> > + pr_alert("%s: tsc=watchdog overridden by earlier tsc=nowatchdog\n",
> > + __func__);
> > + else
> > + tsc_as_watchdog = 1;
> > + }
> > return 1;
> > }
> > @@ -1192,7 +1205,8 @@ static void __init tsc_disable_clocksource_watchdog(void)
> > bool tsc_clocksource_watchdog_disabled(void)
> > {
> > - return !(clocksource_tsc.flags & CLOCK_SOURCE_MUST_VERIFY);
> > + return !(clocksource_tsc.flags & CLOCK_SOURCE_MUST_VERIFY) &&
> > + tsc_as_watchdog && !no_tsc_watchdog;
> > }
> > static void __init check_system_tsc_reliable(void)
> >
> Acked-by: Waiman Long <longman@redhat.com>

Applied, thank you!

Thanx, Paul