All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core
@ 2016-08-31 21:50 John Stultz
  2016-08-31 21:50 ` [PATCH 1/6] hrtimer: Spelling fixes John Stultz
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava

Hey Thomas, Ingo,
  I just wanted to send out an initial queue of timekeeping items
for the 4.9 merge window.

Please let me know if you have any feedback or objections to this
set.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>

thanks
-john

The following changes since commit 3eab887a55424fc2c27553b7bfe32330df83f7b8:

  Linux 4.8-rc4 (2016-08-28 15:04:33 -0700)

are available in the git repository at:

  https://git.linaro.org/people/john.stultz/linux.git fortglx/4.9/time

for you to fetch changes up to a0a6e06d545a753740c9d8d5ce2c4fdd3ab1c021:

  time: alarmtimer: Add tracepoints for alarmtimers (2016-08-31 14:44:18 -0700)

----------------------------------------------------------------
Baolin Wang (1):
      time: alarmtimer: Add tracepoints for alarmtimers

Kyle Walker (1):
      clocksource: Defer override invalidation unless clock is unstable

Pratyush Patel (1):
      hrtimer: Spelling fixes

Ruchi Kandoi (1):
      timekeeping: Prints the amounts of time spent during suspend

Vegard Nossum (2):
      time: Avoid undefined behaviour in timespec64_add_safe()
      time: Avoid undefined behaviour in ktime_add_safe()

 include/linux/ktime.h             |   7 +++++++
 include/linux/time64.h            |   1 +
 include/trace/events/alarmtimer.h | 137 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/time/alarmtimer.c          |  23 +++++++++++++++++---
 kernel/time/clocksource.c         |  15 ++++++++++---
 kernel/time/hrtimer.c             |   6 +++---
 kernel/time/time.c                |   2 +-
 kernel/time/timekeeping_debug.c   |   2 ++
 8 files changed, 183 insertions(+), 10 deletions(-)
 create mode 100644 include/trace/events/alarmtimer.h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] hrtimer: Spelling fixes
  2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
@ 2016-08-31 21:50 ` John Stultz
  2016-08-31 21:50 ` [PATCH 2/6] clocksource: Defer override invalidation unless clock is unstable John Stultz
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: Pratyush Patel, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava, John Stultz

From: Pratyush Patel <pratyushpatel.1995@gmail.com>

Fix a minor spelling error.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Pratyush Patel <pratyushpatel.1995@gmail.com>
[jstultz: Added commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/hrtimer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 9ba7c82..252ea47 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -703,7 +703,7 @@ static void clock_was_set_work(struct work_struct *work)
 static DECLARE_WORK(hrtimer_work, clock_was_set_work);
 
 /*
- * Called from timekeeping and resume code to reprogramm the hrtimer
+ * Called from timekeeping and resume code to reprogram the hrtimer
  * interrupt device on all cpus.
  */
 void clock_was_set_delayed(void)
@@ -1241,7 +1241,7 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 
 	/*
 	 * Note: We clear the running state after enqueue_hrtimer and
-	 * we do not reprogramm the event hardware. Happens either in
+	 * we do not reprogram the event hardware. Happens either in
 	 * hrtimer_start_range_ns() or in hrtimer_interrupt()
 	 *
 	 * Note: Because we dropped the cpu_base->lock above,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] clocksource: Defer override invalidation unless clock is unstable
  2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
  2016-08-31 21:50 ` [PATCH 1/6] hrtimer: Spelling fixes John Stultz
@ 2016-08-31 21:50 ` John Stultz
  2016-08-31 21:50 ` [PATCH 3/6] timekeeping: Prints the amounts of time spent during suspend John Stultz
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: Kyle Walker, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava, Martin Schwidefsky, John Stultz

From: Kyle Walker <kwalker@redhat.com>

Clocksources don't get the VALID_FOR_HRES flag until they have been
checked by a watchdog. However, when using an override, the
clocksource_select logic will clear the override value if the
clocksource is not marked VALID_FOR_HRES during that inititial check.
When using the boot arguments clocksource=<foo>, this selection can
run before the watchdog, and can cause the override to be incorrectly
cleared.

To address this condition, the override_name is only invalidated for
unstable clocksources. Otherwise, the override is left intact until after
the watchdog has validated the clocksource as stable/unstable.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Kyle Walker <kwalker@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/clocksource.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 6a5a310..7e4fad7 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -600,9 +600,18 @@ static void __clocksource_select(bool skipcur)
 		 */
 		if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
 			/* Override clocksource cannot be used. */
-			pr_warn("Override clocksource %s is not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
-				cs->name);
-			override_name[0] = 0;
+			if (cs->flags & CLOCK_SOURCE_UNSTABLE) {
+				pr_warn("Override clocksource %s is unstable and not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
+					cs->name);
+				override_name[0] = 0;
+			} else {
+				/*
+				 * The override cannot be currently verified.
+				 * Deferring to let the watchdog check.
+				 */
+				pr_info("Override clocksource %s is not currently HRT compatible - deferring\n",
+					cs->name);
+			}
 		} else
 			/* Override clocksource can be used. */
 			best = cs;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] timekeeping: Prints the amounts of time spent during suspend
  2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
  2016-08-31 21:50 ` [PATCH 1/6] hrtimer: Spelling fixes John Stultz
  2016-08-31 21:50 ` [PATCH 2/6] clocksource: Defer override invalidation unless clock is unstable John Stultz
