Hi,
In lowres mode, hrtimers are serviced by the tick. But when a hrtimer is
enqueued on a idle or full dynticks target, we need to kick it in order to make
it reconsider the next tick to schedule, to correctly handle the hrtimer's
expiring time.
Now while this kick is correctly performed for periodic timers, hrtimers were a
bit neglected.
get_nohz_timer_target() doesn't return a remote idle target but there is a small
race window here. The elected target has all the time to turn dynticks idle
between the call to get_nohz_timer_target() and the locking of its base. Hence a
risk that we enqueue a hrtimer on a dynticks idle destination without kicking it.
As a result, the hrtimer might be serviced too late in the future.
Also a target elected by get_nohz_timer_target() can be in full dynticks mode
and thus require to be kicked as well. And unlike idle dynticks, this concern
both local and remote targets.
This patchset should fix all these issues.
git://git.linaro.org/people/viresh.kumar/linux.git missing-kick
Similar problems with timers were fixed and sent to LKML earlier:
https://lkml.org/lkml/2014/6/12/588
(Yeah, I stole logs from there :) )
Viresh Kumar (3):
hrtimer: Store cpu-number in 'struct hrtimer_cpu_base'
hrtimer: Kick lowres dynticks targets on hrtimer_start*() calls
hrtimers: don't check hres_active before calling hrtimer_reprogram()
include/linux/hrtimer.h | 2 ++
kernel/hrtimer.c | 49 +++++++++++++++++++++++++++----------------------
2 files changed, 29 insertions(+), 22 deletions(-)
--
2.0.0.rc2
3.13.11.4 -stable review patch. If anyone has any objections, please let me know.
------------------
From: Viresh Kumar <viresh.kumar(a)linaro.org>
commit 84ea7fe37908254c3bd90910921f6e1045c1747a upstream.
switch_hrtimer_base() calls hrtimer_check_target() which ensures that
we do not migrate a timer to a remote cpu if the timer expires before
the current programmed expiry time on that remote cpu.
But __hrtimer_start_range_ns() calls switch_hrtimer_base() before the
new expiry time is set. So the sanity check in hrtimer_check_target()
is operating on stale or even uninitialized data.
Update expiry time before calling switch_hrtimer_base().
[ tglx: Rewrote changelog once again ]
Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
Cc: linaro-kernel(a)lists.linaro.org
Cc: linaro-networking(a)linaro.org
Cc: fweisbec(a)gmail.com
Cc: arvind.chauhan(a)arm.com
Link: http://lkml.kernel.org/r/81999e148745fc51bbcd0615823fbab9b2e87e23.139988225…
Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de>
Signed-off-by: Kamal Mostafa <kamal(a)canonical.com>
---
kernel/hrtimer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 6de65d8..aa14922 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1002,11 +1002,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
/* Remove an active timer from the queue: */
ret = remove_hrtimer(timer, base);
- /* Switch the timer base, if necessary: */
- new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
-
if (mode & HRTIMER_MODE_REL) {
- tim = ktime_add_safe(tim, new_base->get_time());
+ tim = ktime_add_safe(tim, base->get_time());
/*
* CONFIG_TIME_LOW_RES is a temporary way for architectures
* to signal that they simply return xtime in
@@ -1021,6 +1018,9 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
hrtimer_set_expires_range_ns(timer, tim, delta_ns);
+ /* Switch the timer base, if necessary: */
+ new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
+
timer_stats_hrtimer_set_start_info(timer);
leftmost = enqueue_hrtimer(timer, new_base);
--
1.9.1
This is a note to let you know that I have just added a patch titled
hrtimer: Set expiry time before switch_hrtimer_base()
to the linux-3.13.y-queue branch of the 3.13.y.z extended stable tree
which can be found at:
http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/lin…
This patch is scheduled to be released in version 3.13.11.4.
If you, or anyone else, feels it should not be added to this tree, please
reply to this email.
For more information about the 3.13.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
Thanks.
-Kamal
------
>From 314b53cb66cf64991bc729b453bf5521baa952df Mon Sep 17 00:00:00 2001
From: Viresh Kumar <viresh.kumar(a)linaro.org>
Date: Mon, 12 May 2014 13:42:29 +0530
Subject: hrtimer: Set expiry time before switch_hrtimer_base()
commit 84ea7fe37908254c3bd90910921f6e1045c1747a upstream.
switch_hrtimer_base() calls hrtimer_check_target() which ensures that
we do not migrate a timer to a remote cpu if the timer expires before
the current programmed expiry time on that remote cpu.
But __hrtimer_start_range_ns() calls switch_hrtimer_base() before the
new expiry time is set. So the sanity check in hrtimer_check_target()
is operating on stale or even uninitialized data.
Update expiry time before calling switch_hrtimer_base().
[ tglx: Rewrote changelog once again ]
Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
Cc: linaro-kernel(a)lists.linaro.org
Cc: linaro-networking(a)linaro.org
Cc: fweisbec(a)gmail.com
Cc: arvind.chauhan(a)arm.com
Link: http://lkml.kernel.org/r/81999e148745fc51bbcd0615823fbab9b2e87e23.139988225…
Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de>
Signed-off-by: Kamal Mostafa <kamal(a)canonical.com>
---
kernel/hrtimer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 6de65d8..aa14922 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1002,11 +1002,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
/* Remove an active timer from the queue: */
ret = remove_hrtimer(timer, base);
- /* Switch the timer base, if necessary: */
- new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
-
if (mode & HRTIMER_MODE_REL) {
- tim = ktime_add_safe(tim, new_base->get_time());
+ tim = ktime_add_safe(tim, base->get_time());
/*
* CONFIG_TIME_LOW_RES is a temporary way for architectures
* to signal that they simply return xtime in
@@ -1021,6 +1018,9 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
hrtimer_set_expires_range_ns(timer, tim, delta_ns);
+ /* Switch the timer base, if necessary: */
+ new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
+
timer_stats_hrtimer_set_start_info(timer);
leftmost = enqueue_hrtimer(timer, new_base);
--
1.9.1