Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 2/3] time: Fix leapsecond triggered hrtimer/futex load spike issue

52 views
Skip to first unread message

John Stultz

unread,
Jul 5, 2012, 3:20:02 PM7/5/12
to
As widely reported on the internet, some Linux systems after
the leapsecond was inserted are experiencing futex related load
spikes (usually connected to MySQL, Firefox, Thunderbird, Java, etc).

An apparent for this issue workaround is running:
$ date -s "`date`"

Credit: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix

I this issue is due to the leapsecond being added without
calling clock_was_set() to notify the hrtimer subsystem of the
change.

The workaround functions as it forces a clock_was_set()
call from settimeofday().

This fix adds the required clock_was_set() calls to where
we adjust for leapseconds.

NOTE: This fix *depends* on the previous fix, which allows
clock_was_set to be called from atomic context. Do not try
to apply just this patch.

CC: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
CC: Thomas Gleixner <tg...@linutronix.de>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Reported-by: Jan Engelhardt <jen...@inai.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
kernel/time/timekeeping.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6f46a00..cc2991d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -963,6 +963,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set();
}

/* Accumulate raw time */
@@ -1079,6 +1081,8 @@ static void update_wall_time(void)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set();
}

timekeeping_update(false);
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

John Stultz

unread,
Jul 5, 2012, 3:20:02 PM7/5/12
to
Thomas:
So Prarit and my testing over the last few days have gone fine,
and its been quiet otherwise, so I wanted to go ahead and submit this
for inclusion.

As widely reported on the internet, many Linux systems after
the leapsecond was inserted experienced futex related load
spikes (usually connected to MySQL, Firefox, Thunderbird, Java, etc).

An apparent workaround for this issue is running:
This issue stemmed from the timekeeping subsystem not notifying
the hrtimer subsystem that the leapsecond occurred, causing
CLOCK_REALTIME hritmers to be fired one second early, and
sub-second CLOCK_REALTIME hrtimer timeouts to fire immediately
(causing the load spikes).


To address this issue I'm proposing we do three things:
1) Fix the clock_was_set() call to remove the limitation that kept
us from calling it from update_wall_time().

2) Call clock_was_set() when we add/remove a leapsecond.