@ 2016-08-31 21:50 ` John Stultz
  2016-08-31 21:50 ` [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe() John Stultz
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: Ruchi Kandoi, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava, John Stultz

From: Ruchi Kandoi <kandoiruchi@google.com>

In addition to keeping a histogram of suspend times, also
print out the time spent in suspend to dmesg.

This helps to keep track of suspend time while debugging using
kernel logs.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Ruchi Kandoi <kandoiruchi@google.com>
[jstultz: Tweaked commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping_debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/time/timekeeping_debug.c b/kernel/time/timekeeping_debug.c
index 107310a6..ca9fb80 100644
--- a/kernel/time/timekeeping_debug.c
+++ b/kernel/time/timekeeping_debug.c
@@ -75,5 +75,7 @@ void tk_debug_account_sleep_time(struct timespec64 *t)
 	int bin = min(fls(t->tv_sec), NUM_BINS-1);
 
 	sleep_time_bin[bin]++;
+	pr_info("Suspended for %lld.%03lu seconds\n", (s64)t->tv_sec,
+			t->tv_nsec / NSEC_PER_MSEC);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe()
  2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
                   ` (2 preceding siblings ...)
  2016-08-31 21:50 ` [PATCH 3/6] timekeeping: Prints the amounts of time spent during suspend John Stultz
@ 2016-08-31 21:50 ` John Stultz
  2016-09-01  8:02   ` Richard Cochran
  2016-08-31 21:50 ` [PATCH 5/6] time: Avoid undefined behaviour in ktime_add_safe() John Stultz
  2016-08-31 21:50 ` [PATCH 6/6] time: alarmtimer: Add tracepoints for alarmtimers John Stultz
  5 siblings, 1 reply; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: Vegard Nossum, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava, John Stultz

From: Vegard Nossum <vegard.nossum@oracle.com>

I ran into this:

    ================================================================================
    UBSAN: Undefined behaviour in kernel/time/time.c:783:2
    signed integer overflow:
    5273 + 9223372036854771711 cannot be represented in type 'long int'
    CPU: 0 PID: 17363 Comm: trinity-c0 Not tainted 4.8.0-rc1+ #88
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org
    04/01/2014
     0000000000000000 ffff88011457f8f0 ffffffff82344f50 0000000041b58ab3
     ffffffff84f98080 ffffffff82344ea4 ffff88011457f918 ffff88011457f8c8
     ffff88011457f8e0 7fffffffffffefff ffff88011457f6d8 dffffc0000000000
    Call Trace:
     [<ffffffff82344f50>] dump_stack+0xac/0xfc
     [<ffffffff82344ea4>] ? _atomic_dec_and_lock+0xc4/0xc4
     [<ffffffff8242f4c8>] ubsan_epilogue+0xd/0x8a
     [<ffffffff8242fc04>] handle_overflow+0x202/0x23d
     [<ffffffff8242fa02>] ? val_to_string.constprop.6+0x11e/0x11e
     [<ffffffff823c7837>] ? debug_smp_processor_id+0x17/0x20
     [<ffffffff8131b581>] ? __sigqueue_free.part.13+0x51/0x70
     [<ffffffff8146d4e0>] ? rcu_is_watching+0x110/0x110
     [<ffffffff8242fc4d>] __ubsan_handle_add_overflow+0xe/0x10
     [<ffffffff81476ef8>] timespec64_add_safe+0x298/0x340
     [<ffffffff81476c60>] ? timespec_add_safe+0x330/0x330
     [<ffffffff812f7990>] ? wait_noreap_copyout+0x1d0/0x1d0
     [<ffffffff8184bf18>] poll_select_set_timeout+0xf8/0x170
     [<ffffffff8184be20>] ? poll_schedule_timeout+0x2b0/0x2b0
     [<ffffffff813aa9bb>] ? __might_sleep+0x5b/0x260
     [<ffffffff833c8a87>] __sys_recvmmsg+0x107/0x790
     [<ffffffff833c8980>] ? SyS_recvmsg+0x20/0x20
     [<ffffffff81486378>] ? hrtimer_start_range_ns+0x3b8/0x1380
     [<ffffffff845f8bfb>] ? _raw_spin_unlock_irqrestore+0x3b/0x60
     [<ffffffff8148bcea>] ? do_setitimer+0x39a/0x8e0
     [<ffffffff813aa9bb>] ? __might_sleep+0x5b/0x260
     [<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790
     [<ffffffff833c91e9>] SyS_recvmmsg+0xd9/0x160
     [<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790
     [<ffffffff823c7853>] ? __this_cpu_preempt_check+0x13/0x20
     [<ffffffff8162f680>] ? __context_tracking_exit.part.3+0x30/0x1b0
     [<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790
     [<ffffffff81007bd3>] do_syscall_64+0x1b3/0x4b0
     [<ffffffff845f936a>] entry_SYSCALL64_slow_path+0x25/0x25
    ================================================================================

Line 783 is this:

783         set_normalized_timespec64(&res, lhs.tv_sec + rhs.tv_sec,
784                         lhs.tv_nsec + rhs.tv_nsec);

In other words, since lhs.tv_sec and rhs.tv_sec are both time64_t, this
is a signed addition which will cause undefined behaviour on overflow.

Note that this is not currently a huge concern since the kernel should be
built with -fno-strict-overflow by default, but could be a problem in the
future, a problem with older compilers, or other compilers than gcc.

The easiest way to avoid the overflow is to cast one of the arguments to
unsigned (so the addition will be done using unsigned arithmetic).

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/time64.h | 1 +
 kernel/time/time.c     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/time64.h b/include/linux/time64.h
index 7e5d2fa..980c71b 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -5,6 +5,7 @@
 #include <linux/math64.h>
 
 typedef __s64 time64_t;
+typedef __u64 timeu64_t;
 
 /*
  * This wants to go into uapi/linux/time.h once we agreed about the
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 667b933..bd62fb8 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -780,7 +780,7 @@ struct timespec64 timespec64_add_safe(const struct timespec64 lhs,
 {
 	struct timespec64 res;
 
-	set_normalized_timespec64(&res, lhs.tv_sec + rhs.tv_sec,
+	set_normalized_timespec64(&res, (timeu64_t) lhs.tv_sec + rhs.tv_sec,
 			lhs.tv_nsec + rhs.tv_nsec);
 
 	if (unlikely(res.tv_sec < lhs.tv_sec || res.tv_sec < rhs.tv_sec)) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] time: Avoid undefined behaviour in ktime_add_safe()
  2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
                   ` (3 preceding siblings ...)
  2016-08-31 21:50 ` [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe() John Stultz
@ 2016-08-31 21:50 ` John Stultz
  2016-08-31 21:50 ` [PATCH 6/6] time: alarmtimer: Add tracepoints for alarmtimers John Stultz
  5 siblings, 0 replies; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: Vegard Nossum, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava, John Stultz

From: Vegard Nossum <vegard.nossum@oracle.com>

I ran into this:

    ================================================================================
    UBSAN: Undefined behaviour in kernel/time/hrtimer.c:310:16
    signed integer overflow:
    9223372036854775807 + 50000 cannot be represented in type 'long long int'
    CPU: 2 PID: 4798 Comm: trinity-c2 Not tainted 4.8.0-rc1+ #91
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
     0000000000000000 ffff88010ce6fb88 ffffffff82344740 0000000041b58ab3
     ffffffff84f97a20 ffffffff82344694 ffff88010ce6fbb0 ffff88010ce6fb60
     000000000000c350 ffff88010ce6f968 dffffc0000000000 ffffffff857bc320
    Call Trace:
     [<ffffffff82344740>] dump_stack+0xac/0xfc
     [<ffffffff82344694>] ? _atomic_dec_and_lock+0xc4/0xc4
     [<ffffffff8242df78>] ubsan_epilogue+0xd/0x8a
     [<ffffffff8242e6b4>] handle_overflow+0x202/0x23d
     [<ffffffff8242e4b2>] ? val_to_string.constprop.6+0x11e/0x11e
     [<ffffffff8236df71>] ? timerqueue_add+0x151/0x410
     [<ffffffff81485c48>] ? hrtimer_start_range_ns+0x3b8/0x1380
     [<ffffffff81795631>] ? memset+0x31/0x40
     [<ffffffff8242e6fd>] __ubsan_handle_add_overflow+0xe/0x10
     [<ffffffff81488ac9>] hrtimer_nanosleep+0x5d9/0x790
     [<ffffffff814884f0>] ? hrtimer_init_sleeper+0x80/0x80
     [<ffffffff813a9ffb>] ? __might_sleep+0x5b/0x260
     [<ffffffff8148be10>] common_nsleep+0x20/0x30
     [<ffffffff814906c7>] SyS_clock_nanosleep+0x197/0x210
     [<ffffffff81490530>] ? SyS_clock_getres+0x150/0x150
     [<ffffffff823c7113>] ? __this_cpu_preempt_check+0x13/0x20
     [<ffffffff8162ef60>] ? __context_tracking_exit.part.3+0x30/0x1b0
     [<ffffffff81490530>] ? SyS_clock_getres+0x150/0x150
     [<ffffffff81007bd3>] do_syscall_64+0x1b3/0x4b0
     [<ffffffff845f85aa>] entry_SYSCALL64_slow_path+0x25/0x25
    ================================================================================

Add a new ktime_add_unsafe() helper which doesn't check for overflow, but
doesn't throw a UBSAN warning when it does overflow either.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/ktime.h | 7 +++++++
 kernel/time/hrtimer.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/ktime.h b/include/linux/ktime.h
index 2b6a204..3ffc69e 100644
--- a/include/linux/ktime.h
+++ b/include/linux/ktime.h
@@ -64,6 +64,13 @@ static inline ktime_t ktime_set(const s64 secs, const unsigned long nsecs)
 		({ (ktime_t){ .tv64 = (lhs).tv64 + (rhs).tv64 }; })
 
 /*
+ * Same as ktime_add(), but avoids undefined behaviour on overflow; however,
+ * this means that you must check the result for overflow yourself.
+ */
+#define ktime_add_unsafe(lhs, rhs) \
+		({ (ktime_t){ .tv64 = (u64) (lhs).tv64 + (rhs).tv64 }; })
+
+/*
  * Add a ktime_t variable and a scalar nanosecond value.
  * res = kt + nsval:
  */
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 252ea47..bb5ec42 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -307,7 +307,7 @@ EXPORT_SYMBOL_GPL(__ktime_divns);
  */
 ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
 {
-	ktime_t res = ktime_add(lhs, rhs);
+	ktime_t res = ktime_add_unsafe(lhs, rhs);
 
 	/*
 	 * We use KTIME_SEC_MAX here, the maximum timeout which we can
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] time: alarmtimer: Add tracepoints for alarmtimers
  2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
                   ` (4 preceding siblings ...)
  2016-08-31 21:50 ` [PATCH 5/6] time: Avoid undefined behaviour in ktime_add_safe() John Stultz
@ 2016-08-31 21:50 ` John Stultz
  5 siblings, 0 replies; 9+ messages in thread
From: John Stultz @ 2016-08-31 21:50 UTC (permalink / raw)
  To: lkml
  Cc: Baolin Wang, Thomas Gleixner, Ingo Molnar, Richard Cochran,
	Prarit Bhargava, Steven Rostedt, John Stultz

From: Baolin Wang <baolin.wang@linaro.org>

For system debugging, we sometimes want to know who sets one
alarm timer, the time of the timer, when the timer started and
fired and so on. Thus adding tracepoints can help us trace the
alarmtimer information.

For example, when we debug the system supend/resume, if the
system is always resumed by RTC alarm, we can find out which
process set the alarm timer to resume system by below trace log:

......
Binder:2976_6-3473  [005] d..2  1076.587732: alarmtimer_start: process:Binder:2976_6
alarmtimer type:ALARM_BOOTTIME expires:1234154000000 time: 1970-1-1 0:20:35

Binder:2976_7-3480  [002] d..2  1076.592707: alarmtimer_cancel: process:Binder:2976_7
alarmtimer type:ALARM_BOOTTIME expires:1325463060000000000 time: 2012-1-2 0:11:0

Binder:2976_7-3480  [002] d..2  1076.592731: alarmtimer_start: process:Binder:2976_7
alarmtimer type:ALARM_BOOTTIME expires:1325378040000000000 time: 2012-1-1 0:34:0

system_server-2976  [003] d..2  1076.605587: alarmtimer_cancel: process:system_server
alarmtimer type:ALARM_BOOTTIME expires:1234154000000 time: 1970-1-1 0:20:35

system_server-2976  [003] d..2  1076.605608: alarmtimer_start: process:system_server
alarmtimer type:ALARM_BOOTTIME expires:1234155000000 time: 1970-1-1 0:20:35

system_server-3000  [002] ...1  1087.737565: alarmtimer_suspend: alarmtimer type:ALARM_BOOTTIME
expires time: 2012-1-1 0:34:0
......

>From the trace log, we can find out the 'Binder:2976_7' process
set one alarm timer which resumes the system.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
[jstultz: tweaked commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/trace/events/alarmtimer.h | 137 ++++++++++++++++++++++++++++++++++++++
 kernel/time/alarmtimer.c          |  23 ++++++-
 2 files changed, 157 insertions(+), 3 deletions(-)
 create mode 100644 include/trace/events/alarmtimer.h

diff --git a/include/trace/events/alarmtimer.h b/include/trace/events/alarmtimer.h
new file mode 100644
index 0000000..6a34bc9
--- /dev/null
+++ b/include/trace/events/alarmtimer.h
@@ -0,0 +1,137 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM alarmtimer
+
+#if !defined(_TRACE_ALARMTIMER_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ALARMTIMER_H
+
+#include <linux/alarmtimer.h>
+#include <linux/rtc.h>
+#include <linux/tracepoint.h>
+
+TRACE_DEFINE_ENUM(ALARM_REALTIME);
+TRACE_DEFINE_ENUM(ALARM_BOOTTIME);
+
+#define show_alarm_type(type)	__print_flags(type, " | ",	\
+	{ 1 << ALARM_REALTIME, "ALARM_REALTIME" },		\
+	{ 1 << ALARM_BOOTTIME, "ALARM_BOOTTIME" })
+
+DECLARE_EVENT_CLASS(alarm_setting,
+
+	TP_PROTO(struct rtc_time *rtc_time, int flag),
+
+	TP_ARGS(rtc_time, flag),
+
+	TP_STRUCT__entry(
+		__field(unsigned char, second)
+		__field(unsigned char, minute)
+		__field(unsigned char, hour)
+		__field(unsigned char, day)
+		__field(unsigned char, mon)
+		__field(unsigned short, year)
+		__field(unsigned char, alarm_type)
+	),
+
+	TP_fast_assign(
+		__entry->second = rtc_time->tm_sec;
+		__entry->minute = rtc_time->tm_min;
+		__entry->hour = rtc_time->tm_hour;
+		__entry->day = rtc_time->tm_mday;
+		__entry->mon = rtc_time->tm_mon;
+		__entry->year = rtc_time->tm_year;
+		__entry->alarm_type = flag;
+	),
+
+	TP_printk("alarmtimer type:%s expires time: %hu-%u-%u %u:%u:%u",
+		  show_alarm_type((1 << __entry->alarm_type)),
+		  __entry->year + 1900,
+		  __entry->mon + 1,
+		  __entry->day,
+		  __entry->hour,
+		  __entry->minute,
+		  __entry->second
+	)
+);
+
+DEFINE_EVENT(alarm_setting, alarmtimer_suspend,
+
+	TP_PROTO(struct rtc_time *time, int flag),
+
+	TP_ARGS(time, flag)
+);
+
+DECLARE_EVENT_CLASS(alarm_processing,
+
+	TP_PROTO(struct alarm *alarm, char *process_name),
+
+	TP_ARGS(alarm, process_name),
+
+	TP_STRUCT__entry(
+		__field(unsigned long long, expires)
+		__field(unsigned char, second)
+		__field(unsigned char, minute)
+		__field(unsigned char, hour)
+		__field(unsigned char, day)
+		__field(unsigned char, mon)
+		__field(unsigned short, year)
+		__field(unsigned char, alarm_type)
+		__string(name, process_name)
+	),
+
+	TP_fast_assign(
+		__entry->expires = alarm->node.expires.tv64;
+		__entry->second = rtc_ktime_to_tm(alarm->node.expires).tm_sec;
+		__entry->minute = rtc_ktime_to_tm(alarm->node.expires).tm_min;
+		__entry->hour = rtc_ktime_to_tm(alarm->node.expires).tm_hour;
+		__entry->day = rtc_ktime_to_tm(alarm->node.expires).tm_mday;
+		__entry->mon = rtc_ktime_to_tm(alarm->node.expires).tm_mon;
+		__entry->year = rtc_ktime_to_tm(alarm->node.expires).tm_year;
+		__entry->alarm_type = alarm->type;
+		__assign_str(name, process_name);
+	),
+
+	TP_printk("process:%s alarmtimer type:%s expires:%llu "
+		  "time: %hu-%u-%u %u:%u:%u",
+		  __get_str(name),
+		  show_alarm_type((1 << __entry->alarm_type)),
+		  __entry->expires,
+		  __entry->year + 1900,
+		  __entry->mon + 1,
+		  __entry->day,
+		  __entry->hour,
+		  __entry->minute,
+		  __entry->second
+	)
+);
+
+DEFINE_EVENT(alarm_processing, alarmtimer_fired,
+
+	TP_PROTO(struct alarm *alarm, char *process_name),
+
+	TP_ARGS(alarm, process_name)
+);
+
+DEFINE_EVENT(alarm_processing, alarmtimer_start,
+
+	TP_PROTO(struct alarm *alarm, char *process_name),
+
+	TP_ARGS(alarm, process_name)
+);
+
+DEFINE_EVENT(alarm_processing, alarmtimer_restart,
+
+	TP_PROTO(struct alarm *alarm, char *process_name),
+
+	TP_ARGS(alarm, process_name)
+);
+
+DEFINE_EVENT(alarm_processing, alarmtimer_cancel,
+
+	TP_PROTO(struct alarm *alarm, char *process_name),
+
+	TP_ARGS(alarm, process_name)
+);
+
+#endif /* _TRACE_ALARMTIMER_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index c3aad68..7cd15eb 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -26,6 +26,9 @@
 #include <linux/workqueue.h>
 #include <linux/freezer.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/alarmtimer.h>
+
 /**
  * struct alarm_base - Alarm timer bases
  * @lock:		Lock for syncrhonized access to the base
@@ -194,6 +197,7 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer)
 	}
 	spin_unlock_irqrestore(&base->lock, flags);
 
+	trace_alarmtimer_fired(alarm, NULL);
 	return ret;
 
 }
@@ -218,11 +222,11 @@ EXPORT_SYMBOL_GPL(alarm_expires_remaining);
  */
 static int alarmtimer_suspend(struct device *dev)
 {
-	struct rtc_time tm;
+	struct rtc_time tm, tm_set;
 	ktime_t min, now;
 	unsigned long flags;
 	struct rtc_device *rtc;
-	int i;
+	int i, type = 0;
 	int ret;
 
 	spin_lock_irqsave(&freezer_delta_lock, flags);
@@ -247,8 +251,10 @@ static int alarmtimer_suspend(struct device *dev)
 		if (!next)
 			continue;
 		delta = ktime_sub(next->expires, base->gettime());
-		if (!min.tv64 || (delta.tv64 < min.tv64))
+		if (!min.tv64 || (delta.tv64 < min.tv64)) {
 			min = delta;
+			type = i;
+		}
 	}
 	if (min.tv64 == 0)
 		return 0;
@@ -264,6 +270,11 @@ static int alarmtimer_suspend(struct device *dev)
 	now = rtc_tm_to_ktime(tm);
 	now = ktime_add(now, min);
 
+	if (trace_alarmtimer_suspend_enabled()) {
+		tm_set = rtc_ktime_to_tm(now);
+		trace_alarmtimer_suspend(&tm_set, type);
+	}
+
 	/* Set alarm, if in the past reject suspend briefly to handle */
 	ret = rtc_timer_start(rtc, &rtctimer, now, ktime_set(0, 0));
 	if (ret < 0)
@@ -342,6 +353,8 @@ void alarm_start(struct alarm *alarm, ktime_t start)
 	alarmtimer_enqueue(base, alarm);
 	hrtimer_start(&alarm->timer, alarm->node.expires, HRTIMER_MODE_ABS);
 	spin_unlock_irqrestore(&base->lock, flags);
+
+	trace_alarmtimer_start(alarm, current->comm);
 }
 EXPORT_SYMBOL_GPL(alarm_start);
 
@@ -369,6 +382,8 @@ void alarm_restart(struct alarm *alarm)
 	hrtimer_restart(&alarm->timer);
 	alarmtimer_enqueue(base, alarm);
 	spin_unlock_irqrestore(&base->lock, flags);
+
+	trace_alarmtimer_restart(alarm, current->comm);
 }
 EXPORT_SYMBOL_GPL(alarm_restart);
 
@@ -390,6 +405,8 @@ int alarm_try_to_cancel(struct alarm *alarm)
 	if (ret >= 0)
 		alarmtimer_dequeue(base, alarm);
 	spin_unlock_irqrestore(&base->lock, flags);
+
+	trace_alarmtimer_cancel(alarm, current->comm);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(alarm_try_to_cancel);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe()
  2016-08-31 21:50 ` [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe() John Stultz
@ 2016-09-01  8:02   ` Richard Cochran
  2016-09-01  9:37     ` Vegard Nossum
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Cochran @ 2016-09-01  8:02 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Vegard Nossum, Thomas Gleixner, Ingo Molnar, Prarit Bhargava

On Wed, Aug 31, 2016 at 02:50:20PM -0700, John Stultz wrote:
>     UBSAN: Undefined behaviour in kernel/time/time.c:783:2
>     signed integer overflow:
>     5273 + 9223372036854771711 cannot be represented in type 'long int'

...

> Line 783 is this:
> 
> 783         set_normalized_timespec64(&res, lhs.tv_sec + rhs.tv_sec,
> 784                         lhs.tv_nsec + rhs.tv_nsec);

...

> Note that this is not currently a huge concern since the kernel should be
> built with -fno-strict-overflow by default, but could be a problem in the
> future, a problem with older compilers, or other compilers than gcc.

Is this really a concern at all?  The value 9223372036854771711 is a
huge number of seconds.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe()
  2016-09-01  8:02   ` Richard Cochran
@ 2016-09-01  9:37     ` Vegard Nossum
  0 siblings, 0 replies; 9+ messages in thread
From: Vegard Nossum @ 2016-09-01  9:37 UTC (permalink / raw)
  To: Richard Cochran, John Stultz
  Cc: lkml, Thomas Gleixner, Ingo Molnar, Prarit Bhargava

On 09/01/2016 10:02 AM, Richard Cochran wrote:
> On Wed, Aug 31, 2016 at 02:50:20PM -0700, John Stultz wrote:
>>     UBSAN: Undefined behaviour in kernel/time/time.c:783:2
>>     signed integer overflow:
>>     5273 + 9223372036854771711 cannot be represented in type 'long int'
>
> ...
>
>> Line 783 is this:
>>
>> 783         set_normalized_timespec64(&res, lhs.tv_sec + rhs.tv_sec,
>> 784                         lhs.tv_nsec + rhs.tv_nsec);
>
> ...
>
>> Note that this is not currently a huge concern since the kernel should be
>> built with -fno-strict-overflow by default, but could be a problem in the
>> future, a problem with older compilers, or other compilers than gcc.
>
> Is this really a concern at all?  The value 9223372036854771711 is a
> huge number of seconds.

The problem is that "undefined behaviour" means the compiler is
technically free to do whatever it wants -- it could issue an invalid
opcode, which would crash the kernel -- and this bit of code is easily
reached from userspace, making this a potential DOS vector (or worse,
depending on what exactly the compiler chooses to do).

Now the kernel is compiled with -fno-strict-overflow when it is
supported by the compiler, which in practice makes signed integer
overflow defined the same way as unsigned integer overflow (i.e. it
wraps around).

As the patch message says, this could be a problem with older and
non-gcc compilers.

So the concern is not the huge value or an overflow in itself, the
concern is the undefined behaviour where we may not be in control
of what happens.

There's probably at least a dozen other easily reachable overflows like
this in the kernel currently and I have no evidence of any actual
compilers doing really bad things, so yes, this is all somewhat
theoretical, but it's easy to fix which doesn't complicate the code and
adheres to the C language standard, so why not?


Vegard

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-09-01  9:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-31 21:50 [GIT PULL][PATCH 0/6] 4.9 timekeeping changes for tip/timers/core John Stultz
2016-08-31 21:50 ` [PATCH 1/6] hrtimer: Spelling fixes John Stultz
2016-08-31 21:50 ` [PATCH 2/6] clocksource: Defer override invalidation unless clock is unstable John Stultz
2016-08-31 21:50 ` [PATCH 3/6] timekeeping: Prints the amounts of time spent during suspend John Stultz
2016-08-31 21:50 ` [PATCH 4/6] time: Avoid undefined behaviour in timespec64_add_safe() John Stultz
2016-09-01  8:02   ` Richard Cochran
2016-09-01  9:37     ` Vegard Nossum
2016-08-31 21:50 ` [PATCH 5/6] time: Avoid undefined behaviour in ktime_add_safe() John Stultz
2016-08-31 21:50 ` [PATCH 6/6] time: alarmtimer: Add tracepoints for alarmtimers John Stultz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.