In case when timers are migrated to a CPU, after it exits
idle, but before timer base is forwarded, either from
run_timer_softirq()/mod_timer()/add_timer_on(), it's
possible that migrated timers are queued, based on older
clock value. This can cause delays in handling those timers.
For example, consider below sequence of events:
- CPU0 timer1 expires = 59969 and base->clk = 59131. So,
timer is queued at level 2, with next expiry for this timer
= 60032 (due to granularity addition).
- CPU1 enters idle @60007, with next timer expiry @60020.
- CPU1 exits idle.
- CPU0 is hotplugged at 60009, and timers are migrated to
CPU1, with new base->clk = 60007. timer1 is queued,
based on 60007 at level 0, for immediate handling (in
next timer softirq handling).
- CPU1's base->clk is forwarded to 60009, so, in next sched
timer interrupt, timer1 is not handled.
The issue happens as timer wheel collects expired timers
starting from the current clk's index onwards, but migrated
timers, if enqueued, based on older clk value can result
in their index being less than clk's current index.
This can only happen if new base->clk is ahead of
timer->expires, resulting in timer being queued at
new base->clk's current index.
Co-developed-by: Neeraj Upadhyay <[email protected]>
Signed-off-by: Neeraj Upadhyay <[email protected]>
Signed-off-by: Lingutla Chandrasekhar <[email protected]>
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 89a9e1b4264a..f66c7ad55d7a 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1886,6 +1886,12 @@ int timers_dead_cpu(unsigned int cpu)
raw_spin_lock_irq(&new_base->lock);
raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);
+ /*
+ * Before migrating timers, update new base clk to avoid
+ * queueing timers based on older clock value.
+ */
+ forward_timer_base(new_base);
+
BUG_ON(old_base->running_timer);
for (i = 0; i < WHEEL_SIZE; i++)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
Commit-ID: c52232a49e203a65a6e1a670cd5262f59e9364a0
Gitweb: https://git.kernel.org/tip/c52232a49e203a65a6e1a670cd5262f59e9364a0
Author: Lingutla Chandrasekhar <[email protected]>
AuthorDate: Thu, 18 Jan 2018 17:20:22 +0530
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 28 Feb 2018 23:34:33 +0100
timers: Forward timer base before migrating timers
On CPU hotunplug the enqueued timers of the unplugged CPU are migrated to a
live CPU. This happens from the control thread which initiated the unplug.
If the CPU on which the control thread runs came out from a longer idle
period then the base clock of that CPU might be stale because the control
thread runs prior to any event which forwards the clock.
In such a case the timers from the unplugged CPU are queued on the live CPU
based on the stale clock which can cause large delays due to increased
granularity of the outer timer wheels which are far away from base:;clock.
But there is a worse problem than that. The following sequence of events
illustrates it:
- CPU0 timer1 is queued expires = 59969 and base->clk = 59131.
The timer is queued at wheel level 2, with resulting expiry time = 60032
(due to level granularity).
- CPU1 enters idle @60007, with next timer expiry @60020.
- CPU0 is hotplugged at @60009
- CPU1 exits idle and runs the control thread which migrates the
timers from CPU0
timer1 is now queued in level 0 for immediate handling in the next
softirq because the requested expiry time 59969 is before CPU1 base->clk
60007
- CPU1 runs code which forwards the base clock which succeeds because the
next expiring timer. which was collected at idle entry time is still set
to 60020.
So it forwards beyond 60007 and therefore misses to expire the migrated
timer1. That timer gets expired when the wheel wraps around again, which
takes between 63 and 630ms depending on the HZ setting.
Address both problems by invoking forward_timer_base() for the control CPUs
timer base. All other places, which might run into a similar problem
(mod_timer()/add_timer_on()) already invoke forward_timer_base() to avoid
that.
[ tglx: Massaged comment and changelog ]
Fixes: a683f390b93f ("timers: Forward the wheel clock whenever possible")
Co-developed-by: Neeraj Upadhyay <[email protected]>
Signed-off-by: Neeraj Upadhyay <[email protected]>
Signed-off-by: Lingutla Chandrasekhar <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Anna-Maria Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/time/timer.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 48150ab42de9..4a4fd567fb26 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1894,6 +1894,12 @@ int timers_dead_cpu(unsigned int cpu)
raw_spin_lock_irq(&new_base->lock);
raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);
+ /*
+ * The current CPUs base clock might be stale. Update it
+ * before moving the timers over.
+ */
+ forward_timer_base(new_base);
+
BUG_ON(old_base->running_timer);
for (i = 0; i < WHEEL_SIZE; i++)