3) Change hrtimer_interrupt to update the hrtimer base offset values.
This third item provides additional robustness should the
clock_was_set() notification (done via a timer if we're in_atomic)
be delayed significantly.


NOTE: Some reports have been of a hard hang right at or before
the leapsecond. I've not been able to reproduce or diagnose
this, so this fix does not likely address the reported hard
hangs (unless they end up being connected to the futex/hrtimer
issue). Please email lkml and me if you experienced this.

Big thanks to Prarit for shaking out a few issues in the earlier
version of this patch set, as well as the extra effort testing over
the Holiday!

Also, I've already got backports generated for -stable, that I'm
testing and I'll submitting them once I have upstream commit ids for
these patches.

thanks
-john

CC: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
CC: Thomas Gleixner <tg...@linutronix.de>


John Stultz (3):
hrtimer: Fix clock_was_set so it is safe to call from irq context
time: Fix leapsecond triggered hrtimer/futex load spike issue
hrtimer: Update hrtimer base offsets each hrtimer_interrupt

include/linux/hrtimer.h | 3 +++
kernel/hrtimer.c | 31 +++++++++++++++++++++++++++----
kernel/time/timekeeping.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 4 deletions(-)

John Stultz

unread,
Jul 5, 2012, 3:20:02 PM7/5/12
to
NOTE:This is a prerequisite patch that's required to
address the widely observed leap-second related futex/hrtimer
issues.

Currently clock_was_set() is unsafe to be called from irq
context, as it calls on_each_cpu(). This causes problems when
we need to adjust the time from update_wall_time().

To fix this, if clock_was_set is called when irqs are
disabled, we schedule a timer to fire for immedately after
we're out of interrupt context to then notify the hrtimer
subsystem.

CC: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
CC: Thomas Gleixner <tg...@linutronix.de>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Reported-by: Jan Engelhardt <jen...@inai.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
kernel/hrtimer.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ae34bf5..d730678 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -746,7 +746,7 @@ static inline void retrigger_next_event(void *arg) { }
* resolution timer interrupts. On UP we just disable interrupts and
* call the high resolution interrupt code.
*/
-void clock_was_set(void)
+static void do_clock_was_set(unsigned long data)
{
#ifdef CONFIG_HIGH_RES_TIMERS
/* Retrigger the CPU local events everywhere */
@@ -755,6 +755,21 @@ void clock_was_set(void)
timerfd_clock_was_set();
}

+static DEFINE_TIMER(clock_was_set_timer, do_clock_was_set , 0, 0);
+
+void clock_was_set(void)
+{
+ /*
+ * We can't call on_each_cpu() from irq context,
+ * so if irqs are disabled , schedule the clock_was_set
+ * via a timer_list timer for right after.
+ */
+ if (irqs_disabled())
+ mod_timer(&clock_was_set_timer, jiffies);
+ else
+ do_clock_was_set(0);
+}
+
/*
* During resume we might have to reprogram the high resolution timer
* interrupt (on the local CPU):

John Stultz

unread,
Jul 5, 2012, 3:20:02 PM7/5/12
to
This patch introduces a new funciton which captures the
CLOCK_MONOTONIC time, along with the CLOCK_REALTIME and
CLOCK_BOOTTIME offsets at the same moment. This new function
is then used in place of ktime_get() when hrtimer_interrupt()
is expiring timers.

This ensures that any changes to realtime or boottime offsets
are noticed and stored into the per-cpu hrtimer base structures,
prior to doing any hrtimer expiration. This should ensure that
timers are not expired early if the offsets changes under us.

This is useful in the case where clock_was_set() is called from
atomic context and have to schedule the hrtimer base offset update
via a timer, as it provides extra robustness in the face of any
possible timer delay.

CC: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
CC: Thomas Gleixner <tg...@linutronix.de>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
include/linux/hrtimer.h | 3 +++
kernel/hrtimer.c | 14 +++++++++++---
kernel/time/timekeeping.c | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index fd0dc30..f6b2a74 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -320,6 +320,9 @@ extern ktime_t ktime_get(void);
extern ktime_t ktime_get_real(void);
extern ktime_t ktime_get_boottime(void);
extern ktime_t ktime_get_monotonic_offset(void);
+extern void ktime_get_and_real_and_sleep_offset(ktime_t *monotonic,
+ ktime_t *real_offset,
+ ktime_t *sleep_offset);

DECLARE_PER_CPU(struct tick_device, tick_cpu_device);

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index d730678..56600c4 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1258,18 +1258,26 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
void hrtimer_interrupt(struct clock_event_device *dev)
{
struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
- ktime_t expires_next, now, entry_time, delta;
+ ktime_t expires_next, now, entry_time, delta, real_offset, sleep_offset;
int i, retries = 0;

BUG_ON(!cpu_base->hres_active);
cpu_base->nr_events++;
dev->next_event.tv64 = KTIME_MAX;

- entry_time = now = ktime_get();
+
+ ktime_get_and_real_and_sleep_offset(&now, &real_offset, &sleep_offset);
+
+ entry_time = now;
retry:
expires_next.tv64 = KTIME_MAX;

raw_spin_lock(&cpu_base->lock);
+
+ /* Update base offsets, to avoid early wakeups */
+ cpu_base->clock_base[HRTIMER_BASE_REALTIME].offset = real_offset;
+ cpu_base->clock_base[HRTIMER_BASE_BOOTTIME].offset = sleep_offset;
+
/*
* We set expires_next to KTIME_MAX here with cpu_base->lock
* held to prevent that a timer is enqueued in our queue via
@@ -1346,7 +1354,7 @@ retry:
* interrupt routine. We give it 3 attempts to avoid
* overreacting on some spurious event.
*/
- now = ktime_get();
+ ktime_get_and_real_and_sleep_offset(&now, &real_offset, &sleep_offset);
cpu_base->nr_retries++;
if (++retries < 3)
goto retry;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index cc2991d..b3404cf 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1251,6 +1251,40 @@ void get_xtime_and_monotonic_and_sleep_offset(struct timespec *xtim,
}

/**
+ * ktime_get_and_real_and_sleep_offset() - hrtimer helper, gets monotonic ktime,
+ * realtime offset, and sleep offsets.
+ */
+void ktime_get_and_real_and_sleep_offset(ktime_t *monotonic,
+ ktime_t *real_offset,
+ ktime_t *sleep_offset)
+{
+ unsigned long seq;
+ struct timespec wtom, sleep;
+ u64 secs, nsecs;
+
+ do {
+ seq = read_seqbegin(&timekeeper.lock);
+
+ secs = timekeeper.xtime.tv_sec +
+ timekeeper.wall_to_monotonic.tv_sec;
+ nsecs = timekeeper.xtime.tv_nsec +
+ timekeeper.wall_to_monotonic.tv_nsec;
+ nsecs += timekeeping_get_ns();
+ /* If arch requires, add in gettimeoffset() */
+ nsecs += arch_gettimeoffset();
+
+ wtom = timekeeper.wall_to_monotonic;
+ sleep = timekeeper.total_sleep_time;
+ } while (read_seqretry(&timekeeper.lock, seq));
+
+ *monotonic = ktime_add_ns(ktime_set(secs, 0), nsecs);
+ set_normalized_timespec(&wtom, -wtom.tv_sec, -wtom.tv_nsec);
+ *real_offset = timespec_to_ktime(wtom);
+ *sleep_offset = timespec_to_ktime(sleep);
+}
+
+
+/**
* ktime_get_monotonic_offset() - get wall_to_monotonic in ktime_t format
*/
ktime_t ktime_get_monotonic_offset(void)

tip-bot for John Stultz

unread,
Jul 9, 2012, 5:50:03 AM7/9/12
to
Commit-ID: d7e2e7fef1f0f4e1e614353a6c5eef4e18d98d2d
Gitweb: http://git.kernel.org/tip/d7e2e7fef1f0f4e1e614353a6c5eef4e18d98d2d
Author: John Stultz <john...@us.ibm.com>
AuthorDate: Thu, 5 Jul 2012 15:12:18 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Mon, 9 Jul 2012 11:35:38 +0200

hrtimer: Update hrtimer base offsets each hrtimer_interrupt

This patch introduces a new funciton which captures the
CLOCK_MONOTONIC time, along with the CLOCK_REALTIME and
CLOCK_BOOTTIME offsets at the same moment. This new function
is then used in place of ktime_get() when hrtimer_interrupt()
is expiring timers.

This ensures that any changes to realtime or boottime offsets
are noticed and stored into the per-cpu hrtimer base structures,
prior to doing any hrtimer expiration. This should ensure that
timers are not expired early if the offsets changes under us.

This is useful in the case where clock_was_set() is called from
atomic context and have to schedule the hrtimer base offset update
via a timer, as it provides extra robustness in the face of any
possible timer delay.

Signed-off-by: John Stultz <john...@us.ibm.com>
Acked-by: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341515538-5100-4-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>

tip-bot for John Stultz

unread,
Jul 9, 2012, 5:50:03 AM7/9/12
to
Commit-ID: adf374cf61394dd41c027c74a81f9bef90f7a640
Gitweb: http://git.kernel.org/tip/adf374cf61394dd41c027c74a81f9bef90f7a640
Author: John Stultz <john...@us.ibm.com>
AuthorDate: Thu, 5 Jul 2012 15:12:16 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Mon, 9 Jul 2012 11:35:38 +0200

hrtimer: Fix clock_was_set so it is safe to call from irq context

NOTE: This is a prerequisite patch that's required to
address the widely observed leap-second related futex/hrtimer
issues.

Currently clock_was_set() is unsafe to be called from irq
context, as it calls on_each_cpu(). This causes problems when
we need to adjust the time from update_wall_time().

To fix this, if clock_was_set is called when irqs are
disabled, we schedule a timer to fire for immedately after
we're out of interrupt context to then notify the hrtimer
subsystem.

Reported-by: Jan Engelhardt <jen...@inai.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
Acked-by: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341515538-5100-2-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
kernel/hrtimer.c | 17 ++++++++++++++++-
1 files changed, 16 insertions(+), 1 deletions(-)

tip-bot for John Stultz

unread,
Jul 9, 2012, 5:50:04 AM7/9/12
to
Commit-ID: bb88e92477def647976cd3d6964af98beceba900
Gitweb: http://git.kernel.org/tip/bb88e92477def647976cd3d6964af98beceba900
Author: John Stultz <john...@us.ibm.com>
AuthorDate: Thu, 5 Jul 2012 15:12:17 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Mon, 9 Jul 2012 11:35:38 +0200

time: Fix leapsecond triggered hrtimer/futex load spike issue

As widely reported on the internet, some Linux systems after
the leapsecond was inserted are experiencing futex related load
spikes (usually connected to MySQL, Firefox, Thunderbird, Java, etc).

An apparent for this issue workaround is running:
$ date -s "`date`"

Credit: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix

I this issue is due to the leapsecond being added without
calling clock_was_set() to notify the hrtimer subsystem of the
change.

The workaround functions as it forces a clock_was_set()
call from settimeofday().

This fix adds the required clock_was_set() calls to where
we adjust for leapseconds.

NOTE: This fix *depends* on the previous fix, which allows
clock_was_set to be called from atomic context. Do not try
to apply just this patch.

Reported-by: Jan Engelhardt <jen...@inai.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
Acked-by: Prarit Bhargava <pra...@redhat.com>
CC: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341515538-5100-3-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
kernel/time/timekeeping.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6f46a00..cc2991d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -963,6 +963,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set();
}

/* Accumulate raw time */
@@ -1079,6 +1081,8 @@ static void update_wall_time(void)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set();
}

timekeeping_update(false);
--

John Stultz

unread,
Jul 10, 2012, 6:50:01 PM7/10/12
to
The update of the hrtimer base offsets on all cpus cannot be made
atomically from the timekeeper.lock held and interrupt disabled region
as smp function calls are not allowed there.

clock_was_set(), which enforces the update on all cpus, is called
either from preemptible process context in case of do_settimeofday()
or from the softirq context when the offset modification happened in
the timer interrupt itself due to a leap second.

In both cases there is a race window for an hrtimer interrupt between
dropping timekeeper lock, enabling interrupts and clock_was_set()
issuing the updates. Any interrupt which arrives in that window will
see the new time but operate on stale offsets.

So we need to make sure that an hrtimer interrupt always sees a
consistent state of time and offsets.

ktime_get_update_offsets() allows us to get the current monotonic time
and update the per cpu hrtimer base offsets from hrtimer_interrupt()
to capture a consistent state of monotonic time and the offsets. The
function replaces the existing ktime_get() calls in hrtimer_interrupt().

The overhead of the new function vs. ktime_get() is minimal as it just
adds two store operations.

This ensures that any changes to realtime or boottime offsets are
noticed and stored into the per-cpu hrtimer base structures, prior to
any hrtimer expiration and guarantees that timers are not expired early.

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Signed-off-by: John Stultz <john...@us.ibm.com>
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
kernel/hrtimer.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 14a260a..669522b 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -657,6 +657,14 @@ static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer,
return 0;
}

+static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base)
+{
+ ktime_t *offs_real = &base->clock_base[HRTIMER_BASE_REALTIME].offset;
+ ktime_t *offs_boot = &base->clock_base[HRTIMER_BASE_BOOTTIME].offset;
+
+ return ktime_get_update_offsets(offs_real, offs_boot);
+}
+
/*
* Retrigger next event is called after clock was set
*
@@ -665,22 +673,12 @@ static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer,
static void retrigger_next_event(void *arg)
{
struct hrtimer_cpu_base *base = &__get_cpu_var(hrtimer_bases);
- struct timespec realtime_offset, xtim, wtm, sleep;

if (!hrtimer_hres_active())
return;

- /* Optimized out for !HIGH_RES */
- get_xtime_and_monotonic_and_sleep_offset(&xtim, &wtm, &sleep);
- set_normalized_timespec(&realtime_offset, -wtm.tv_sec, -wtm.tv_nsec);
-
- /* Adjust CLOCK_REALTIME offset */
raw_spin_lock(&base->lock);
- base->clock_base[HRTIMER_BASE_REALTIME].offset =
- timespec_to_ktime(realtime_offset);
- base->clock_base[HRTIMER_BASE_BOOTTIME].offset =
- timespec_to_ktime(sleep);
-
+ hrtimer_update_base(base);
hrtimer_force_reprogram(base, 0);
raw_spin_unlock(&base->lock);
}
@@ -710,7 +708,6 @@ static int hrtimer_switch_to_hres(void)
base->clock_base[i].resolution = KTIME_HIGH_RES;

tick_setup_sched_timer();
-
/* "Retrigger" the interrupt to get things going */
retrigger_next_event(NULL);
local_irq_restore(flags);
@@ -1264,7 +1261,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
dev->next_event.tv64 = KTIME_MAX;

raw_spin_lock(&cpu_base->lock);
- entry_time = now = ktime_get();
+ entry_time = now = hrtimer_update_base(cpu_base);
retry:
expires_next.tv64 = KTIME_MAX;
/*
@@ -1342,9 +1339,12 @@ retry:
* We need to prevent that we loop forever in the hrtimer
* interrupt routine. We give it 3 attempts to avoid
* overreacting on some spurious event.
+ *
+ * Acquire base lock for updating the offsets and retrieving
+ * the current time.
*/
raw_spin_lock(&cpu_base->lock);
- now = ktime_get();
+ now = hrtimer_update_base(cpu_base);
cpu_base->nr_retries++;
if (++retries < 3)
goto retry;
--
1.7.9.5

John Stultz

unread,
Jul 10, 2012, 6:50:01 PM7/10/12
to
Over the weekend, Thomas got a chance to review the leap second fix
in more detail and had a few additional changes he wanted to make
to improve performance as well as style.

So this iteration includes his modifications.

Once merged, I'll be working to get the backports finished as quickly
as I can and sent to -stable.

thanks
-john

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org


John Stultz (3):
hrtimer: Provide clock_was_set_delayed()
timekeeping: Fix leapsecond triggered load spike issue
hrtimer: Update hrtimer base offsets each hrtimer_interrupt

Thomas Gleixner (3):
timekeeping: Maintain ktime_t based offsets for hrtimers
hrtimer: Move lock held region in hrtimer_interrupt()
timekeeping: Provide hrtimer update function

include/linux/hrtimer.h | 6 ++++-
kernel/hrtimer.c | 53 ++++++++++++++++++++++++++------------
kernel/time/timekeeping.c | 63 +++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 103 insertions(+), 19 deletions(-)

John Stultz

unread,
Jul 10, 2012, 6:50:01 PM7/10/12
to
From: Thomas Gleixner <tg...@linutronix.de>

We need to update the base offsets from this code and we need to do
that under base->lock. Move the lock held region around the
ktime_get() calls. The ktime_get() calls are going to be replaced with
a function which gets the time and the offsets atomically.

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
kernel/hrtimer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 7c20cb8..14a260a 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1263,11 +1263,10 @@ void hrtimer_interrupt(struct clock_event_device *dev)
cpu_base->nr_events++;
dev->next_event.tv64 = KTIME_MAX;

+ raw_spin_lock(&cpu_base->lock);
entry_time = now = ktime_get();
retry:
expires_next.tv64 = KTIME_MAX;
-
- raw_spin_lock(&cpu_base->lock);
/*
* We set expires_next to KTIME_MAX here with cpu_base->lock
* held to prevent that a timer is enqueued in our queue via
@@ -1344,6 +1343,7 @@ retry:
* interrupt routine. We give it 3 attempts to avoid
* overreacting on some spurious event.
*/
+ raw_spin_lock(&cpu_base->lock);
now = ktime_get();
cpu_base->nr_retries++;
if (++retries < 3)
@@ -1356,6 +1356,7 @@ retry:
*/
cpu_base->nr_hangs++;
cpu_base->hang_detected = 1;
+ raw_spin_unlock(&cpu_base->lock);
delta = ktime_sub(now, entry_time);
if (delta.tv64 > cpu_base->max_hang_time.tv64)
cpu_base->max_hang_time = delta;

John Stultz

unread,
Jul 10, 2012, 6:50:01 PM7/10/12
to
From: Thomas Gleixner <tg...@linutronix.de>

To finally fix the infamous leap second issue and other race windows
caused by functions which change the offsets between the various time
bases (CLOCK_MONOTONIC, CLOCK_REALTIME and CLOCK_BOOTTIME) we need a
function which atomically gets the current monotonic time and updates
the offsets of CLOCK_REALTIME and CLOCK_BOOTTIME with minimalistic
overhead. The previous patch which provides ktime_t offsets allows us
to make this function almost as cheap as ktime_get() which is going to
be replaced in hrtimer_interrupt().

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
include/linux/hrtimer.h | 1 +
kernel/time/timekeeping.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 4c3dac8..25b214e 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -323,6 +323,7 @@ extern ktime_t ktime_get(void);
extern ktime_t ktime_get_real(void);
extern ktime_t ktime_get_boottime(void);
extern ktime_t ktime_get_monotonic_offset(void);
+extern ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot);

DECLARE_PER_CPU(struct tick_device, tick_cpu_device);

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 1c038da..269b1fe 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1271,6 +1271,40 @@ void get_xtime_and_monotonic_and_sleep_offset(struct timespec *xtim,
} while (read_seqretry(&timekeeper.lock, seq));
}

+#ifdef CONFIG_HIGH_RES_TIMERS
+/**
+ * ktime_get_update_offsets - hrtimer helper
+ * @offs_real: pointer to storage for monotonic -> realtime offset
+ * @offs_boot: pointer to storage for monotonic -> boottime offset
+ *
+ * Returns current monotonic time and updates the offsets
+ * Called from hrtimer_interupt() or retrigger_next_event()
+ */
+ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
+{
+ ktime_t now;
+ unsigned int seq;
+ u64 secs, nsecs;
+
+ do {
+ seq = read_seqbegin(&timekeeper.lock);
+
+ secs = timekeeper.xtime.tv_sec;
+ nsecs = timekeeper.xtime.tv_nsec;
+ nsecs += timekeeping_get_ns();
+ /* If arch requires, add in gettimeoffset() */
+ nsecs += arch_gettimeoffset();
+
+ *offs_real = timekeeper.offs_real;
+ *offs_boot = timekeeper.offs_boot;
+ } while (read_seqretry(&timekeeper.lock, seq));
+
+ now = ktime_add_ns(ktime_set(secs, 0), nsecs);
+ now = ktime_sub(now, *offs_real);
+ return now;
+}
+#endif
+
/**
* ktime_get_monotonic_offset() - get wall_to_monotonic in ktime_t format
*/

