* [PATCH 2/9] timekeeping: Fix potential lost pv notification of time change
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 3/9] timekeeping: Avoid possible deadlock from clock_was_set_delayed John Stultz
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Sasha Levin, Thomas Gleixner, Ingo Molnar,
David Vrabel, Konrad Rzeszutek Wilk, Prarit Bhargava,
Richard Cochran, xen-devel, stable
In 780427f0e11 (Indicate that clock was set in the pvclock
gtod notifier), logic was added to pass a CLOCK_WAS_SET
notification to the pvclock notifier chain.
While that patch added a action flag returned from
accumulate_nsecs_to_secs(), it only uses the returned value
in one location, and not in the logarithmic accumulation.
This means if a leap second triggered during the logarithmic
accumulation (which is most likely where it would happen),
the notification that the clock was set would not make it to
the pv notifiers.
This patch extends the logarithmic_accumulation pass down
that action flag so proper notification will occur.
This patch also changes the varialbe action -> clock_set
per Ingo's suggestion.
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: <xen-devel@lists.xen.org>
Cc: stable <stable@vger.kernel.org> #3.11+
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 7488f0b..051855f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1256,7 +1256,7 @@ out_adjust:
static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
{
u64 nsecps = (u64)NSEC_PER_SEC << tk->shift;
- unsigned int action = 0;
+ unsigned int clock_set = 0;
while (tk->xtime_nsec >= nsecps) {
int leap;
@@ -1279,10 +1279,10 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
__timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
clock_was_set_delayed();
- action = TK_CLOCK_WAS_SET;
+ clock_set = TK_CLOCK_WAS_SET;
}
}
- return action;
+ return clock_set;
}
/**
@@ -1295,7 +1295,8 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
* Returns the unconsumed cycles.
*/
static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset,
- u32 shift)
+ u32 shift,
+ unsigned int *clock_set)
{
cycle_t interval = tk->cycle_interval << shift;
u64 raw_nsecs;
@@ -1309,7 +1310,7 @@ static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset,
tk->cycle_last += interval;
tk->xtime_nsec += tk->xtime_interval << shift;
- accumulate_nsecs_to_secs(tk);
+ *clock_set |= accumulate_nsecs_to_secs(tk);
/* Accumulate raw time */
raw_nsecs = (u64)tk->raw_interval << shift;
@@ -1367,7 +1368,7 @@ static void update_wall_time(void)
struct timekeeper *tk = &shadow_timekeeper;
cycle_t offset;
int shift = 0, maxshift;
- unsigned int action;
+ unsigned int clock_set = 0;
unsigned long flags;
raw_spin_lock_irqsave(&timekeeper_lock, flags);
@@ -1402,7 +1403,8 @@ static void update_wall_time(void)
maxshift = (64 - (ilog2(ntp_tick_length())+1)) - 1;
shift = min(shift, maxshift);
while (offset >= tk->cycle_interval) {
- offset = logarithmic_accumulation(tk, offset, shift);
+ offset = logarithmic_accumulation(tk, offset, shift,
+ &clock_set);
if (offset < tk->cycle_interval<<shift)
shift--;
}
@@ -1420,7 +1422,7 @@ static void update_wall_time(void)
* Finally, make sure that after the rounding
* xtime_nsec isn't larger than NSEC_PER_SEC
*/
- action = accumulate_nsecs_to_secs(tk);
+ clock_set |= accumulate_nsecs_to_secs(tk);
write_seqcount_begin(&timekeeper_seq);
/* Update clock->cycle_last with the new value */
@@ -1436,7 +1438,7 @@ static void update_wall_time(void)
* updating.
*/
memcpy(real_tk, tk, sizeof(*tk));
- timekeeping_update(real_tk, action);
+ timekeeping_update(real_tk, clock_set);
write_seqcount_end(&timekeeper_seq);
out:
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/9] timekeeping: Fix potential lost pv notification of time change
@ 2014-01-02 22:23 ` John Stultz
0 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: Prarit Bhargava, Richard Cochran, stable, xen-devel, John Stultz,
David Vrabel, Sasha Levin, Thomas Gleixner, Ingo Molnar
In 780427f0e11 (Indicate that clock was set in the pvclock
gtod notifier), logic was added to pass a CLOCK_WAS_SET
notification to the pvclock notifier chain.
While that patch added a action flag returned from
accumulate_nsecs_to_secs(), it only uses the returned value
in one location, and not in the logarithmic accumulation.
This means if a leap second triggered during the logarithmic
accumulation (which is most likely where it would happen),
the notification that the clock was set would not make it to
the pv notifiers.
This patch extends the logarithmic_accumulation pass down
that action flag so proper notification will occur.
This patch also changes the varialbe action -> clock_set
per Ingo's suggestion.
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: <xen-devel@lists.xen.org>
Cc: stable <stable@vger.kernel.org> #3.11+
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 7488f0b..051855f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1256,7 +1256,7 @@ out_adjust:
static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
{
u64 nsecps = (u64)NSEC_PER_SEC << tk->shift;
- unsigned int action = 0;
+ unsigned int clock_set = 0;
while (tk->xtime_nsec >= nsecps) {
int leap;
@@ -1279,10 +1279,10 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
__timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
clock_was_set_delayed();
- action = TK_CLOCK_WAS_SET;
+ clock_set = TK_CLOCK_WAS_SET;
}
}
- return action;
+ return clock_set;
}
/**
@@ -1295,7 +1295,8 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
* Returns the unconsumed cycles.
*/
static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset,
- u32 shift)
+ u32 shift,
+ unsigned int *clock_set)
{
cycle_t interval = tk->cycle_interval << shift;
u64 raw_nsecs;
@@ -1309,7 +1310,7 @@ static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset,
tk->cycle_last += interval;
tk->xtime_nsec += tk->xtime_interval << shift;
- accumulate_nsecs_to_secs(tk);
+ *clock_set |= accumulate_nsecs_to_secs(tk);
/* Accumulate raw time */
raw_nsecs = (u64)tk->raw_interval << shift;
@@ -1367,7 +1368,7 @@ static void update_wall_time(void)
struct timekeeper *tk = &shadow_timekeeper;
cycle_t offset;
int shift = 0, maxshift;
- unsigned int action;
+ unsigned int clock_set = 0;
unsigned long flags;
raw_spin_lock_irqsave(&timekeeper_lock, flags);
@@ -1402,7 +1403,8 @@ static void update_wall_time(void)
maxshift = (64 - (ilog2(ntp_tick_length())+1)) - 1;
shift = min(shift, maxshift);
while (offset >= tk->cycle_interval) {
- offset = logarithmic_accumulation(tk, offset, shift);
+ offset = logarithmic_accumulation(tk, offset, shift,
+ &clock_set);
if (offset < tk->cycle_interval<<shift)
shift--;
}
@@ -1420,7 +1422,7 @@ static void update_wall_time(void)
* Finally, make sure that after the rounding
* xtime_nsec isn't larger than NSEC_PER_SEC
*/
- action = accumulate_nsecs_to_secs(tk);
+ clock_set |= accumulate_nsecs_to_secs(tk);
write_seqcount_begin(&timekeeper_seq);
/* Update clock->cycle_last with the new value */
@@ -1436,7 +1438,7 @@ static void update_wall_time(void)
* updating.
*/
memcpy(real_tk, tk, sizeof(*tk));
- timekeeping_update(real_tk, action);
+ timekeeping_update(real_tk, clock_set);
write_seqcount_end(&timekeeper_seq);
out:
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/9] timekeeping: Avoid possible deadlock from clock_was_set_delayed
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
2014-01-02 22:23 ` John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 4/9] tick/timekeeping: Call update_wall_time outside the jiffies lock John Stultz
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Thomas Gleixner, Prarit Bhargava, Richard Cochran,
Ingo Molnar, Sasha Levin, stable
As part of normal operaions, the hrtimer subsystem frequently calls
into the timekeeping code, creating a locking order of
hrtimer locks -> timekeeping locks
clock_was_set_delayed() was suppoed to allow us to avoid deadlocks
between the timekeeping the hrtimer subsystem, so that we could
notify the hrtimer subsytem the time had changed while holding
the timekeeping locks. This was done by scheduling delayed work
that would run later once we were out of the timekeeing code.
But unfortunately the lock chains are complex enoguh that in
scheduling delayed work, we end up eventually trying to grab
an hrtimer lock.
Sasha Levin noticed this in testing when the new seqlock lockdep
enablement triggered the following (somewhat abrieviated) message:
[ 251.100221] ======================================================
[ 251.100221] [ INFO: possible circular locking dependency detected ]
[ 251.100221] 3.13.0-rc2-next-20131206-sasha-00005-g8be2375-dirty #4053 Not tainted
[ 251.101967] -------------------------------------------------------
[ 251.101967] kworker/10:1/4506 is trying to acquire lock:
[ 251.101967] (timekeeper_seq){----..}, at: [<ffffffff81160e96>] retrigger_next_event+0x56/0x70
[ 251.101967]
[ 251.101967] but task is already holding lock:
[ 251.101967] (hrtimer_bases.lock#11){-.-...}, at: [<ffffffff81160e7c>] retrigger_next_event+0x3c/0x70
[ 251.101967]
[ 251.101967] which lock already depends on the new lock.
[ 251.101967]
[ 251.101967]
[ 251.101967] the existing dependency chain (in reverse order) is:
[ 251.101967]
-> #5 (hrtimer_bases.lock#11){-.-...}:
[snipped]
-> #4 (&rt_b->rt_runtime_lock){-.-...}:
[snipped]
-> #3 (&rq->lock){-.-.-.}:
[snipped]
-> #2 (&p->pi_lock){-.-.-.}:
[snipped]
-> #1 (&(&pool->lock)->rlock){-.-...}:
[ 251.101967] [<ffffffff81194803>] validate_chain+0x6c3/0x7b0
[ 251.101967] [<ffffffff81194d9d>] __lock_acquire+0x4ad/0x580
[ 251.101967] [<ffffffff81194ff2>] lock_acquire+0x182/0x1d0
[ 251.101967] [<ffffffff84398500>] _raw_spin_lock+0x40/0x80
[ 251.101967] [<ffffffff81153e69>] __queue_work+0x1a9/0x3f0
[ 251.101967] [<ffffffff81154168>] queue_work_on+0x98/0x120
[ 251.101967] [<ffffffff81161351>] clock_was_set_delayed+0x21/0x30
[ 251.101967] [<ffffffff811c4bd1>] do_adjtimex+0x111/0x160
[ 251.101967] [<ffffffff811e2711>] compat_sys_adjtimex+0x41/0x70
[ 251.101967] [<ffffffff843a4b49>] ia32_sysret+0x0/0x5
[ 251.101967]
-> #0 (timekeeper_seq){----..}:
[snipped]
[ 251.101967] other info that might help us debug this:
[ 251.101967]
[ 251.101967] Chain exists of:
timekeeper_seq --> &rt_b->rt_runtime_lock --> hrtimer_bases.lock#11
[ 251.101967] Possible unsafe locking scenario:
[ 251.101967]
[ 251.101967] CPU0 CPU1
[ 251.101967] ---- ----
[ 251.101967] lock(hrtimer_bases.lock#11);
[ 251.101967] lock(&rt_b->rt_runtime_lock);
[ 251.101967] lock(hrtimer_bases.lock#11);
[ 251.101967] lock(timekeeper_seq);
[ 251.101967]
[ 251.101967] *** DEADLOCK ***
[ 251.101967]
[ 251.101967] 3 locks held by kworker/10:1/4506:
[ 251.101967] #0: (events){.+.+.+}, at: [<ffffffff81154960>] process_one_work+0x200/0x530
[ 251.101967] #1: (hrtimer_work){+.+...}, at: [<ffffffff81154960>] process_one_work+0x200/0x530
[ 251.101967] #2: (hrtimer_bases.lock#11){-.-...}, at: [<ffffffff81160e7c>] retrigger_next_event+0x3c/0x70
[ 251.101967]
[ 251.101967] stack backtrace:
[ 251.101967] CPU: 10 PID: 4506 Comm: kworker/10:1 Not tainted 3.13.0-rc2-next-20131206-sasha-00005-g8be2375-dirty #4053
[ 251.101967] Workqueue: events clock_was_set_work
So the best solution is to avoid calling clock_was_set_delayed() while
holding the timekeeping lock, and instead using a flag variable to
decide if we should call clock_was_set() once we've released the locks.
This works for the case here, where the do_adjtimex() was the deadlock
trigger point. Unfortuantely, in update_wall_time() we still hold
the jiffies lock, which would deadlock with the ipi triggered by
clock_was_set(), preventing us from calling it even after we drop the
timekeeping lock. So instead call clock_was_set_delayed() at that point.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: stable <stable@vger.kernel.org> #3.10+
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Tested-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 051855f..d62682b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1278,7 +1278,6 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
__timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
- clock_was_set_delayed();
clock_set = TK_CLOCK_WAS_SET;
}
}
@@ -1442,6 +1441,19 @@ static void update_wall_time(void)
write_seqcount_end(&timekeeper_seq);
out:
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
+ if (clock_was_set) {
+ /*
+ * XXX - I'd rather we just call clock_was_set(), but
+ * since we're currently holding the jiffies lock, calling
+ * clock_was_set would trigger an ipi which would then grab
+ * the jiffies lock and we'd deadlock. :(
+ * The right solution should probably be droping
+ * the jiffies lock before calling update_wall_time
+ * but that requires some rework of the tick sched
+ * code.
+ */
+ clock_was_set_delayed();
+ }
}
/**
@@ -1702,11 +1714,13 @@ int do_adjtimex(struct timex *txc)
if (tai != orig_tai) {
__timekeeping_set_tai_offset(tk, tai);
timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
- clock_was_set_delayed();
}
write_seqcount_end(&timekeeper_seq);
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
+ if (tai != orig_tai)
+ clock_was_set();
+
ntp_notify_cmos_timer();
return ret;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/9] tick/timekeeping: Call update_wall_time outside the jiffies lock
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 3/9] timekeeping: Avoid possible deadlock from clock_was_set_delayed John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 5/9] timekeeping: Fix CLOCK_TAI timer/nanosleep delays John Stultz
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Sasha Levin, Thomas Gleixner, Prarit Bhargava,
Richard Cochran, Ingo Molnar
Since the xtime lock was split into the timekeeping lock and
the jiffies lock, we no longer need to call update_wall_time()
while holding the jiffies lock.
Thus, this patch splits update_wall_time() out from do_timer().
This allows us to get away from calling clock_was_set_delayed()
in update_wall_time() and instead use the standard clock_was_set()
call that previously would deadlock, as it causes the jiffies lock
to be acquired.
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/tick-common.c | 1 +
kernel/time/tick-internal.h | 1 +
kernel/time/tick-sched.c | 1 +
kernel/time/timekeeping.c | 19 ++++---------------
4 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 64522ec..91c5f27 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -70,6 +70,7 @@ static void tick_periodic(int cpu)
do_timer(1);
write_sequnlock(&jiffies_lock);
+ update_wall_time();
}
update_process_times(user_mode(get_irq_regs()));
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index e2bced5..8329669 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -155,3 +155,4 @@ static inline int tick_device_is_functional(struct clock_event_device *dev)
#endif
extern void do_timer(unsigned long ticks);
+extern void update_wall_time(void);
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 2afd43f..c58b03d 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -86,6 +86,7 @@ static void tick_do_update_jiffies64(ktime_t now)
tick_next_period = ktime_add(last_jiffies_update, tick_period);
}
write_sequnlock(&jiffies_lock);
+ update_wall_time();
}
/*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d62682b..44b7e6b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1360,7 +1360,7 @@ static inline void old_vsyscall_fixup(struct timekeeper *tk)
* update_wall_time - Uses the current clocksource to increment the wall time
*
*/
-static void update_wall_time(void)
+void update_wall_time(void)
{
struct clocksource *clock;
struct timekeeper *real_tk = &timekeeper;
@@ -1441,19 +1441,8 @@ static void update_wall_time(void)
write_seqcount_end(&timekeeper_seq);
out:
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
- if (clock_was_set) {
- /*
- * XXX - I'd rather we just call clock_was_set(), but
- * since we're currently holding the jiffies lock, calling
- * clock_was_set would trigger an ipi which would then grab
- * the jiffies lock and we'd deadlock. :(
- * The right solution should probably be droping
- * the jiffies lock before calling update_wall_time
- * but that requires some rework of the tick sched
- * code.
- */
- clock_was_set_delayed();
- }
+ if (clock_set)
+ clock_was_set();
}
/**
@@ -1598,7 +1587,6 @@ struct timespec get_monotonic_coarse(void)
void do_timer(unsigned long ticks)
{
jiffies_64 += ticks;
- update_wall_time();
calc_global_load(ticks);
}
@@ -1756,4 +1744,5 @@ void xtime_update(unsigned long ticks)
write_seqlock(&jiffies_lock);
do_timer(ticks);
write_sequnlock(&jiffies_lock);
+ update_wall_time();
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/9] timekeeping: Fix CLOCK_TAI timer/nanosleep delays
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
` (2 preceding siblings ...)
2014-01-02 22:23 ` [PATCH 4/9] tick/timekeeping: Call update_wall_time outside the jiffies lock John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 6/9] timekeeping: Fix missing timekeeping_update in suspend path John Stultz
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Sasha Levin, Thomas Gleixner, Prarit Bhargava,
Richard Cochran, Ingo Molnar, stable
A think-o in the calculation of the monotonic -> tai time offset
results in CLOCK_TAI timers and nanosleeps to expire late (the
latency is ~2x the tai offset).
Fix this by adding the tai offset from the realtime offset instead
of subtracting.
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: stable <stable@vger.kernel.org> #3.10+
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 44b7e6b..3f6a827 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -77,7 +77,7 @@ static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec wtm)
tk->wall_to_monotonic = wtm;
set_normalized_timespec(&tmp, -wtm.tv_sec, -wtm.tv_nsec);
tk->offs_real = timespec_to_ktime(tmp);
- tk->offs_tai = ktime_sub(tk->offs_real, ktime_set(tk->tai_offset, 0));
+ tk->offs_tai = ktime_add(tk->offs_real, ktime_set(tk->tai_offset, 0));
}
static void tk_set_sleep_time(struct timekeeper *tk, struct timespec t)
@@ -595,7 +595,7 @@ s32 timekeeping_get_tai_offset(void)
static void __timekeeping_set_tai_offset(struct timekeeper *tk, s32 tai_offset)
{
tk->tai_offset = tai_offset;
- tk->offs_tai = ktime_sub(tk->offs_real, ktime_set(tai_offset, 0));
+ tk->offs_tai = ktime_add(tk->offs_real, ktime_set(tai_offset, 0));
}
/**
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/9] timekeeping: Fix missing timekeeping_update in suspend path
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
` (3 preceding siblings ...)
2014-01-02 22:23 ` [PATCH 5/9] timekeeping: Fix CLOCK_TAI timer/nanosleep delays John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 7/9] timekeeper: fix comment typo for tk_setup_internals() John Stultz
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Sasha Levin, Thomas Gleixner, Prarit Bhargava,
Richard Cochran, Ingo Molnar, stable
Since 48cdc135d4840 (Implement a shadow timekeeper), we have to
call timekeeping_update() after any adjustment to the timekeeping
structure in order to make sure that any adjustments to the structure
persist.
In the timekeeping suspend path, we udpate the timekeeper
structure, so we should be sure to update the shadow-timekeeper
before releasing the timekeeping locks. Currently this isn't done.
In most cases, the next time related code to run would be
timekeeping_resume, which does update the shadow-timekeeper, but
in an abundence of caution, this patch adds the call to
timekeeping_update() in the suspend path.
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: stable <stable@vger.kernel.org> #3.10+
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 3f6a827..2793c43 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1024,6 +1024,8 @@ static int timekeeping_suspend(void)
timekeeping_suspend_time =
timespec_add(timekeeping_suspend_time, delta_delta);
}
+
+ timekeeping_update(tk, TK_MIRROR);
write_seqcount_end(&timekeeper_seq);
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/9] timekeeper: fix comment typo for tk_setup_internals()
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
` (4 preceding siblings ...)
2014-01-02 22:23 ` [PATCH 6/9] timekeeping: Fix missing timekeeping_update in suspend path John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 8/9] rtc-cmos: Add an alarm disable quirk John Stultz
2014-01-02 22:23 ` [PATCH 9/9] timekeeping: Remove comment that's mostly out of date John Stultz
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML; +Cc: Yijing Wang, John Stultz
From: Yijing Wang <wangyijing@huawei.com>
Fix trivial comment typo for tk_setup_internals().
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 2793c43..3ff3064 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -90,8 +90,9 @@ static void tk_set_sleep_time(struct timekeeper *tk, struct timespec t)
}
/**
- * timekeeper_setup_internals - Set up internals to use clocksource clock.
+ * tk_setup_internals - Set up internals to use clocksource clock.
*
+ * @tk: The target timekeeper to setup.
* @clock: Pointer to clocksource.
*
* Calculates a fixed cycle/nsec interval for a given clocksource/adjustment
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/9] rtc-cmos: Add an alarm disable quirk
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
` (5 preceding siblings ...)
2014-01-02 22:23 ` [PATCH 7/9] timekeeper: fix comment typo for tk_setup_internals() John Stultz
@ 2014-01-02 22:23 ` John Stultz
2014-01-02 22:23 ` [PATCH 9/9] timekeeping: Remove comment that's mostly out of date John Stultz
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML
Cc: Borislav Petkov, Brecht Machiels, Thomas Gleixner, John Stultz,
Rabin Vincent, stable, Borislav Petkov
From: Borislav Petkov <bp@alien8.de>
41c7f7424259f ("rtc: Disable the alarm in the hardware (v2)") added the
functionality to disable the RTC wake alarm when shutting down the box.
However, there are at least two b0rked BIOSes we know about:
https://bugzilla.novell.com/show_bug.cgi?id=812592
https://bugzilla.novell.com/show_bug.cgi?id=805740
where, when wakeup alarm is enabled in the BIOS, the machine reboots
automatically right after shutdown, regardless of what wakeup time is
programmed.
Bisecting the issue lead to this patch so disable its functionality with
a DMI quirk only for those boxes.
Cc: Brecht Machiels <brecht@mos6581.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Rabin Vincent <rabin.vincent@stericsson.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
[jstultz: Changed variable name for clarity, added extra dmi entry]
Tested-by: Brecht Machiels <brecht@mos6581.org>
Tested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/rtc/rtc-cmos.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index f148762..a2325bc 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -34,11 +34,11 @@
#include <linux/interrupt.h>
#include <linux/spinlock.h>
#include <linux/platform_device.h>
-#include <linux/mod_devicetable.h>
#include <linux/log2.h>
#include <linux/pm.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/dmi.h>
/* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */
#include <asm-generic/rtc.h>
@@ -377,6 +377,51 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)
return 0;
}
+/*
+ * Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes.
+ */
+static bool alarm_disable_quirk;
+
+static int __init set_alarm_disable_quirk(const struct dmi_system_id *id)
+{
+ alarm_disable_quirk = true;
+ pr_info("rtc-cmos: BIOS has alarm-disable quirk. ");
+ pr_info("RTC alarms disabled\n");
+ return 0;
+}
+
+static const struct dmi_system_id rtc_quirks[] __initconst = {
+ /* https://bugzilla.novell.com/show_bug.cgi?id=805740 */
+ {
+ .callback = set_alarm_disable_quirk,
+ .ident = "IBM Truman",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "4852570"),
+ },
+ },
+ /* https://bugzilla.novell.com/show_bug.cgi?id=812592 */
+ {
+ .callback = set_alarm_disable_quirk,
+ .ident = "Gigabyte GA-990XA-UD3",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR,
+ "Gigabyte Technology Co., Ltd."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "GA-990XA-UD3"),
+ },
+ },
+ /* http://permalink.gmane.org/gmane.linux.kernel/1604474 */
+ {
+ .callback = set_alarm_disable_quirk,
+ .ident = "Toshiba Satellite L300",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Satellite L300"),
+ },
+ },
+ {}
+};
+
static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)
{
struct cmos_rtc *cmos = dev_get_drvdata(dev);
@@ -385,6 +430,9 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)
if (!is_valid_irq(cmos->irq))
return -EINVAL;
+ if (alarm_disable_quirk)
+ return 0;
+
spin_lock_irqsave(&rtc_lock, flags);
if (enabled)
@@ -1157,6 +1205,8 @@ static int __init cmos_init(void)
platform_driver_registered = true;
}
+ dmi_check_system(rtc_quirks);
+
if (retval == 0)
return 0;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 9/9] timekeeping: Remove comment that's mostly out of date
2014-01-02 22:23 ` [PATCH 1/9] timekeeping: Fix lost updates to tai adjustment John Stultz
` (6 preceding siblings ...)
2014-01-02 22:23 ` [PATCH 8/9] rtc-cmos: Add an alarm disable quirk John Stultz
@ 2014-01-02 22:23 ` John Stultz
7 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2014-01-02 22:23 UTC (permalink / raw)
To: LKML; +Cc: John Stultz
Prior to 92bb1fcf57a0c2e45f7e67fbf0a8ed475a749236 (Only
do nanosecond rounding on GENERIC_TIME_VSYSCALL_OLD
systems), the comment here was accuate, but now we can
mostly avoid the extra rounding which causes the unlikey
to be actually likely here.
So remove the out of date comment.
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/timekeeping.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 3ff3064..abfa4e8 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1134,16 +1134,6 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
* we can adjust by 1.
*/
error >>= 2;
- /*
- * XXX - In update_wall_time, we round up to the next
- * nanosecond, and store the amount rounded up into
- * the error. This causes the likely below to be unlikely.
- *
- * The proper fix is to avoid rounding up by using
- * the high precision tk->xtime_nsec instead of
- * xtime.tv_nsec everywhere. Fixing this will take some
- * time.
- */
if (likely(error <= interval))
adj = 1;
else
--
1.8.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread