PATCH v2 clocksource 8/7] clocksource: Enable TSC watchdog checking of HPET and PMTMR only when requested
Commit Message
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>
Comments
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>
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
@@ -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
@@ -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)