John Stultz

unread,
Jul 10, 2012, 6:50:01 PM7/10/12
to
From: Thomas Gleixner <tg...@linutronix.de>

We need to update the base offsets from this code and we need to do
that under base->lock. Move the lock held region around the
ktime_get() calls. The ktime_get() calls are going to be replaced with
a function which gets the time and the offsets atomically.

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---

John Stultz

unread,
Jul 10, 2012, 6:50:02 PM7/10/12
to
From: Thomas Gleixner <tg...@linutronix.de>

We need to update the hrtimer clock offsets from the hrtimer interrupt
context. To avoid conversions from timespec to ktime_t maintain a
ktime_t based representation of those offsets in the timekeeper. This
puts the conversion overhead into the code which updates the
underlying offsets and provides fast accessible values in the hrtimer
interrupt.

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
kernel/time/timekeeping.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index a413e59..1c038da 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -70,6 +70,12 @@ struct timekeeper {
/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
struct timespec raw_time;

+ /* Offset clock monotonic -> clock realtime */
+ ktime_t offs_real;
+
+ /* Offset clock monotonic -> clock boottime */
+ ktime_t offs_boot;
+
/* Seqlock for all timekeeper values */
seqlock_t lock;
};
@@ -172,6 +178,14 @@ static inline s64 timekeeping_get_ns_raw(void)
return clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
}

+static void update_rt_offset(void)
+{
+ struct timespec tmp, *wtm = &timekeeper.wall_to_monotonic;
+
+ set_normalized_timespec(&tmp, -wtm->tv_sec, -wtm->tv_nsec);
+ timekeeper.offs_real = timespec_to_ktime(tmp);
+}
+
/* must hold write on timekeeper.lock */
static void timekeeping_update(bool clearntp)
{
@@ -179,6 +193,7 @@ static void timekeeping_update(bool clearntp)
timekeeper.ntp_error = 0;
ntp_clear();
}
+ update_rt_offset();
update_vsyscall(&timekeeper.xtime, &timekeeper.wall_to_monotonic,
timekeeper.clock, timekeeper.mult);
}
@@ -604,6 +619,7 @@ void __init timekeeping_init(void)
}
set_normalized_timespec(&timekeeper.wall_to_monotonic,
-boot.tv_sec, -boot.tv_nsec);
+ update_rt_offset();
timekeeper.total_sleep_time.tv_sec = 0;
timekeeper.total_sleep_time.tv_nsec = 0;
write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -612,6 +628,12 @@ void __init timekeeping_init(void)
/* time in seconds when suspend began */
static struct timespec timekeeping_suspend_time;

+static void update_sleep_time(struct timespec t)
+{
+ timekeeper.total_sleep_time = t;
+ timekeeper.offs_boot = timespec_to_ktime(t);
+}
+
/**
* __timekeeping_inject_sleeptime - Internal function to add sleep interval
* @delta: pointer to a timespec delta value
@@ -630,8 +652,7 @@ static void __timekeeping_inject_sleeptime(struct timespec *delta)
timekeeper.xtime = timespec_add(timekeeper.xtime, *delta);
timekeeper.wall_to_monotonic =
timespec_sub(timekeeper.wall_to_monotonic, *delta);
- timekeeper.total_sleep_time = timespec_add(
- timekeeper.total_sleep_time, *delta);
+ update_sleep_time(timespec_add(timekeeper.total_sleep_time, *delta));

John Stultz

unread,
Jul 10, 2012, 6:50:02 PM7/10/12
to
The timekeeping code misses an update of the hrtimer subsystem after a
leap second happened. Due to that timers based on CLOCK_REALTIME are
either expiring a second early or late depending on whether a leap
second has been inserted or deleted until an operation is initiated
which causes that update. Unless the update happens by some other
means this discrepancy between the timekeeping and the hrtimer data
stays forever and timers are expired either early or late.

The reported immediate workaround - $ data -s "`date`" - is causing a
call to clock_was_set() which updates the hrtimer data structures.
See: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix

Add the missing clock_was_set() call to update_wall_time() in case of
a leap second event. The actual update is deferred to softirq context
as the necessary smp function call cannot be invoked from hard
interrupt context.

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Reported-by: Jan Engelhardt <jen...@inai.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
kernel/time/timekeeping.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6f46a00..a413e59 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -963,6 +963,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set_delayed();
}

/* Accumulate raw time */
@@ -1079,6 +1081,8 @@ static void update_wall_time(void)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set_delayed();
}

timekeeping_update(false);

John Stultz

unread,
Jul 10, 2012, 6:50:02 PM7/10/12
to
clock_was_set() cannot be called from hard interrupt context because
it calls on_each_cpu(). For fixing the widely reported leap seconds
issue it's necessary to call it from the timer interrupt context.

Provide a new function which denotes it in the hrtimer cpu base
structure of the cpu on which it is called and raising the timer
softirq.

We then execute the clock_was_set() notificiation in the timer softirq
context in hrtimer_run_pending().

