timer: Prevent base->clk from moving backward
Frederic Weisbecker
maintainerPATCH 1/1 timer: Prevent base->clk from moving backward
When a timer is enqueued with a negative delta (ie: expiry is below base->clk), it gets added to the wheel as expiring now (base->clk).
Yet the value that gets stored in base->next_expiry, while calling trigger_dyntick_cpu(), is the initial timer->expires value. The resulting state becomes:
base->next_expiry < base->clk
On the next timer enqueue, forward_timer_base() may accidentally rewind base->clk. As a possible outcome, timers may expire way too early, the worst case being that the highest wheel levels get spuriously processed again.
To prevent from that, make sure that base->next_expiry doesn't get below base->clk.
--- kernel/time/timer.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
kernel/time/timer.c +14 -3
@@ -584,7 +584,15 @@ trigger_dyntick_cpu(struct timer_base *base, struct timer_list *timer)* Set the next expiry time and kick the CPU so it can reevaluate the * wheel: */ - base->next_expiry = timer->expires; +if (time_before (timer->expires, base->clk)) { +/* +* Prevent from forward_timer_base() moving the base->clk +* backward +*/ + base->next_expiry = base->clk; + }else { + base->next_expiry = timer->expires; + }wake_up_nohz_cpu (base->cpu); }
@@ -896,10 +904,13 @@ static inline void forward_timer_base(struct timer_base *base)* If the next expiry value is > jiffies, then we fast forward to * jiffies otherwise we forward to the next expiry value. */ -if (time_after (base->next_expiry, jnow)) +if (time_after (base->next_expiry, jnow)) { base->clk = jnow; -else + }else { +if (WARN_ON_ONCE (time_before (base->next_expiry, base->clk))) +return ; base->clk = base->next_expiry; + }#endif }
Signature · 1 line
2.26.2
Hi,
Quoted 17 lines · Frederic Weisbecker
When a timer is enqueued with a negative delta (ie: expiry is below
base->clk), it gets added to the wheel as expiring now (base->clk).Yet the value that gets stored in
base->next_expiry, while callingtrigger_dyntick_cpu(), is the initialtimer->expiresvalue. The resulting state becomes:base->next_expiry < base->clkOn the next timer enqueue,
forward_timer_base()may accidentally rewindbase->clk. As a possible outcome, timers may expire way too early, the worst case being that the highest wheel levels get spuriously processed again.To prevent from that, make sure that
base->next_expirydoesn't get belowbase->clk.
Quoted 6 lines
Fixes a683f390b93f ("timers: Forward the wheel clock whenever possible")Signed-off-by Frederic Weisbecker <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Anna-Maria Gleixner <[email protected]>Cc Juri Lelli <[email protected]>---
Thanks,
Anna-Maria
The following commit has been merged into the timers/urgent branch of tip:
The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: 30c66fc30ee7a98c4f3adf5fb7e213b61884474f Gitweb: https://git.kernel.org/tip/30c66fc30ee7a98c4f3adf5fb7e213b61884474f Author: Frederic Weisbecker <[email protected]> AuthorDate: Fri, 03 Jul 2020 03:06:57 +02:00 Committer: Thomas Gleixner <[email protected]> CommitterDate: Thu, 09 Jul 2020 11:56:57 +02:00
timer: Prevent base->clk from moving backward
When a timer is enqueued with a negative delta (ie: expiry is below base->clk), it gets added to the wheel as expiring now (base->clk).
Yet the value that gets stored in base->next_expiry, while calling trigger_dyntick_cpu(), is the initial timer->expires value. The resulting state becomes:
base->next_expiry < base->clk
On the next timer enqueue, forward_timer_base() may accidentally rewind base->clk. As a possible outcome, timers may expire way too early, the worst case being that the highest wheel levels get spuriously processed again.
To prevent from that, make sure that base->next_expiry doesn't get below base->clk.
--- kernel/time/timer.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
kernel/time/timer.c +14 -3
@@ -584,7 +584,15 @@ trigger_dyntick_cpu(struct timer_base *base, struct timer_list *timer)* Set the next expiry time and kick the CPU so it can reevaluate the * wheel: */ - base->next_expiry = timer->expires; +if (time_before (timer->expires, base->clk)) { +/* +* Prevent from forward_timer_base() moving the base->clk +* backward +*/ + base->next_expiry = base->clk; + }else { + base->next_expiry = timer->expires; + }wake_up_nohz_cpu (base->cpu); }
@@ -896,10 +904,13 @@ static inline void forward_timer_base(struct timer_base *base)* If the next expiry value is > jiffies, then we fast forward to * jiffies otherwise we forward to the next expiry value. */ -if (time_after (base->next_expiry, jnow)) +if (time_after (base->next_expiry, jnow)) { base->clk = jnow; -else + }else { +if (WARN_ON_ONCE (time_before (base->next_expiry, base->clk))) +return ; base->clk = base->next_expiry; + }#endif }
Sasha Levin
maintainerHi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag fixing commit: a683f390b93f ("timers: Forward the wheel clock whenever possible").
The bot has tested the following trees: v5.7.8, v5.4.51, v4.19.132, v4.14.188, v4.9.230.
v5.7.8: Build OK!
v5.4.51: Build OK!
v4.19.132: Build OK!
v4.14.188: Failed to apply! Possible dependencies:
30587589251a0 ("timer: Fix coding style")v4.9.230: Failed to apply! Possible dependencies:
30587589251a0 ("timer: Fix coding style")NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
Signature · 2 lines
Thanks Sasha