From: Adrian Hunter <adrian.hunter@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
Nicholas Piggin <npiggin@gmail.com>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
"Aneesh Kumar K.V" <aneesh.kumar@kernel.org>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Andy Lutomirski <luto@kernel.org>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
John Stultz <jstultz@google.com>, Stephen Boyd <sboyd@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Randy Dunlap <rdunlap@infradead.org>,
Bjorn Helgaas <bhelgaas@google.com>,
Arnd Bergmann <arnd@arndb.de>,
Anna-Maria Behnsen <anna-maria@linutronix.de>,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-s390@vger.kernel.org
Subject: [PATCH V2 19/19] clocksource: Make watchdog and suspend-timing multiplication overflow safe
Date: Mon, 25 Mar 2024 08:40:23 +0200 [thread overview]
Message-ID: <20240325064023.2997-20-adrian.hunter@intel.com> (raw)
In-Reply-To: <20240325064023.2997-1-adrian.hunter@intel.com>
Kernel timekeeping is designed to keep the change in cycles (since the last
timer interrupt) below max_cycles, which prevents multiplication overflow
when converting cycles to nanoseconds. However, if timer interrupts stop,
the clocksource_cyc2ns() calculation will eventually overflow.
Add protection against that. Simplify by folding together
clocksource_delta() and clocksource_cyc2ns() into cycles_to_nsec_safe().
Check against max_cycles, falling back to a slower higher precision
calculation.
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
kernel/time/clocksource.c | 42 +++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index e5b260aa0e02..4d50d53ac719 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -20,6 +20,16 @@
#include "tick-internal.h"
#include "timekeeping_internal.h"
+static noinline u64 cycles_to_nsec_safe(struct clocksource *cs, u64 start, u64 end)
+{
+ u64 delta = clocksource_delta(end, start, cs->mask);
+
+ if (likely(delta < cs->max_cycles))
+ return clocksource_cyc2ns(delta, cs->mult, cs->shift);
+
+ return mul_u64_u32_shr(delta, cs->mult, cs->shift);
+}
+
/**
* clocks_calc_mult_shift - calculate mult/shift factors for scaled math of clocks
* @mult: pointer to mult variable
@@ -222,8 +232,8 @@ enum wd_read_status {
static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow, u64 *wdnow)
{
unsigned int nretries, max_retries;
- u64 wd_end, wd_end2, wd_delta;
int64_t wd_delay, wd_seq_delay;
+ u64 wd_end, wd_end2;
max_retries = clocksource_get_max_watchdog_retry();
for (nretries = 0; nretries <= max_retries; nretries++) {
@@ -234,9 +244,7 @@ static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow,
wd_end2 = watchdog->read(watchdog);
local_irq_enable();
- wd_delta = clocksource_delta(wd_end, *wdnow, watchdog->mask);
- wd_delay = clocksource_cyc2ns(wd_delta, watchdog->mult,
- watchdog->shift);
+ wd_delay = cycles_to_nsec_safe(watchdog, *wdnow, wd_end);
if (wd_delay <= WATCHDOG_MAX_SKEW) {
if (nretries > 1 || nretries >= max_retries) {
pr_warn("timekeeping watchdog on CPU%d: %s retried %d times before success\n",
@@ -254,8 +262,7 @@ static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow,
* report system busy, reinit the watchdog and skip the current
* watchdog test.
*/
- wd_delta = clocksource_delta(wd_end2, wd_end, watchdog->mask);
- wd_seq_delay = clocksource_cyc2ns(wd_delta, watchdog->mult, watchdog->shift);
+ wd_seq_delay = cycles_to_nsec_safe(watchdog, wd_end, wd_end2);
if (wd_seq_delay > WATCHDOG_MAX_SKEW/2)
goto skip_test;
}
@@ -366,8 +373,7 @@ void clocksource_verify_percpu(struct clocksource *cs)
delta = (csnow_end - csnow_mid) & cs->mask;
if (delta < 0)
cpumask_set_cpu(cpu, &cpus_ahead);
- delta = clocksource_delta(csnow_end, csnow_begin, cs->mask);
- cs_nsec = clocksource_cyc2ns(delta, cs->mult, cs->shift);
+ cs_nsec = cycles_to_nsec_safe(cs, csnow_begin, csnow_end);
if (cs_nsec > cs_nsec_max)
cs_nsec_max = cs_nsec;
if (cs_nsec < cs_nsec_min)
@@ -398,8 +404,8 @@ static inline void clocksource_reset_watchdog(void)
static void clocksource_watchdog(struct timer_list *unused)
{
- u64 csnow, wdnow, cslast, wdlast, delta;
int64_t wd_nsec, cs_nsec, interval;
+ u64 csnow, wdnow, cslast, wdlast;
int next_cpu, reset_pending;
struct clocksource *cs;
enum wd_read_status read_ret;
@@ -456,12 +462,8 @@ static void clocksource_watchdog(struct timer_list *unused)
continue;
}
- delta = clocksource_delta(wdnow, cs->wd_last, watchdog->mask);
- wd_nsec = clocksource_cyc2ns(delta, watchdog->mult,
- watchdog->shift);
-
- delta = clocksource_delta(csnow, cs->cs_last, cs->mask);
- cs_nsec = clocksource_cyc2ns(delta, cs->mult, cs->shift);
+ wd_nsec = cycles_to_nsec_safe(watchdog, cs->wd_last, wdnow);
+ cs_nsec = cycles_to_nsec_safe(cs, cs->cs_last, csnow);
wdlast = cs->wd_last; /* save these in case we print them */
cslast = cs->cs_last;
cs->cs_last = csnow;
@@ -832,7 +834,7 @@ void clocksource_start_suspend_timing(struct clocksource *cs, u64 start_cycles)
*/
u64 clocksource_stop_suspend_timing(struct clocksource *cs, u64 cycle_now)
{
- u64 now, delta, nsec = 0;
+ u64 now, nsec = 0;
if (!suspend_clocksource)
return 0;
@@ -847,12 +849,8 @@ u64 clocksource_stop_suspend_timing(struct clocksource *cs, u64 cycle_now)
else
now = suspend_clocksource->read(suspend_clocksource);
- if (now > suspend_start) {
- delta = clocksource_delta(now, suspend_start,
- suspend_clocksource->mask);
- nsec = mul_u64_u32_shr(delta, suspend_clocksource->mult,
- suspend_clocksource->shift);
- }
+ if (now > suspend_start)
+ nsec = cycles_to_nsec_safe(suspend_clocksource, suspend_start, now);
/*
* Disable the suspend timer to save power if current clocksource is
--
2.34.1
WARNING: multiple messages have this Message-ID (diff)
From: Adrian Hunter <adrian.hunter@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
John Stultz <jstultz@google.com>,
"H. Peter Anvin" <hpa@zytor.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
linux-s390@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
x86@kernel.org, "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>,
Ingo Molnar <mingo@redhat.com>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Nicholas Piggin <npiggin@gmail.com>,
Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
Anna-Maria Behnsen <anna-maria@linutronix.de>,
Stephen Boyd <sboyd@kernel.org>,
Randy Dunlap <rdunlap@infradead.org>,
linux-kernel@vger.kernel.org, Sven Schnelle <svens@linux.ibm.com>,
linuxppc-dev@lists.ozlabs.org
Subject: [PATCH V2 19/19] clocksource: Make watchdog and suspend-timing multiplication overflow safe
Date: Mon, 25 Mar 2024 08:40:23 +0200 [thread overview]
Message-ID: <20240325064023.2997-20-adrian.hunter@intel.com> (raw)
In-Reply-To: <20240325064023.2997-1-adrian.hunter@intel.com>
Kernel timekeeping is designed to keep the change in cycles (since the last
timer interrupt) below max_cycles, which prevents multiplication overflow
when converting cycles to nanoseconds. However, if timer interrupts stop,
the clocksource_cyc2ns() calculation will eventually overflow.
Add protection against that. Simplify by folding together
clocksource_delta() and clocksource_cyc2ns() into cycles_to_nsec_safe().
Check against max_cycles, falling back to a slower higher precision
calculation.
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
kernel/time/clocksource.c | 42 +++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index e5b260aa0e02..4d50d53ac719 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -20,6 +20,16 @@
#include "tick-internal.h"
#include "timekeeping_internal.h"
+static noinline u64 cycles_to_nsec_safe(struct clocksource *cs, u64 start, u64 end)
+{
+ u64 delta = clocksource_delta(end, start, cs->mask);
+
+ if (likely(delta < cs->max_cycles))
+ return clocksource_cyc2ns(delta, cs->mult, cs->shift);
+
+ return mul_u64_u32_shr(delta, cs->mult, cs->shift);
+}
+
/**
* clocks_calc_mult_shift - calculate mult/shift factors for scaled math of clocks
* @mult: pointer to mult variable
@@ -222,8 +232,8 @@ enum wd_read_status {
static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow, u64 *wdnow)
{
unsigned int nretries, max_retries;
- u64 wd_end, wd_end2, wd_delta;
int64_t wd_delay, wd_seq_delay;
+ u64 wd_end, wd_end2;
max_retries = clocksource_get_max_watchdog_retry();
for (nretries = 0; nretries <= max_retries; nretries++) {
@@ -234,9 +244,7 @@ static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow,
wd_end2 = watchdog->read(watchdog);
local_irq_enable();
- wd_delta = clocksource_delta(wd_end, *wdnow, watchdog->mask);
- wd_delay = clocksource_cyc2ns(wd_delta, watchdog->mult,
- watchdog->shift);
+ wd_delay = cycles_to_nsec_safe(watchdog, *wdnow, wd_end);
if (wd_delay <= WATCHDOG_MAX_SKEW) {
if (nretries > 1 || nretries >= max_retries) {
pr_warn("timekeeping watchdog on CPU%d: %s retried %d times before success\n",
@@ -254,8 +262,7 @@ static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow,
* report system busy, reinit the watchdog and skip the current
* watchdog test.
*/
- wd_delta = clocksource_delta(wd_end2, wd_end, watchdog->mask);
- wd_seq_delay = clocksource_cyc2ns(wd_delta, watchdog->mult, watchdog->shift);
+ wd_seq_delay = cycles_to_nsec_safe(watchdog, wd_end, wd_end2);
if (wd_seq_delay > WATCHDOG_MAX_SKEW/2)
goto skip_test;
}
@@ -366,8 +373,7 @@ void clocksource_verify_percpu(struct clocksource *cs)
delta = (csnow_end - csnow_mid) & cs->mask;
if (delta < 0)
cpumask_set_cpu(cpu, &cpus_ahead);
- delta = clocksource_delta(csnow_end, csnow_begin, cs->mask);
- cs_nsec = clocksource_cyc2ns(delta, cs->mult, cs->shift);
+ cs_nsec = cycles_to_nsec_safe(cs, csnow_begin, csnow_end);
if (cs_nsec > cs_nsec_max)
cs_nsec_max = cs_nsec;
if (cs_nsec < cs_nsec_min)
@@ -398,8 +404,8 @@ static inline void clocksource_reset_watchdog(void)
static void clocksource_watchdog(struct timer_list *unused)
{
- u64 csnow, wdnow, cslast, wdlast, delta;
int64_t wd_nsec, cs_nsec, interval;
+ u64 csnow, wdnow, cslast, wdlast;
int next_cpu, reset_pending;
struct clocksource *cs;
enum wd_read_status read_ret;
@@ -456,12 +462,8 @@ static void clocksource_watchdog(struct timer_list *unused)
continue;
}
- delta = clocksource_delta(wdnow, cs->wd_last, watchdog->mask);
- wd_nsec = clocksource_cyc2ns(delta, watchdog->mult,
- watchdog->shift);
-
- delta = clocksource_delta(csnow, cs->cs_last, cs->mask);
- cs_nsec = clocksource_cyc2ns(delta, cs->mult, cs->shift);
+ wd_nsec = cycles_to_nsec_safe(watchdog, cs->wd_last, wdnow);
+ cs_nsec = cycles_to_nsec_safe(cs, cs->cs_last, csnow);
wdlast = cs->wd_last; /* save these in case we print them */
cslast = cs->cs_last;
cs->cs_last = csnow;
@@ -832,7 +834,7 @@ void clocksource_start_suspend_timing(struct clocksource *cs, u64 start_cycles)
*/
u64 clocksource_stop_suspend_timing(struct clocksource *cs, u64 cycle_now)
{
- u64 now, delta, nsec = 0;
+ u64 now, nsec = 0;
if (!suspend_clocksource)
return 0;
@@ -847,12 +849,8 @@ u64 clocksource_stop_suspend_timing(struct clocksource *cs, u64 cycle_now)
else
now = suspend_clocksource->read(suspend_clocksource);
- if (now > suspend_start) {
- delta = clocksource_delta(now, suspend_start,
- suspend_clocksource->mask);
- nsec = mul_u64_u32_shr(delta, suspend_clocksource->mult,
- suspend_clocksource->shift);
- }
+ if (now > suspend_start)
+ nsec = cycles_to_nsec_safe(suspend_clocksource, suspend_start, now);
/*
* Disable the suspend timer to save power if current clocksource is
--
2.34.1
next prev parent reply other threads:[~2024-03-25 6:42 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-25 6:40 [PATCH V2 00/19] timekeeping: Handle potential multiplication overflow Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 01/19] vdso: Consolidate vdso_calc_delta() Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 02/19] vdso: Consolidate nanoseconds calculation Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 03/19] vdso: Add CONFIG_GENERIC_VDSO_OVERFLOW_PROTECT Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 04/19] math64: Tidy mul_u64_u32_shr() Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] math64: Tidy up mul_u64_u32_shr() tip-bot2 for Adrian Hunter
2024-04-24 15:11 ` [PATCH V2 04/19] math64: Tidy mul_u64_u32_shr() Peter Zijlstra
2024-04-24 15:11 ` Peter Zijlstra
2024-03-25 6:40 ` [PATCH V2 05/19] vdso: math64: Provide mul_u64_u32_add_u64_shr() Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] vdso, " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 06/19] vdso: Add vdso_data::max_cycles Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] vdso: Add vdso_data:: Max_cycles tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 07/19] vdso: Make delta calculation overflow safe Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 08/19] x86/vdso: " Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 09/19] timekeeping: Move timekeeping helper functions Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 10/19] timekeeping: Rename fast_tk_get_delta_ns() to __timekeeping_get_ns() Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 11/19] timekeeping: Tidy timekeeping_cycles_to_ns() slightly Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 12/19] timekeeping: Reuse timekeeping_cycles_to_ns() Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 13/19] timekeeping: Refactor timekeeping helpers Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 14/19] timekeeping: Consolidate " Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 15/19] timekeeping: Fold in timekeeping_delta_to_ns() Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 16/19] timekeeping: Prepare timekeeping_cycles_to_ns() for overflow safety Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 17/19] timekeeping: Make delta calculation overflow safe Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` [PATCH V2 18/19] timekeeping: Let timekeeping_cycles_to_ns() handle both under and overflow Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 6:40 ` Adrian Hunter [this message]
2024-03-25 6:40 ` [PATCH V2 19/19] clocksource: Make watchdog and suspend-timing multiplication overflow safe Adrian Hunter
2024-04-08 13:10 ` [tip: timers/core] " tip-bot2 for Adrian Hunter
2024-03-25 18:11 ` [PATCH V2 00/19] timekeeping: Handle potential multiplication overflow Arnd Bergmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240325064023.2997-20-adrian.hunter@intel.com \
--to=adrian.hunter@intel.com \
--cc=agordeev@linux.ibm.com \
--cc=aneesh.kumar@kernel.org \
--cc=anna-maria@linutronix.de \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=borntraeger@linux.ibm.com \
--cc=bp@alien8.de \
--cc=christophe.leroy@csgroup.eu \
--cc=dave.hansen@linux.intel.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=hpa@zytor.com \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=mpe@ellerman.id.au \
--cc=naveen.n.rao@linux.ibm.com \
--cc=npiggin@gmail.com \
--cc=peterz@infradead.org \
--cc=rdunlap@infradead.org \
--cc=sboyd@kernel.org \
--cc=svens@linux.ibm.com \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@arm.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.