CC: Ingo Molnar <mi...@kernel.org>
CC: Peter Zijlstra <a.p.zi...@chello.nl>
CC: Prarit Bhargava <pra...@redhat.com>
CC: Thomas Gleixner <tg...@linutronix.de>
CC: sta...@vger.kernel.org
Reported-by: Jan Engelhardt <jen...@inai.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
---
include/linux/hrtimer.h | 5 ++++-
kernel/hrtimer.c | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index fd0dc30..4c3dac8 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -165,6 +165,7 @@ enum hrtimer_base_type {
* @lock: lock protecting the base and associated clock bases
* and timers
* @active_bases: Bitfield to mark bases with active timers
+ * @clock_was_set: Indicates that clock was set from irq context.
* @expires_next: absolute time of the next event which was scheduled
* via clock_set_next_event()
* @hres_active: State of high resolution mode
@@ -177,7 +178,8 @@ enum hrtimer_base_type {
*/
struct hrtimer_cpu_base {
raw_spinlock_t lock;
- unsigned long active_bases;
+ unsigned int active_bases;
+ unsigned int clock_was_set;
#ifdef CONFIG_HIGH_RES_TIMERS
ktime_t expires_next;
int hres_active;
@@ -309,6 +311,7 @@ static inline int hrtimer_is_hres_active(struct hrtimer *timer)
#endif

extern void clock_was_set(void);
+extern void clock_was_set_delayed(void);
#ifdef CONFIG_TIMERFD
extern void timerfd_clock_was_set(void);
#else
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ae34bf5..7c20cb8 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -756,6 +756,19 @@ void clock_was_set(void)
}

/*
+ * Called from timekeeping code to reprogramm the hrtimer interrupt
+ * device. If called from the timer interrupt context we defer it to
+ * softirq context.
+ */
+void clock_was_set_delayed(void)
+{
+ struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
+
+ cpu_base->clock_was_set = 1;
+ __raise_softirq_irqoff(TIMER_SOFTIRQ);
+}
+
+/*
* During resume we might have to reprogram the high resolution timer
* interrupt (on the local CPU):
*/
@@ -1413,6 +1426,13 @@ static inline void __hrtimer_peek_ahead_timers(void) { }
*/
void hrtimer_run_pending(void)
{
+ struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
+
+ if (cpu_base->clock_was_set) {
+ cpu_base->clock_was_set = 0;
+ clock_was_set();
+ }
+
if (hrtimer_hres_active())
return;

John Stultz

unread,
Jul 10, 2012, 7:00:01 PM7/10/12
to
On 07/10/2012 03:43 PM, John Stultz wrote:
> Over the weekend, Thomas got a chance to review the leap second fix
> in more detail and had a few additional changes he wanted to make
> to improve performance as well as style.
>
> So this iteration includes his modifications.
>
> Once merged, I'll be working to get the backports finished as quickly
> as I can and sent to -stable.

Also, Thomas pointed out that the entire fix might be too large for this
late in the -rc cycle.

Thus we may want to merge only the first two patches as the "simple fix"
which will avoid the major problems seen for 3.5, and then leave the
last four patches, which really enforce correctness, for 3.6 merge
window (backporting variants to -stable).

But I'll leave that to Thomas/Ingo/Linus to decide.

thanks
-john

John Stultz

unread,
Jul 10, 2012, 7:10:01 PM7/10/12
to
On 07/10/2012 03:43 PM, John Stultz wrote:
> Over the weekend, Thomas got a chance to review the leap second fix
> in more detail and had a few additional changes he wanted to make
> to improve performance as well as style.
>
> So this iteration includes his modifications.
>
> Once merged, I'll be working to get the backports finished as quickly
> as I can and sent to -stable.
>

Once again, here's the test case I've been using to stress test
leapsecond insertion/deletion (the deletion testing is new since last
round).

thanks
-john

leap-a-day.c

Peter Zijlstra

unread,
Jul 11, 2012, 7:10:02 AM7/11/12
to
On Tue, 2012-07-10 at 18:43 -0400, John Stultz wrote:
> Over the weekend, Thomas got a chance to review the leap second fix
> in more detail and had a few additional changes he wanted to make
> to improve performance as well as style.
>
> So this iteration includes his modifications.
>
> Once merged, I'll be working to get the backports finished as quickly
> as I can and sent to -stable.

I think we all dislike 1/6, but I can't come up with anything saner
either. The rest looks good.

The only thing I found (and already mentioned to Thomas) is that the
Changelog of 1/6 could be improved by mentioning why we're in hardirq
context to begin with.

Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>

Ingo Molnar

unread,
Jul 11, 2012, 7:20:02 AM7/11/12
to

* John Stultz <john...@us.ibm.com> wrote:

> Over the weekend, Thomas got a chance to review the leap
> second fix in more detail and had a few additional changes he
> wanted to make to improve performance as well as style.
>
> So this iteration includes his modifications.

Yep, looks much saner now that the separate 'delayed' parameter
is gone. For all patches:

Reviewed-by: Ingo Molnar <mi...@kernel.org>

Thanks,

Ingo

Prarit Bhargava

unread,
Jul 11, 2012, 8:20:01 AM7/11/12
to


On 07/10/2012 06:43 PM, John Stultz wrote:
> clock_was_set() cannot be called from hard interrupt context because
> it calls on_each_cpu(). For fixing the widely reported leap seconds
> issue it's necessary to call it from the timer interrupt context.
>
> Provide a new function which denotes it in the hrtimer cpu base
> structure of the cpu on which it is called and raising the timer
> softirq.
>
> We then execute the clock_was_set() notificiation in the timer softirq
> context in hrtimer_run_pending().

I wish there was a nicer way to do this ... but looking at the code I can't
figure out a better way. (no offense John, it's just the way the code is ;) )

P.

Prarit Bhargava

unread,
Jul 11, 2012, 8:20:02 AM7/11/12
to


On 07/10/2012 06:43 PM, John Stultz wrote:
> Over the weekend, Thomas got a chance to review the leap second fix
> in more detail and had a few additional changes he wanted to make
> to improve performance as well as style.
>
> So this iteration includes his modifications.
>
> Once merged, I'll be working to get the backports finished as quickly
> as I can and sent to -stable.
>
> thanks
> -john
>

Add an Acked-by: Prarit Bhargava <pra...@redhat.com>

John -- I'll do some testing of this patchset today and tomorrow and let you
know the results.

P.

Thomas Gleixner

unread,
Jul 11, 2012, 8:50:01 AM7/11/12
to
On Wed, 11 Jul 2012, Prarit Bhargava wrote:
> On 07/10/2012 06:43 PM, John Stultz wrote:
> > clock_was_set() cannot be called from hard interrupt context because
> > it calls on_each_cpu(). For fixing the widely reported leap seconds
> > issue it's necessary to call it from the timer interrupt context.
> >
> > Provide a new function which denotes it in the hrtimer cpu base
> > structure of the cpu on which it is called and raising the timer
> > softirq.
> >
> > We then execute the clock_was_set() notificiation in the timer softirq
> > context in hrtimer_run_pending().
>
> I wish there was a nicer way to do this ... but looking at the code I can't
> figure out a better way. (no offense John, it's just the way the code is ;) )

Yeah, I had the same discussion with Peter earlier today. There is
only a rather limited set of options.

1) Retrigger the timer interrupt vectors on all CPUs - except the one
we are running on, but we have no interface for that at the moment

2) Do the nasty __smp_call_function_single() hack

Preallocate call_single_data for all cpus and do a
__smp_call_function_single() on all online cpus.

This can be called from hard interrupt context or irq disabled
regions.

That would allow to get rid of the whole delay magic all
together.

Thoughts?

tglx

Peter Zijlstra

unread,
Jul 11, 2012, 9:10:01 AM7/11/12
to
The __smp_call_function_single() thing isn't particularly pretty either
and a lot more code to boot..

static DEFINE_PER_CPU(struct call_single_data, cws_csd);

void clock_was_set(void)
{
int cpu;

for_each_online_cpu(cpu) {
struct call_single_data *csd = &per_cpu(cws_csd, cpu);

if (csd->flags & CSD_FLAG_LOCK)
continue; /* a pending request is good enough */

csd->func = retrigger_next_event;

__smp_call_function_single(cpu, csd, 0);
}

timerfd_clock_was_set();
}

It also is a for_each_cpu loop with preemption disabled, not pretty :/

Prarit Bhargava

unread,
Jul 11, 2012, 9:10:02 AM7/11/12
to

>> I wish there was a nicer way to do this ... but looking at the code I can't
>> figure out a better way. (no offense John, it's just the way the code is ;) )
>
> Yeah, I had the same discussion with Peter earlier today. There is
> only a rather limited set of options.
>
> 1) Retrigger the timer interrupt vectors on all CPUs - except the one
> we are running on, but we have no interface for that at the moment
>
> 2) Do the nasty __smp_call_function_single() hack
>
> Preallocate call_single_data for all cpus and do a
> __smp_call_function_single() on all online cpus.
>
> This can be called from hard interrupt context or irq disabled
> regions.
>
> That would allow to get rid of the whole delay magic all
> together.
>
> Thoughts?
>

Both of those options seem like a lot of work for something that happens once
every 3-4 years, and may not happen ever again[1]. Based on that statement, if
we're going to modify code I would prefer that it be as lightweight as possible.
So, in terms of the kernel, option 2 is likely the best way to go rather than
introducing new code that will be used once every 3-4 years.

I keep asking the question of why the mechanism of inserting a leap second isn't
moved into userspace ntpd (or some other appropriate daemon). I suppose there
is a risk of ntpd being starved out on heavily loaded systems...

P.

[1] http://en.wikipedia.org/wiki/Leap_second#Proposal_to_abolish_leap_seconds

Peter Zijlstra

unread,
Jul 11, 2012, 9:40:03 AM7/11/12
to
On Wed, 2012-07-11 at 09:05 -0400, Prarit Bhargava wrote:
>
> Both of those options seem like a lot of work for something that happens once
> every 3-4 years, and may not happen ever again[1]. Based on that statement, if
> we're going to modify code I would prefer that it be as lightweight as possible.
> So, in terms of the kernel, option 2 is likely the best way to go rather than
> introducing new code that will be used once every 3-4 years.

Full agreed, however if we implement clock_was_set() like I just
proposed we'd use that code for every time the clock was modified, which
is a lot more often.

That said, I'm not a particular fan of it..

Thomas Gleixner

unread,
Jul 11, 2012, 11:20:02 AM7/11/12
to
Right. I think with the atomic update of the offset in the timer
interrupt we are on the safe side. The main problem of timers expiring
early forever is covered by this.

Thinking more about it.

If time goes backwards, then the IPI is pointless. The already armed
clockevent device will fire too early, hrtimer_interrupt will update
and just rearm it. That's one "spurious" event.

So we only need it in the case of time going forward.

Though with the leap second the maximum observable delay is 1 second
on a completely idle core. Surely nothing to worry about for an event
which happens rarely. So we could safely avoid the whole delayed
business and just do the timerfd notification, though I wonder if even
that is necessary in the leap second case.

On NOHZ=n systems the IPI is pointless as well. The maximum lateness
will be 10ms for HZ=100. Nothing we should worry about.

That leaves NOHZ enabled systems and there we might be clever and
avoid the IPIs to those cores which are not idle and let the tick
interrupt deal with it. And we can make the calls async and just let
them raise the hrtimer softirq on those cores, which will run the
hrtimer interrupt code and take care of everything.

Thoughts?

tglx

Peter Zijlstra

unread,
Jul 11, 2012, 12:00:02 PM7/11/12
to
On Wed, 2012-07-11 at 17:18 +0200, Thomas Gleixner wrote:

> Right. I think with the atomic update of the offset in the timer
> interrupt we are on the safe side. The main problem of timers expiring
> early forever is covered by this.
>
> Thinking more about it.
>
> If time goes backwards, then the IPI is pointless. The already armed
> clockevent device will fire too early, hrtimer_interrupt will update
> and just rearm it. That's one "spurious" event.
>
> So we only need it in the case of time going forward.
>
> Though with the leap second the maximum observable delay is 1 second
> on a completely idle core. Surely nothing to worry about for an event
> which happens rarely. So we could safely avoid the whole delayed
> business and just do the timerfd notification, though I wonder if even
> that is necessary in the leap second case.
>
> On NOHZ=n systems the IPI is pointless as well. The maximum lateness
> will be 10ms for HZ=100. Nothing we should worry about.
>
> That leaves NOHZ enabled systems and there we might be clever and
> avoid the IPIs to those cores which are not idle and let the tick
> interrupt deal with it. And we can make the calls async and just let
> them raise the hrtimer softirq on those cores, which will run the
> hrtimer interrupt code and take care of everything.
>
> Thoughts?


static void nohz_hrtimer_softirq(void *unused)
{
raise_softirq(HRTIMER_SOFTIRQ);
}

static void kick_nohz_cpus(void)
{
smp_call_function_many(nohz.idle_cpus_mask, nohz_hrtimer_softirq, NULL, 0);
}

Same problem as before though, can't be sending IPIs while in hardirq
context..

And you cannot do the same trick with a CFD as with the CSD, some CPUs
might need it again while others are still pending.

John Stultz

unread,
Jul 11, 2012, 12:50:03 PM7/11/12
to
On 07/11/2012 08:18 AM, Thomas Gleixner wrote:
> That leaves NOHZ enabled systems and there we might be clever and
> avoid the IPIs to those cores which are not idle and let the tick
> interrupt deal with it. And we can make the calls async and just let
> them raise the hrtimer softirq on those cores, which will run the
> hrtimer interrupt code and take care of everything.

I'm not familiar with the details of the code that tracks which cores
are idle or not, but I'd worry with this approach that there might be
further races in determining which cores are idle and which cores are
about to go idle with stale base offsets.

I'll see if my worry is unfounded, but it might be a bit too clever for
rare events.

thanks
-john

tip-bot for Thomas Gleixner

unread,
Jul 11, 2012, 5:50:01 PM7/11/12
to
Commit-ID: f6c06abfb3972ad4914cef57d8348fcb2932bc3b
Gitweb: http://git.kernel.org/tip/f6c06abfb3972ad4914cef57d8348fcb2932bc3b
Author: Thomas Gleixner <tg...@linutronix.de>
AuthorDate: Tue, 10 Jul 2012 18:43:24 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Wed, 11 Jul 2012 23:34:39 +0200

timekeeping: Provide hrtimer update function

To finally fix the infamous leap second issue and other race windows
caused by functions which change the offsets between the various time
bases (CLOCK_MONOTONIC, CLOCK_REALTIME and CLOCK_BOOTTIME) we need a
function which atomically gets the current monotonic time and updates
the offsets of CLOCK_REALTIME and CLOCK_BOOTTIME with minimalistic
overhead. The previous patch which provides ktime_t offsets allows us
to make this function almost as cheap as ktime_get() which is going to
be replaced in hrtimer_interrupt().

Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Reviewed-by: Ingo Molnar <mi...@kernel.org>
Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Cc: sta...@vger.kernel.org
Signed-off-by: John Stultz <john...@us.ibm.com>
Link: http://lkml.kernel.org/r/1341960205-56738-7-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
include/linux/hrtimer.h | 1 +
kernel/time/timekeeping.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index c9ec940..cc07d27 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -327,6 +327,7 @@ extern ktime_t ktime_get(void);

tip-bot for Thomas Gleixner

unread,
Jul 11, 2012, 5:50:02 PM7/11/12
to
Commit-ID: 196951e91262fccda81147d2bcf7fdab08668b40
Gitweb: http://git.kernel.org/tip/196951e91262fccda81147d2bcf7fdab08668b40
Author: Thomas Gleixner <tg...@linutronix.de>
AuthorDate: Tue, 10 Jul 2012 18:43:23 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Wed, 11 Jul 2012 23:34:38 +0200

hrtimers: Move lock held region in hrtimer_interrupt()

We need to update the base offsets from this code and we need to do
that under base->lock. Move the lock held region around the
ktime_get() calls. The ktime_get() calls are going to be replaced with
a function which gets the time and the offsets atomically.

Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Reviewed-by: Ingo Molnar <mi...@kernel.org>
Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Cc: sta...@vger.kernel.org
Signed-off-by: John Stultz <john...@us.ibm.com>
Link: http://lkml.kernel.org/r/1341960205-56738-6-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
kernel/hrtimer.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 3c24fb2..8f320af 100644

tip-bot for John Stultz

unread,
Jul 11, 2012, 5:50:02 PM7/11/12
to
Commit-ID: 4873fa070ae84a4115f0b3c9dfabc224f1bc7c51
Gitweb: http://git.kernel.org/tip/4873fa070ae84a4115f0b3c9dfabc224f1bc7c51
Author: John Stultz <john...@us.ibm.com>
AuthorDate: Tue, 10 Jul 2012 18:43:20 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Wed, 11 Jul 2012 23:34:37 +0200

timekeeping: Fix leapsecond triggered load spike issue

The timekeeping code misses an update of the hrtimer subsystem after a
leap second happened. Due to that timers based on CLOCK_REALTIME are
either expiring a second early or late depending on whether a leap
second has been inserted or deleted until an operation is initiated
which causes that update. Unless the update happens by some other
means this discrepancy between the timekeeping and the hrtimer data
stays forever and timers are expired either early or late.

The reported immediate workaround - $ data -s "`date`" - is causing a
call to clock_was_set() which updates the hrtimer data structures.
See: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix

Add the missing clock_was_set() call to update_wall_time() in case of
a leap second event. The actual update is deferred to softirq context
as the necessary smp function call cannot be invoked from hard
interrupt context.

Signed-off-by: John Stultz <john...@us.ibm.com>
Reported-by: Jan Engelhardt <jen...@inai.de>
Reviewed-by: Ingo Molnar <mi...@kernel.org>
Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Cc: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341960205-56738-3-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
kernel/time/timekeeping.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6f46a00..a413e59 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -963,6 +963,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set_delayed();
}

/* Accumulate raw time */
@@ -1079,6 +1081,8 @@ static void update_wall_time(void)
leap = second_overflow(timekeeper.xtime.tv_sec);
timekeeper.xtime.tv_sec += leap;
timekeeper.wall_to_monotonic.tv_sec -= leap;
+ if (leap)
+ clock_was_set_delayed();
}

timekeeping_update(false);
--

tip-bot for John Stultz

unread,
Jul 11, 2012, 5:50:01 PM7/11/12
to
Commit-ID: f55a6faa384304c89cfef162768e88374d3312cb
Gitweb: http://git.kernel.org/tip/f55a6faa384304c89cfef162768e88374d3312cb
Author: John Stultz <john...@us.ibm.com>
AuthorDate: Tue, 10 Jul 2012 18:43:19 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Wed, 11 Jul 2012 23:34:37 +0200

hrtimer: Provide clock_was_set_delayed()

clock_was_set() cannot be called from hard interrupt context because
it calls on_each_cpu().

For fixing the widely reported leap seconds issue it is necessary to
call it from hard interrupt context, i.e. the timer tick code, which
does the timekeeping updates.

Provide a new function which denotes it in the hrtimer cpu base
structure of the cpu on which it is called and raise the hrtimer
softirq. We then execute the clock_was_set() notificiation from
softirq context in run_hrtimer_softirq(). The hrtimer softirq is
rarely used, so polling the flag there is not a performance issue.

[ tglx: Made it depend on CONFIG_HIGH_RES_TIMERS. We really should get
rid of all this ifdeffery ASAP ]

Signed-off-by: John Stultz <john...@us.ibm.com>
Reported-by: Jan Engelhardt <jen...@inai.de>
Reviewed-by: Ingo Molnar <mi...@kernel.org>
Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Cc: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341960205-56738-2-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
include/linux/hrtimer.h | 9 ++++++++-
kernel/hrtimer.c | 20 ++++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index fd0dc30..c9ec940 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -165,6 +165,7 @@ enum hrtimer_base_type {
* @lock: lock protecting the base and associated clock bases
* and timers
* @active_bases: Bitfield to mark bases with active timers
+ * @clock_was_set: Indicates that clock was set from irq context.
* @expires_next: absolute time of the next event which was scheduled
* via clock_set_next_event()
* @hres_active: State of high resolution mode
@@ -177,7 +178,8 @@ enum hrtimer_base_type {
*/
struct hrtimer_cpu_base {
raw_spinlock_t lock;
- unsigned long active_bases;
+ unsigned int active_bases;
+ unsigned int clock_was_set;
#ifdef CONFIG_HIGH_RES_TIMERS
ktime_t expires_next;
int hres_active;
@@ -286,6 +288,8 @@ extern void hrtimer_peek_ahead_timers(void);
# define MONOTONIC_RES_NSEC HIGH_RES_NSEC
# define KTIME_MONOTONIC_RES KTIME_HIGH_RES

+extern void clock_was_set_delayed(void);
+
#else

# define MONOTONIC_RES_NSEC LOW_RES_NSEC
@@ -306,6 +310,9 @@ static inline int hrtimer_is_hres_active(struct hrtimer *timer)
{
return 0;
}
+
+static inline void clock_was_set_delayed(void) { }
+
#endif

extern void clock_was_set(void);
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ae34bf5..3c24fb2 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -717,6 +717,19 @@ static int hrtimer_switch_to_hres(void)
return 1;
}

+/*
+ * Called from timekeeping code to reprogramm the hrtimer interrupt
+ * device. If called from the timer interrupt context we defer it to
+ * softirq context.
+ */
+void clock_was_set_delayed(void)
+{
+ struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
+
+ cpu_base->clock_was_set = 1;
+ __raise_softirq_irqoff(HRTIMER_SOFTIRQ);
+}
+
#else

static inline int hrtimer_hres_active(void) { return 0; }
@@ -1395,6 +1408,13 @@ void hrtimer_peek_ahead_timers(void)

static void run_hrtimer_softirq(struct softirq_action *h)
{
+ struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
+
+ if (cpu_base->clock_was_set) {
+ cpu_base->clock_was_set = 0;
+ clock_was_set();
+ }
+
hrtimer_peek_ahead_timers();

tip-bot for John Stultz

unread,
Jul 11, 2012, 5:50:02 PM7/11/12
to
Commit-ID: 5baefd6d84163443215f4a99f6a20f054ef11236
Gitweb: http://git.kernel.org/tip/5baefd6d84163443215f4a99f6a20f054ef11236
Author: John Stultz <john...@us.ibm.com>
AuthorDate: Tue, 10 Jul 2012 18:43:25 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Wed, 11 Jul 2012 23:34:39 +0200

hrtimer: Update hrtimer base offsets each hrtimer_interrupt

The update of the hrtimer base offsets on all cpus cannot be made
atomically from the timekeeper.lock held and interrupt disabled region
as smp function calls are not allowed there.

clock_was_set(), which enforces the update on all cpus, is called
either from preemptible process context in case of do_settimeofday()
or from the softirq context when the offset modification happened in
the timer interrupt itself due to a leap second.

In both cases there is a race window for an hrtimer interrupt between
dropping timekeeper lock, enabling interrupts and clock_was_set()
issuing the updates. Any interrupt which arrives in that window will
see the new time but operate on stale offsets.

So we need to make sure that an hrtimer interrupt always sees a
consistent state of time and offsets.

ktime_get_update_offsets() allows us to get the current monotonic time
and update the per cpu hrtimer base offsets from hrtimer_interrupt()
to capture a consistent state of monotonic time and the offsets. The
function replaces the existing ktime_get() calls in hrtimer_interrupt().

The overhead of the new function vs. ktime_get() is minimal as it just
adds two store operations.

This ensures that any changes to realtime or boottime offsets are
noticed and stored into the per-cpu hrtimer base structures, prior to
any hrtimer expiration and guarantees that timers are not expired early.

Signed-off-by: John Stultz <john...@us.ibm.com>
Reviewed-by: Ingo Molnar <mi...@kernel.org>
Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Cc: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341960205-56738-8-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
kernel/hrtimer.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 8f320af..6db7a5e 100644

tip-bot for Thomas Gleixner

unread,
Jul 11, 2012, 5:50:02 PM7/11/12
to
Commit-ID: 5b9fe759a678e05be4937ddf03d50e950207c1c0
Gitweb: http://git.kernel.org/tip/5b9fe759a678e05be4937ddf03d50e950207c1c0
Author: Thomas Gleixner <tg...@linutronix.de>
AuthorDate: Tue, 10 Jul 2012 18:43:21 -0400
Committer: Thomas Gleixner <tg...@linutronix.de>
CommitDate: Wed, 11 Jul 2012 23:34:38 +0200

timekeeping: Maintain ktime_t based offsets for hrtimers

We need to update the hrtimer clock offsets from the hrtimer interrupt
context. To avoid conversions from timespec to ktime_t maintain a
ktime_t based representation of those offsets in the timekeeper. This
puts the conversion overhead into the code which updates the
underlying offsets and provides fast accessible values in the hrtimer
interrupt.

Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: John Stultz <john...@us.ibm.com>
Reviewed-by: Ingo Molnar <mi...@kernel.org>
Acked-by: Peter Zijlstra <a.p.zi...@chello.nl>
Acked-by: Prarit Bhargava <pra...@redhat.com>
Cc: sta...@vger.kernel.org
Link: http://lkml.kernel.org/r/1341960205-56738-4-gi...@us.ibm.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
---
kernel/time/timekeeping.c | 25 +++++++++++++++++++++++--
1 files changed, 23 insertions(+), 2 deletions(-)

Jan Ceuleers

unread,
Jul 12, 2012, 3:50:02 AM7/12/12
to
On 07/11/2012 06:47 PM, John Stultz wrote:
> I'll see if my worry is unfounded, but it might be a bit too clever for rare events.

Full ACK.

There is an unfortunate history of critical-to-moderately-serious bugs in
the leap second handling, so I submit that what is needed is a simple,
obviously-correct and robust mechanism. Robust statically, but also in the
face of code churn because these code paths are exercised so rarely out in
the wild.

Just my opinion, FWIW.

Jan

Prarit Bhargava

unread,
Jul 12, 2012, 8:40:02 AM7/12/12
to


On 07/12/2012 03:44 AM, Jan Ceuleers wrote:
> On 07/11/2012 06:47 PM, John Stultz wrote:
>> I'll see if my worry is unfounded, but it might be a bit too clever for rare events.
>
> Full ACK.
>
> There is an unfortunate history of critical-to-moderately-serious bugs in
> the leap second handling, so I submit that what is needed is a simple,
> obviously-correct and robust mechanism. Robust statically, but also in the
> face of code churn because these code paths are exercised so rarely out in
> the wild.
>
> Just my opinion, FWIW.
>

Ditto - and it's not just FWIW.

John (and everyone else), I think we're over-thinking this. Would it be nice to
get an extremely elegant solution to this? Yeah ... it would. But the reality
is that we're not going to get there and IMO we're making things too complex for
this little piece of code.

Acked-by: Prarit Bhargava <pra...@redhat.com>

IMO, this is the simplest way to move forward with this code.

P.

Prarit Bhargava

unread,
Jul 12, 2012, 8:40:03 AM7/12/12
to


On 07/11/2012 07:17 AM, Ingo Molnar wrote:
>
> * John Stultz <john...@us.ibm.com> wrote:
>
>> Over the weekend, Thomas got a chance to review the leap
>> second fix in more detail and had a few additional changes he
>> wanted to make to improve performance as well as style.


John,

FYI -- Using a mix of AMD and Intel systems (big and small, large memory and
small memory footprint), current test runs are ~18 hours at this point without
any issues seen, using a slightly modified version of your leap-a-day.c .

P.

Jiri Bohac

unread,
Jul 12, 2012, 7:00:03 PM7/12/12
to
On Tue, Jul 10, 2012 at 03:53:59PM -0700, John Stultz wrote:
> On 07/10/2012 03:43 PM, John Stultz wrote:
> >Over the weekend, Thomas got a chance to review the leap second fix
> >in more detail and had a few additional changes he wanted to make
> >to improve performance as well as style.
> >
> >So this iteration includes his modifications.
> >
> >Once merged, I'll be working to get the backports finished as quickly
> >as I can and sent to -stable.

looking at the proposed 2.6.32.y stable patch at:
http://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=18d208632bf17aed56c581b882868b2be44be71e;hp=6d224606bb8eec78027522d6dd5abfea8108c41a
Is this the final version you are about to send to -stable?

In 2.6.32 timekeeping_leap_insert() is not called from the timer
interrupt, but from the leap_timer hrtimer.

I think the new clock_was_set_timer will thus not be called by
irq_exit() because TIMER_SOFTIRQ has not been raised. Unless
TIMER_SOFTIRQ is raised, clock_was_set() will not be called until
the next periodic timer interrupt, correct?

Wouldn't the original schedule_work() approach work better for
2.6.32?

Or do you plan backporting the most recent version to 2.6.32?

Thanks,

--
Jiri Bohac <jbo...@suse.cz>
SUSE Labs, SUSE CZ

John Stultz

unread,
Jul 12, 2012, 8:00:02 PM7/12/12
to
On 07/12/2012 03:43 PM, Jiri Bohac wrote:
> On Tue, Jul 10, 2012 at 03:53:59PM -0700, John Stultz wrote:
>> On 07/10/2012 03:43 PM, John Stultz wrote:
>>> Over the weekend, Thomas got a chance to review the leap second fix
>>> in more detail and had a few additional changes he wanted to make
>>> to improve performance as well as style.
>>>
>>> So this iteration includes his modifications.
>>>
>>> Once merged, I'll be working to get the backports finished as quickly
>>> as I can and sent to -stable.
> looking at the proposed 2.6.32.y stable patch at:
> http://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=18d208632bf17aed56c581b882868b2be44be71e;hp=6d224606bb8eec78027522d6dd5abfea8108c41a
> Is this the final version you are about to send to -stable?
No, this isn't what I'm sending to -stable. That was my backport that
was done was prior to merging Thomas' modifications from over the
weekend. Having, so far, done this backporting 3 times or so, I figured
I'd just wait until something got committed upstream before trying to
backport it again. :)


> In 2.6.32 timekeeping_leap_insert() is not called from the timer
> interrupt, but from the leap_timer hrtimer.
>
> I think the new clock_was_set_timer will thus not be called by
> irq_exit() because TIMER_SOFTIRQ has not been raised. Unless
> TIMER_SOFTIRQ is raised, clock_was_set() will not be called until
> the next periodic timer interrupt, correct?
>
> Wouldn't the original schedule_work() approach work better for
> 2.6.32?
>
> Or do you plan backporting the most recent version to 2.6.32?

I'll be backporting & testing the most recent version once it is
committed upstream.

thanks
-john

John Stultz

unread,
Jul 12, 2012, 8:50:01 PM7/12/12
to
Prarit noticed a small bug with the test case. We're not initializing
the timespec tv_nsec value before sleeping, so in some cases
clock_nanosleep will return -EINVAL.

This version fixes this and cleans up some of the timespec/timeval usage.

thanks
-john

leap-a-day.c

Andreas Schwab

unread,
Jul 15, 2012, 11:30:02 AM7/15/12
to
This breaks resume on the iBook G4 (PowerBook6,7). Apparently during or
before noirq resume the system is hanging by the same amount of time as
the system was sleeping.

Andreas.

--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Andreas Schwab

unread,
Jul 15, 2012, 12:10:02 PM7/15/12
to
Andreas Schwab <sch...@linux-m68k.org> writes:

> This breaks resume on the iBook G4 (PowerBook6,7). Apparently during or
> before noirq resume the system is hanging by the same amount of time as
> the system was sleeping.

The point where the time is wasted actually appears to be _after_ resume
(the elapsed time for the resume itself isn't affected).

Rafael J. Wysocki

unread,
Jul 15, 2012, 4:30:01 PM7/15/12
to
On Sunday, July 15, 2012, Andreas Schwab wrote:
> This breaks resume on the iBook G4 (PowerBook6,7). Apparently during or
> before noirq resume the system is hanging by the same amount of time as
> the system was sleeping.

I'm able to reproduce this problem on Toshiba Portege R500 with similar
symptoms, although that box sometimes hangs hard during resume from system
suspend with the "caps lock" LED blinking.

Reverting the patch fixes the problem 100% of the time.

Thanks,
Rafael
0 new messages