timekeeping: Unify clock MONOTONIC and clock BOOTTIME

PATCH In review Thomas Gleixner 13 participants 40 messages Mar 1, 2018 – Apr 26, 2018

Subsystems (9)

Documentation HID Tracing Input drivers +5 more subsystems

lore

Share & tools

lore

Thomas Gleixner

author

2018-03-01 16:52 UTC #0 lore raw

cover timekeeping: Unify clock MONOTONIC and clock BOOTTIME

While working through the melted spectrum induced backlog I found that last years discussion about unifying clock MONOTONIC and clock BOOTTIME has died out and obviously nobody had time to do testing on that half baken RFC patch:

https://lkml.kernel.org/r/alpine.DEB.2.20.1711180123360.2186@nanos

So I sat down and revisited it a bit more carefully than last time. The following series is the result of this work. The main differences are:

1) Provide CLOCK_MONOTONIC_ACTIVE which has the current CLOCK_MONOTONIC behaviour, i.e. it's monotonic time since boot minus the time spent in suspend.

The reason why I introduced it is that currently it's possible for user space to determine the time spent in suspend by subtracting clock monotonic and clock boot time time stamps. When CLOCK_MONOTONIC behaves like CLOCK_BOOTTIME then this is not longer possible.

As a side effect when CLOCK_MONOTONIC_ACTIVE is supported by a kernel then this is also an indicator that CLOCK_MONOTONIC and CLOCK_BOOTTIME are identical. So it's possible to determine this w/o playing games with kernel versions etc. Older kernels simply return -EiNVAL if CLOCK_MONOTONIC_ACTIVE is requested in clock_gettime().

2) Fixed a few places which I missed last time, which means also that my warning in the original changelog was justified.

3) Consolidated the core users to get rid of the seperate implementations for the two clocks

Note, there are user space visible side effects. The difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is well documented and it's unclear whether applications rely on it or not.

This really needs lot of testing, documentation updates and more input from userspace folks to make a final decision.

Thanks,

tglx

---

 Documentation/trace/ftrace.txt      |   14 +-----
 drivers/input/evdev.c               |    7 ---
 include/linux/hrtimer.h             |    2 
 include/linux/timekeeper_internal.h |    2 
 include/linux/timekeeping.h         |   37 +++++------------
 include/uapi/linux/time.h           |    1 
 kernel/time/hrtimer.c               |   16 -------
 kernel/time/posix-stubs.c           |    2 
 kernel/time/posix-timers.c          |   26 ++++--------
 kernel/time/tick-common.c           |   15 ++++++
 kernel/time/tick-internal.h         |    6 ++
 kernel/time/tick-sched.c            |    9 ++++
 kernel/time/timekeeping.c           |   78 ++++++++++++++++++------------------
 kernel/time/timekeeping.h           |    1 
 kernel/trace/trace.c                |    2 
 15 files changed, 104 insertions(+), 114 deletions(-)

Thomas Gleixner

author

2018-03-01 16:52 UTC #1 lore raw

PATCH 1/7 timekeeping: Provide CLOCK_MONOTONIC_ACTIVE

The planned change to unify the behaviour of clock MONOTONIC and clock BOOTTIME vs. suspend removes the ability to retrieve the active nonsuspended time of a system.

Provide CLOCK_MONOTONIC_ACTIVE which return the active non suspended time of the system via clock_gettime(). This is the behaviour of CLOCK_MONOTONIC bevor the BOOTTIME/MONOTONIC unification.

It also allows applications to detect programmatically that clock MONOTONIC and clock BOOTTIME are identical.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/timekeeper_internal.h |    2 ++
 include/linux/timekeeping.h         |    1 +
 include/uapi/linux/time.h           |    1 +
 kernel/time/posix-stubs.c           |    2 ++
 kernel/time/posix-timers.c          |   13 +++++++++++++
 kernel/time/timekeeping.c           |   36 ++++++++++++++++++++++++++++++++++++
 6 files changed, 55 insertions(+)

include/linux/timekeeper_internal.h +2 -0

@@ -52,6 +52,7 @@ struct tk_read_base {
  * @offs_real:		Offset clock monotonic -> clock realtime
  * @offs_boot:		Offset clock monotonic -> clock boottime
  * @offs_tai:		Offset clock monotonic -> clock tai
+ * @time_suspended:	Accumulated suspend time
  * @tai_offset:		The current UTC to TAI offset in seconds
  * @clock_was_set_seq:	The sequence number of clock was set events
  * @cs_was_changed_seq:	The sequence number of clocksource change events

@@ -94,6 +95,7 @@ struct timekeeper {
 	ktime_t			offs_real;
 	ktime_t			offs_boot;
 	ktime_t			offs_tai;
+	ktime_t			time_suspended;
 	s32			tai_offset;
 	unsigned int		clock_was_set_seq;
 	u8			cs_was_changed_seq;

include/linux/timekeeping.h +1 -0

@@ -32,6 +32,7 @@ extern void getrawmonotonic64(struct tim
 extern void ktime_get_ts64(struct timespec64 *ts);
 extern time64_t ktime_get_seconds(void);
 extern time64_t ktime_get_real_seconds(void);
+extern void ktime_get_active_ts64(struct timespec64 *ts);
 
 extern int __getnstimeofday64(struct timespec64 *tv);
 extern void getnstimeofday64(struct timespec64 *tv);

include/uapi/linux/time.h +1 -0

@@ -61,6 +61,7 @@ struct itimerval {
  */
 #define CLOCK_SGI_CYCLE			10
 #define CLOCK_TAI			11
+#define CLOCK_MONOTONIC_ACTIVE		12
 
 #define MAX_CLOCKS			16
 #define CLOCKS_MASK			(CLOCK_REALTIME | CLOCK_MONOTONIC)

kernel/time/posix-stubs.c +2 -0

@@ -73,6 +73,8 @@ int do_clock_gettime(clockid_t which_clo
 	case CLOCK_BOOTTIME:
 		get_monotonic_boottime64(tp);
 		break;
+	case CLOCK_MONOTONIC_ACTIVE:
+		ktime_get_active_ts64(tp);
 	default:
 		return -EINVAL;
 	}

kernel/time/posix-timers.c +13 -0

@@ -263,6 +263,13 @@ static int posix_get_tai(clockid_t which
 	return 0;
 }
 
+static int posix_get_monotonic_active(clockid_t which_clock,
+				      struct timespec64 *tp)
+{
+	ktime_get_active_ts64(tp);
+	return 0;
+}
+
 static int posix_get_hrtimer_res(clockid_t which_clock, struct timespec64 *tp)
 {
 	tp->tv_sec = 0;

@@ -1330,6 +1337,11 @@ static const struct k_clock clock_bootti
 	.timer_arm		= common_hrtimer_arm,
 };
 
+static const struct k_clock clock_monotonic_active = {
+	.clock_getres		= posix_get_hrtimer_res,
+	.clock_get		= posix_get_monotonic_active,
+};
+
 static const struct k_clock * const posix_clocks[] = {
 	[CLOCK_REALTIME]		= &clock_realtime,
 	[CLOCK_MONOTONIC]		= &clock_monotonic,

@@ -1342,6 +1354,7 @@ static const struct k_clock * const posi
 	[CLOCK_REALTIME_ALARM]		= &alarm_clock,
 	[CLOCK_BOOTTIME_ALARM]		= &alarm_clock,
 	[CLOCK_TAI]			= &clock_tai,
+	[CLOCK_MONOTONIC_ACTIVE]	= &clock_monotonic_active,
 };
 
 static const struct k_clock *clockid_to_kclock(const clockid_t id)

kernel/time/timekeeping.c +36 -0

@@ -139,6 +139,9 @@ static void tk_set_wall_to_mono(struct t
 static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
 {
 	tk->offs_boot = ktime_add(tk->offs_boot, delta);
+
+	/* Accumulate time spent in suspend */
+	tk->time_suspended += delta;
 }
 
 /*

@@ -886,6 +889,39 @@ void ktime_get_ts64(struct timespec64 *t
 EXPORT_SYMBOL_GPL(ktime_get_ts64);
 
 /**
+ * ktime_get_active_ts64 - Get the active nonsuspended monotonic clock
+ * @ts:		pointer to timespec variable
+ *
+ * The function calculates the monotonic clock from the realtime clock and
+ * the wall_to_monotonic offset, subtracts the accumulated suspend time and
+ * stores the result in normalized timespec64 format in the variable
+ * pointed to by @ts.
+ */
+void ktime_get_active_ts64(struct timespec64 *ts)
+{
+	struct timekeeper *tk = &tk_core.timekeeper;
+	struct timespec64 tomono, tsusp;
+	u64 nsec, nssusp;
+	unsigned int seq;
+
+	WARN_ON(timekeeping_suspended);
+
+	do {
+		seq = read_seqcount_begin(&tk_core.seq);
+		ts->tv_sec = tk->xtime_sec;
+		nsec = timekeeping_get_ns(&tk->tkr_mono);
+		tomono = tk->wall_to_monotonic;
+		nssusp = tk->time_suspended;
+	} while (read_seqcount_retry(&tk_core.seq, seq));
+
+	ts->tv_sec += tomono.tv_sec;
+	ts->tv_nsec = 0;
+	timespec64_add_ns(ts, nsec + tomono.tv_nsec);
+	tsusp = ns_to_timespec64(nssusp);
+	*ts = timespec64_sub(*ts, tsusp);
+}
+
+/**
  * ktime_get_seconds - Get the seconds portion of CLOCK_MONOTONIC
  *
  * Returns the seconds portion of CLOCK_MONOTONIC with a single non

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:07 UTC #2 lore raw

[tip:timers/core] timekeeping: Add the new CLOCK_MONOTONIC_ACTIVE clock

timekeeping: Add the new CLOCK_MONOTONIC_ACTIVE clock

Commit-ID:  72199320d49dbafa1a99f94f1cd60dc90035c154
Gitweb:     https://git.kernel.org/tip/72199320d49dbafa1a99f94f1cd60dc90035c154
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:32 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:21 +0100

timekeeping: Add the new CLOCK_MONOTONIC_ACTIVE clock

The planned change to unify the behaviour of the MONOTONIC and BOOTTIME clocks vs. suspend removes the ability to retrieve the active non-suspended time of a system.

Provide a new CLOCK_MONOTONIC_ACTIVE clock which returns the active non-suspended time of the system via clock_gettime().

This preserves the old behaviour of CLOCK_MONOTONIC before the BOOTTIME/MONOTONIC unification.

This new clock also allows applications to detect programmatically that the MONOTONIC and BOOTTIME clocks are identical.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/timekeeper_internal.h |  2 ++
 include/linux/timekeeping.h         |  1 +
 include/uapi/linux/time.h           |  1 +
 kernel/time/posix-stubs.c           |  2 ++
 kernel/time/posix-timers.c          | 13 +++++++++++++
 kernel/time/timekeeping.c           | 36 ++++++++++++++++++++++++++++++++++++
 6 files changed, 55 insertions(+)

include/linux/timekeeper_internal.h +2 -0

index 7acb953298a7..4b3dca173e89 100644
@@ -52,6 +52,7 @@ struct tk_read_base {
  * @offs_real:		Offset clock monotonic -> clock realtime
  * @offs_boot:		Offset clock monotonic -> clock boottime
  * @offs_tai:		Offset clock monotonic -> clock tai
+ * @time_suspended:	Accumulated suspend time
  * @tai_offset:		The current UTC to TAI offset in seconds
  * @clock_was_set_seq:	The sequence number of clock was set events
  * @cs_was_changed_seq:	The sequence number of clocksource change events

@@ -94,6 +95,7 @@ struct timekeeper {
 	ktime_t			offs_real;
 	ktime_t			offs_boot;
 	ktime_t			offs_tai;
+	ktime_t			time_suspended;
 	s32			tai_offset;
 	unsigned int		clock_was_set_seq;
 	u8			cs_was_changed_seq;

include/linux/timekeeping.h +1 -0

index b17bcce58bc4..440b1935d3a5 100644
@@ -32,6 +32,7 @@ extern void getrawmonotonic64(struct timespec64 *ts);
 extern void ktime_get_ts64(struct timespec64 *ts);
 extern time64_t ktime_get_seconds(void);
 extern time64_t ktime_get_real_seconds(void);
+extern void ktime_get_active_ts64(struct timespec64 *ts);
 
 extern int __getnstimeofday64(struct timespec64 *tv);
 extern void getnstimeofday64(struct timespec64 *tv);

include/uapi/linux/time.h +1 -0

index 53f8dd84beb5..61a187df8da2 100644
@@ -61,6 +61,7 @@ struct itimerval {
  */
 #define CLOCK_SGI_CYCLE			10
 #define CLOCK_TAI			11
+#define CLOCK_MONOTONIC_ACTIVE		12
 
 #define MAX_CLOCKS			16
 #define CLOCKS_MASK			(CLOCK_REALTIME | CLOCK_MONOTONIC)

kernel/time/posix-stubs.c +2 -0

index b258bee13b02..6259dbc0191a 100644
@@ -73,6 +73,8 @@ int do_clock_gettime(clockid_t which_clock, struct timespec64 *tp)
 	case CLOCK_BOOTTIME:
 		get_monotonic_boottime64(tp);
 		break;
+	case CLOCK_MONOTONIC_ACTIVE:
+		ktime_get_active_ts64(tp);
 	default:
 		return -EINVAL;
 	}

kernel/time/posix-timers.c +13 -0

index 75043046914e..556fe02a47a4 100644
@@ -263,6 +263,13 @@ static int posix_get_tai(clockid_t which_clock, struct timespec64 *tp)
 	return 0;
 }
 
+static int posix_get_monotonic_active(clockid_t which_clock,
+				      struct timespec64 *tp)
+{
+	ktime_get_active_ts64(tp);
+	return 0;
+}
+
 static int posix_get_hrtimer_res(clockid_t which_clock, struct timespec64 *tp)
 {
 	tp->tv_sec = 0;

@@ -1330,6 +1337,11 @@ static const struct k_clock clock_boottime = {
 	.timer_arm		= common_hrtimer_arm,
 };
 
+static const struct k_clock clock_monotonic_active = {
+	.clock_getres		= posix_get_hrtimer_res,
+	.clock_get		= posix_get_monotonic_active,
+};
+
 static const struct k_clock * const posix_clocks[] = {
 	[CLOCK_REALTIME]		= &clock_realtime,
 	[CLOCK_MONOTONIC]		= &clock_monotonic,

@@ -1342,6 +1354,7 @@ static const struct k_clock * const posix_clocks[] = {
 	[CLOCK_REALTIME_ALARM]		= &alarm_clock,
 	[CLOCK_BOOTTIME_ALARM]		= &alarm_clock,
 	[CLOCK_TAI]			= &clock_tai,
+	[CLOCK_MONOTONIC_ACTIVE]	= &clock_monotonic_active,
 };
 
 static const struct k_clock *clockid_to_kclock(const clockid_t id)

kernel/time/timekeeping.c +36 -0

index e11760121cb2..a2b7f583e64e 100644
@@ -139,6 +139,9 @@ static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
 static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
 {
 	tk->offs_boot = ktime_add(tk->offs_boot, delta);
+
+	/* Accumulate time spent in suspend */
+	tk->time_suspended += delta;
 }
 
 /*

@@ -886,6 +889,39 @@ void ktime_get_ts64(struct timespec64 *ts)
 }
 EXPORT_SYMBOL_GPL(ktime_get_ts64);
 
+/**
+ * ktime_get_active_ts64 - Get the active non-suspended monotonic clock
+ * @ts:		pointer to timespec variable
+ *
+ * The function calculates the monotonic clock from the realtime clock and
+ * the wall_to_monotonic offset, subtracts the accumulated suspend time and
+ * stores the result in normalized timespec64 format in the variable
+ * pointed to by @ts.
+ */
+void ktime_get_active_ts64(struct timespec64 *ts)
+{
+	struct timekeeper *tk = &tk_core.timekeeper;
+	struct timespec64 tomono, tsusp;
+	u64 nsec, nssusp;
+	unsigned int seq;
+
+	WARN_ON(timekeeping_suspended);
+
+	do {
+		seq = read_seqcount_begin(&tk_core.seq);
+		ts->tv_sec = tk->xtime_sec;
+		nsec = timekeeping_get_ns(&tk->tkr_mono);
+		tomono = tk->wall_to_monotonic;
+		nssusp = tk->time_suspended;
+	} while (read_seqcount_retry(&tk_core.seq, seq));
+
+	ts->tv_sec += tomono.tv_sec;
+	ts->tv_nsec = 0;
+	timespec64_add_ns(ts, nsec + tomono.tv_nsec);
+	tsusp = ns_to_timespec64(nssusp);
+	*ts = timespec64_sub(*ts, tsusp);
+}
+
 /**
  * ktime_get_seconds - Get the seconds portion of CLOCK_MONOTONIC
  *

Thomas Gleixner

author

2018-03-01 16:53 UTC #3 lore raw

PATCH 2/7 timekeeping: Make clock MONOTONIC behave like clock BOOTTIME

In reply to Thomas Gleixner

Clock MONOTONIC is not fast forwarded by the time spent in suspend on resume. This is only done for clock BOOTTIME. The reason why clock MONOTONIC is not forwarded is historical. The original Linux implementation was using jiffies as a base for clock MONOTONIC and jiffies have never been advanced after resume.

At some point when timekeeping was unified in the core code, clock MONONOTIC was advanced after resume which also advanced jiffies causing interesting side effects. As a consequence the clock MONOTONIC forwarding was disabled again and clock BOOTTIME was introduced, which allows to read time since boot.

Back then it was not possible to completely distangle clock MONOTONIC and jiffies because there were still interfaces which exposed clock MONOTONIC behaviour based on the timer wheel and therefore jiffies.

As of today none of the clock MONONOTIC facilities depends on jiffies anymore so the forwarding can be done seperately. This is achieved by forwarding the variables which are used for the jiffies update after resume before the tick is restarted,

In timekeeping resume, the change is rather simple. Instead of updating the offset between clock MONOTONIC and clock REALTIME/BOOTTIME, advance the time keeper base for the MONOTONIC and the MONOTONIC_RAW clock by the time spent in suspend.

Clock MONOTONIC is now the same as clock BOOTTIME and the offset between clock REALTIME and clock MONOTONIC is the same as before suspend.

There might be side effects in applications, which rely on the (unfortunately) well documented behaviour of clock MONOTONIC, but the downsides of the existing behaviour are probably worse.

There is one obvious issue. Up to now it was possible to retrieve the time spent in suspend by observing the delta between clock MONOTONIC and clock BOOTTIME. This is not longer available, but the previously introduced mechanism to read the active nonsuspended monotonic time can mitigate that in a detectable fashion.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc John Stultz <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Linus Torvalds <[email protected]>

---
 kernel/time/tick-common.c   |   15 +++++++++++++++
 kernel/time/tick-internal.h |    6 ++++++
 kernel/time/tick-sched.c    |    9 +++++++++
 kernel/time/timekeeping.c   |    7 ++++---
 4 files changed, 34 insertions(+), 3 deletions(-)

kernel/time/tick-common.c +15 -0

@@ -419,6 +419,19 @@ void tick_suspend_local(void)
 	clockevents_shutdown(td->evtdev);
 }
 
+static void tick_forward_next_period(void)
+{
+	ktime_t delta, now = ktime_get();
+	u64 n;
+
+	delta = ktime_sub(now, tick_next_period);
+	n = ktime_divns(delta, tick_period);
+	tick_next_period += n * tick_period;
+	if (tick_next_period < now)
+		tick_next_period += tick_period;
+	tick_sched_forward_next_period();
+}
+
 /**
  * tick_resume_local - Resume the local tick device
  *

@@ -431,6 +444,8 @@ void tick_resume_local(void)
 	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
 	bool broadcast = tick_resume_check_broadcast();
 
+	tick_forward_next_period();
+
 	clockevents_tick_resume(td->evtdev);
 	if (!broadcast) {
 		if (td->mode == TICKDEV_MODE_PERIODIC)

kernel/time/tick-internal.h +6 -0

@@ -141,6 +141,12 @@ static inline void tick_check_oneshot_br
 static inline bool tick_broadcast_oneshot_available(void) { return tick_oneshot_possible(); }
 #endif /* !(BROADCAST && ONESHOT) */
 
+#if defined(CONFIG_NO_HZ_COMMON) || defined(CONFIG_HIGH_RES_TIMERS)
+extern void tick_sched_forward_next_period(void);
+#else
+static inline void tick_sched_forward_next_period(void) { }
+#endif
+
 /* NO_HZ_FULL internal */
 #ifdef CONFIG_NO_HZ_FULL
 extern void tick_nohz_init(void);

kernel/time/tick-sched.c +9 -0

@@ -52,6 +52,15 @@ struct tick_sched *tick_get_tick_sched(i
 static ktime_t last_jiffies_update;
 
 /*
+ * Called after resume. Make sure that jiffies are not fast forwarded due to
+ * clock monotonic being forwarded by the suspended time.
+ */
+void tick_sched_forward_next_period(void)
+{
+	last_jiffies_update = tick_next_period;
+}
+
+/*
  * Must be called with interrupts disabled !
  */
 static void tick_do_update_jiffies64(ktime_t now)

kernel/time/timekeeping.c +4 -3

@@ -138,7 +138,9 @@ static void tk_set_wall_to_mono(struct t
 
 static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
 {
-	tk->offs_boot = ktime_add(tk->offs_boot, delta);
+	/* Update both bases so mono and raw stay coupled. */
+	tk->tkr_mono.base += delta;
+	tk->tkr_raw.base += delta;
 
 	/* Accumulate time spent in suspend */
 	tk->time_suspended += delta;

@@ -1621,7 +1623,6 @@ static void __timekeeping_inject_sleepti
 		return;
 	}
 	tk_xtime_add(tk, delta);
-	tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, *delta));
 	tk_update_sleep_time(tk, timespec64_to_ktime(*delta));
 	tk_debug_account_sleep_time(delta);
 }

@@ -2202,7 +2203,7 @@ void update_wall_time(void)
 void getboottime64(struct timespec64 *ts)
 {
 	struct timekeeper *tk = &tk_core.timekeeper;
-	ktime_t t = ktime_sub(tk->offs_real, tk->offs_boot);
+	ktime_t t = ktime_sub(tk->offs_real, tk->time_suspended);
 
 	*ts = ktime_to_timespec64(t);
 }

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:08 UTC #4 lore raw

[tip:timers/core] timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock

timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock

Commit-ID:  d6ed449afdb38f89a7b38ec50e367559e1b8f71f
Gitweb:     https://git.kernel.org/tip/d6ed449afdb38f89a7b38ec50e367559e1b8f71f
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:33 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:22 +0100

timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock

The MONOTONIC clock is not fast forwarded by the time spent in suspend on resume. This is only done for the BOOTTIME clock. The reason why the MONOTONIC clock is not forwarded is historical: the original Linux implementation was using jiffies as a base for the MONOTONIC clock and jiffies have never been advanced after resume.

At some point when timekeeping was unified in the core code, the MONONOTIC clock was advanced after resume which also advanced jiffies causing interesting side effects. As a consequence the the MONOTONIC clock forwarding was disabled again and the BOOTTIME clock was introduced, which allows to read time since boot.

Back then it was not possible to completely distangle the MONOTONIC clock and jiffies because there were still interfaces which exposed the MONOTONIC clock behaviour based on the timer wheel and therefore jiffies.

As of today none of the MONOTONIC clock facilities depends on jiffies anymore so the forwarding can be done seperately. This is achieved by forwarding the variables which are used for the jiffies update after resume before the tick is restarted,

In timekeeping resume, the change is rather simple. Instead of updating the offset between the MONOTONIC clock and the REALTIME/BOOTTIME clocks, advance the time keeper base for the MONOTONIC and the MONOTONIC_RAW clocks by the time spent in suspend.

The MONOTONIC clock is now the same as the BOOTTIME clock and the offset between the REALTIME and the MONOTONIC clocks is the same as before suspend.

There might be side effects in applications, which rely on the (unfortunately) well documented behaviour of the MONOTONIC clock, but the downsides of the existing behaviour are probably worse.

There is one obvious issue. Up to now it was possible to retrieve the time spent in suspend by observing the delta between the MONOTONIC clock and the BOOTTIME clock. This is not longer available, but the previously introduced mechanism to read the active non-suspended monotonic time can mitigate that in a detectable fashion.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/time/tick-common.c   | 15 +++++++++++++++
 kernel/time/tick-internal.h |  6 ++++++
 kernel/time/tick-sched.c    |  9 +++++++++
 kernel/time/timekeeping.c   |  7 ++++---
 4 files changed, 34 insertions(+), 3 deletions(-)

kernel/time/tick-common.c +15 -0

index 49edc1c4f3e6..099572ca4a8f 100644
@@ -419,6 +419,19 @@ void tick_suspend_local(void)
 	clockevents_shutdown(td->evtdev);
 }
 
+static void tick_forward_next_period(void)
+{
+	ktime_t delta, now = ktime_get();
+	u64 n;
+
+	delta = ktime_sub(now, tick_next_period);
+	n = ktime_divns(delta, tick_period);
+	tick_next_period += n * tick_period;
+	if (tick_next_period < now)
+		tick_next_period += tick_period;
+	tick_sched_forward_next_period();
+}
+
 /**
  * tick_resume_local - Resume the local tick device
  *

@@ -431,6 +444,8 @@ void tick_resume_local(void)
 	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
 	bool broadcast = tick_resume_check_broadcast();
 
+	tick_forward_next_period();
+
 	clockevents_tick_resume(td->evtdev);
 	if (!broadcast) {
 		if (td->mode == TICKDEV_MODE_PERIODIC)

kernel/time/tick-internal.h +6 -0

index e277284c2831..21efab7485ca 100644
@@ -141,6 +141,12 @@ static inline void tick_check_oneshot_broadcast_this_cpu(void) { }
 static inline bool tick_broadcast_oneshot_available(void) { return tick_oneshot_possible(); }
 #endif /* !(BROADCAST && ONESHOT) */
 
+#if defined(CONFIG_NO_HZ_COMMON) || defined(CONFIG_HIGH_RES_TIMERS)
+extern void tick_sched_forward_next_period(void);
+#else
+static inline void tick_sched_forward_next_period(void) { }
+#endif
+
 /* NO_HZ_FULL internal */
 #ifdef CONFIG_NO_HZ_FULL
 extern void tick_nohz_init(void);

kernel/time/tick-sched.c +9 -0

index 29a5733eff83..f53e37b5d248 100644
@@ -51,6 +51,15 @@ struct tick_sched *tick_get_tick_sched(int cpu)
  */
 static ktime_t last_jiffies_update;
 
+/*
+ * Called after resume. Make sure that jiffies are not fast forwarded due to
+ * clock monotonic being forwarded by the suspended time.
+ */
+void tick_sched_forward_next_period(void)
+{
+	last_jiffies_update = tick_next_period;
+}
+
 /*
  * Must be called with interrupts disabled !
  */

kernel/time/timekeeping.c +4 -3

index a2b7f583e64e..b509fe7acd64 100644
@@ -138,7 +138,9 @@ static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec64 wtm)
 
 static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
 {
-	tk->offs_boot = ktime_add(tk->offs_boot, delta);
+	/* Update both bases so mono and raw stay coupled. */
+	tk->tkr_mono.base += delta;
+	tk->tkr_raw.base += delta;
 
 	/* Accumulate time spent in suspend */
 	tk->time_suspended += delta;

@@ -1622,7 +1624,6 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
 		return;
 	}
 	tk_xtime_add(tk, delta);
-	tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, *delta));
 	tk_update_sleep_time(tk, timespec64_to_ktime(*delta));
 	tk_debug_account_sleep_time(delta);
 }

@@ -2155,7 +2156,7 @@ out:
 void getboottime64(struct timespec64 *ts)
 {
 	struct timekeeper *tk = &tk_core.timekeeper;
-	ktime_t t = ktime_sub(tk->offs_real, tk->offs_boot);
+	ktime_t t = ktime_sub(tk->offs_real, tk->time_suspended);
 
 	*ts = ktime_to_timespec64(t);
 }

Thomas Gleixner

author

2018-03-01 16:33 UTC #5 lore raw

PATCH 3/7 Input: evdev - Conflate clock MONOTONIC and BOOTTIME

In reply to Thomas Gleixner

Now that clock MONOTONIC and BOOTTIME are indentical remove all the special casing. The user space visible interfaces still support both clocks.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc [email protected]

---
 drivers/input/evdev.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

drivers/input/evdev.c +1 -6

@@ -31,7 +31,6 @@
 enum evdev_clock_type {
 	EV_CLK_REAL = 0,
 	EV_CLK_MONO,
-	EV_CLK_BOOT,
 	EV_CLK_MAX
 };

@@ -198,12 +197,10 @@ static int evdev_set_clk_type(struct evd
 	case CLOCK_REALTIME:
 		clk_type = EV_CLK_REAL;
 		break;
+	case CLOCK_BOOTTIME:
 	case CLOCK_MONOTONIC:
 		clk_type = EV_CLK_MONO;
 		break;
-	case CLOCK_BOOTTIME:
-		clk_type = EV_CLK_BOOT;
-		break;
 	default:
 		return -EINVAL;
 	}

@@ -314,8 +311,6 @@ static void evdev_events(struct input_ha
 
 	ev_time[EV_CLK_MONO] = ktime_get();
 	ev_time[EV_CLK_REAL] = ktime_mono_to_real(ev_time[EV_CLK_MONO]);
-	ev_time[EV_CLK_BOOT] = ktime_mono_to_any(ev_time[EV_CLK_MONO],
-						 TK_OFFS_BOOT);
 
 	rcu_read_lock();

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:08 UTC #6 lore raw

[tip:timers/core] Input: Evdev - unify MONOTONIC and BOOTTIME clock behavior

Input: Evdev - unify MONOTONIC and BOOTTIME clock behavior

Commit-ID:  f2d6fdbfd2389a38598d448cb8dc09d946c1b87e
Gitweb:     https://git.kernel.org/tip/f2d6fdbfd2389a38598d448cb8dc09d946c1b87e
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:34 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:22 +0100

Input: Evdev - unify MONOTONIC and BOOTTIME clock behavior

Now that the MONOTONIC and BOOTTIME clocks are indentical, remove all the special casing.

The user space visible interfaces still support both clocks, but their behavior is identical.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 drivers/input/evdev.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

drivers/input/evdev.c +1 -6

index c81c79d01d93..46115a392098 100644
@@ -31,7 +31,6 @@
 enum evdev_clock_type {
 	EV_CLK_REAL = 0,
 	EV_CLK_MONO,
-	EV_CLK_BOOT,
 	EV_CLK_MAX
 };

@@ -198,12 +197,10 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
 	case CLOCK_REALTIME:
 		clk_type = EV_CLK_REAL;
 		break;
+	case CLOCK_BOOTTIME:
 	case CLOCK_MONOTONIC:
 		clk_type = EV_CLK_MONO;
 		break;
-	case CLOCK_BOOTTIME:
-		clk_type = EV_CLK_BOOT;
-		break;
 	default:
 		return -EINVAL;
 	}

@@ -314,8 +311,6 @@ static void evdev_events(struct input_handle *handle,
 
 	ev_time[EV_CLK_MONO] = ktime_get();
 	ev_time[EV_CLK_REAL] = ktime_mono_to_real(ev_time[EV_CLK_MONO]);
-	ev_time[EV_CLK_BOOT] = ktime_mono_to_any(ev_time[EV_CLK_MONO],
-						 TK_OFFS_BOOT);
 
 	rcu_read_lock();

Thomas Gleixner

author

2018-03-01 16:53 UTC #7 lore raw

PATCH 4/7 timekeeping: Remove boot time specific code

In reply to Thomas Gleixner

Now that clock MONOTONIC and clock BOOTTIME are the same, remove all the special handling from timekeeping. Keep wrappers for the existing users of the *boot* timekeeper interfaces.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/timekeeping.h |   42 +++++++++++++++++-------------------------
 kernel/time/timekeeping.c   |   31 -------------------------------
 2 files changed, 17 insertions(+), 56 deletions(-)

include/linux/timekeeping.h +17 ~~-25~~

@@ -38,15 +38,19 @@ extern int __getnstimeofday64(struct tim
 extern void getnstimeofday64(struct timespec64 *tv);
 extern void getboottime64(struct timespec64 *ts);
 
-#define ktime_get_real_ts64(ts)	getnstimeofday64(ts)
+#define ktime_get_real_ts64(ts)		getnstimeofday64(ts)
+
+/* Clock BOOTTIME compatibility wrappers */
+static inline void get_monotonic_boottime64(struct timespec64 *ts)
+{
+	ktime_get_ts64(ts);
+}
 
 /*
  * ktime_t based interfaces
  */
-
 enum tk_offsets {
 	TK_OFFS_REAL,
-	TK_OFFS_BOOT,
 	TK_OFFS_TAI,
 	TK_OFFS_MAX,
 };

@@ -57,6 +61,10 @@ extern ktime_t ktime_mono_to_any(ktime_t
 extern ktime_t ktime_get_raw(void);
 extern u32 ktime_get_resolution_ns(void);
 
+/* Clock BOOTTIME compatibility wrappers */
+static inline ktime_t ktime_get_boottime(void) { return ktime_get(); }
+static inline u64 ktime_get_boot_ns(void) { return ktime_get(); }
+
 /**
  * ktime_get_real - get the real (wall-) time in ktime_t format
  */

@@ -66,17 +74,6 @@ static inline ktime_t ktime_get_real(voi
 }
 
 /**
- * ktime_get_boottime - Returns monotonic time since boot in ktime_t format
- *
- * This is similar to CLOCK_MONTONIC/ktime_get, but also includes the
- * time spent in suspend.
- */
-static inline ktime_t ktime_get_boottime(void)
-{
-	return ktime_get_with_offset(TK_OFFS_BOOT);
-}
-
-/**
  * ktime_get_clocktai - Returns the TAI time of day in ktime_t format
  */
 static inline ktime_t ktime_get_clocktai(void)

@@ -102,11 +99,6 @@ static inline u64 ktime_get_real_ns(void
 	return ktime_to_ns(ktime_get_real());
 }
 
-static inline u64 ktime_get_boot_ns(void)
-{
-	return ktime_to_ns(ktime_get_boottime());
-}
-
 static inline u64 ktime_get_tai_ns(void)
 {
 	return ktime_to_ns(ktime_get_clocktai());

@@ -119,17 +111,17 @@ static inline u64 ktime_get_raw_ns(void)
 
 extern u64 ktime_get_mono_fast_ns(void);
 extern u64 ktime_get_raw_fast_ns(void);
-extern u64 ktime_get_boot_fast_ns(void);
 extern u64 ktime_get_real_fast_ns(void);
 
-/*
- * timespec64 interfaces utilizing the ktime based ones
- */
-static inline void get_monotonic_boottime64(struct timespec64 *ts)
+/* Clock BOOTTIME compatibility wrappers */
+static inline u64 ktime_get_boot_fast_ns(void)
 {
-	*ts = ktime_to_timespec64(ktime_get_boottime());
+	return ktime_get_mono_fast_ns();
 }
 
+/*
+ * timespec64 interfaces utilizing the ktime based ones
+ */
 static inline void timekeeping_clocktai64(struct timespec64 *ts)
 {
 	*ts = ktime_to_timespec64(ktime_get_clocktai());

kernel/time/timekeeping.c +0 ~~-31~~

@@ -472,36 +472,6 @@ u64 ktime_get_raw_fast_ns(void)
 }
 EXPORT_SYMBOL_GPL(ktime_get_raw_fast_ns);
 
-/**
- * ktime_get_boot_fast_ns - NMI safe and fast access to boot clock.
- *
- * To keep it NMI safe since we're accessing from tracing, we're not using a
- * separate timekeeper with updates to monotonic clock and boot offset
- * protected with seqlocks. This has the following minor side effects:
- *
- * (1) Its possible that a timestamp be taken after the boot offset is updated
- * but before the timekeeper is updated. If this happens, the new boot offset
- * is added to the old timekeeping making the clock appear to update slightly
- * earlier:
- *    CPU 0                                        CPU 1
- *    timekeeping_inject_sleeptime64()
- *    __timekeeping_inject_sleeptime(tk, delta);
- *                                                 timestamp();
- *    timekeeping_update(tk, TK_CLEAR_NTP...);
- *
- * (2) On 32-bit systems, the 64-bit boot offset (tk->offs_boot) may be
- * partially updated.  Since the tk->offs_boot update is a rare event, this
- * should be a rare occurrence which postprocessing should be able to handle.
- */
-u64 notrace ktime_get_boot_fast_ns(void)
-{
-	struct timekeeper *tk = &tk_core.timekeeper;
-
-	return (ktime_get_mono_fast_ns() + ktime_to_ns(tk->offs_boot));
-}
-EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);
-
-
 /*
  * See comment for __ktime_get_fast_ns() vs. timestamp ordering
  */

@@ -793,7 +763,6 @@ EXPORT_SYMBOL_GPL(ktime_get_resolution_n
 
 static ktime_t *offsets[TK_OFFS_MAX] = {
 	[TK_OFFS_REAL]	= &tk_core.timekeeper.offs_real,
-	[TK_OFFS_BOOT]	= &tk_core.timekeeper.offs_boot,
 	[TK_OFFS_TAI]	= &tk_core.timekeeper.offs_tai,
 };

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:08 UTC #8 lore raw

[tip:timers/core] timekeeping: Remove boot time specific code

timekeeping: Remove boot time specific code

Commit-ID:  d6c7270e913db75ca5fdc79915ba780e97ae2857
Gitweb:     https://git.kernel.org/tip/d6c7270e913db75ca5fdc79915ba780e97ae2857
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:35 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:22 +0100

timekeeping: Remove boot time specific code

Now that the MONOTONIC and BOOTTIME clocks are the same, remove all the special handling from timekeeping. Keep wrappers for the existing users of the *boot* timekeeper interfaces.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/timekeeping.h | 42 +++++++++++++++++-------------------------
 kernel/time/timekeeping.c   | 31 -------------------------------
 2 files changed, 17 insertions(+), 56 deletions(-)

include/linux/timekeeping.h +17 ~~-25~~

index 440b1935d3a5..abb396731332 100644
@@ -38,15 +38,19 @@ extern int __getnstimeofday64(struct timespec64 *tv);
 extern void getnstimeofday64(struct timespec64 *tv);
 extern void getboottime64(struct timespec64 *ts);
 
-#define ktime_get_real_ts64(ts)	getnstimeofday64(ts)
+#define ktime_get_real_ts64(ts)		getnstimeofday64(ts)
+
+/* Clock BOOTTIME compatibility wrappers */
+static inline void get_monotonic_boottime64(struct timespec64 *ts)
+{
+	ktime_get_ts64(ts);
+}
 
 /*
  * ktime_t based interfaces
  */
-
 enum tk_offsets {
 	TK_OFFS_REAL,
-	TK_OFFS_BOOT,
 	TK_OFFS_TAI,
 	TK_OFFS_MAX,
 };

@@ -57,6 +61,10 @@ extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
 extern ktime_t ktime_get_raw(void);
 extern u32 ktime_get_resolution_ns(void);
 
+/* Clock BOOTTIME compatibility wrappers */
+static inline ktime_t ktime_get_boottime(void) { return ktime_get(); }
+static inline u64 ktime_get_boot_ns(void) { return ktime_get(); }
+
 /**
  * ktime_get_real - get the real (wall-) time in ktime_t format
  */

@@ -65,17 +73,6 @@ static inline ktime_t ktime_get_real(void)
 	return ktime_get_with_offset(TK_OFFS_REAL);
 }
 
-/**
- * ktime_get_boottime - Returns monotonic time since boot in ktime_t format
- *
- * This is similar to CLOCK_MONTONIC/ktime_get, but also includes the
- * time spent in suspend.
- */
-static inline ktime_t ktime_get_boottime(void)
-{
-	return ktime_get_with_offset(TK_OFFS_BOOT);
-}
-
 /**
  * ktime_get_clocktai - Returns the TAI time of day in ktime_t format
  */

@@ -102,11 +99,6 @@ static inline u64 ktime_get_real_ns(void)
 	return ktime_to_ns(ktime_get_real());
 }
 
-static inline u64 ktime_get_boot_ns(void)
-{
-	return ktime_to_ns(ktime_get_boottime());
-}
-
 static inline u64 ktime_get_tai_ns(void)
 {
 	return ktime_to_ns(ktime_get_clocktai());

@@ -119,17 +111,17 @@ static inline u64 ktime_get_raw_ns(void)
 
 extern u64 ktime_get_mono_fast_ns(void);
 extern u64 ktime_get_raw_fast_ns(void);
-extern u64 ktime_get_boot_fast_ns(void);
 extern u64 ktime_get_real_fast_ns(void);
 
-/*
- * timespec64 interfaces utilizing the ktime based ones
- */
-static inline void get_monotonic_boottime64(struct timespec64 *ts)
+/* Clock BOOTTIME compatibility wrappers */
+static inline u64 ktime_get_boot_fast_ns(void)
 {
-	*ts = ktime_to_timespec64(ktime_get_boottime());
+	return ktime_get_mono_fast_ns();
 }
 
+/*
+ * timespec64 interfaces utilizing the ktime based ones
+ */
 static inline void timekeeping_clocktai64(struct timespec64 *ts)
 {
 	*ts = ktime_to_timespec64(ktime_get_clocktai());

kernel/time/timekeeping.c +0 ~~-31~~

index b509fe7acd64..8355c8803282 100644
@@ -473,36 +473,6 @@ u64 ktime_get_raw_fast_ns(void)
 }
 EXPORT_SYMBOL_GPL(ktime_get_raw_fast_ns);
 
-/**
- * ktime_get_boot_fast_ns - NMI safe and fast access to boot clock.
- *
- * To keep it NMI safe since we're accessing from tracing, we're not using a
- * separate timekeeper with updates to monotonic clock and boot offset
- * protected with seqlocks. This has the following minor side effects:
- *
- * (1) Its possible that a timestamp be taken after the boot offset is updated
- * but before the timekeeper is updated. If this happens, the new boot offset
- * is added to the old timekeeping making the clock appear to update slightly
- * earlier:
- *    CPU 0                                        CPU 1
- *    timekeeping_inject_sleeptime64()
- *    __timekeeping_inject_sleeptime(tk, delta);
- *                                                 timestamp();
- *    timekeeping_update(tk, TK_CLEAR_NTP...);
- *
- * (2) On 32-bit systems, the 64-bit boot offset (tk->offs_boot) may be
- * partially updated.  Since the tk->offs_boot update is a rare event, this
- * should be a rare occurrence which postprocessing should be able to handle.
- */
-u64 notrace ktime_get_boot_fast_ns(void)
-{
-	struct timekeeper *tk = &tk_core.timekeeper;
-
-	return (ktime_get_mono_fast_ns() + ktime_to_ns(tk->offs_boot));
-}
-EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);
-
-
 /*
  * See comment for __ktime_get_fast_ns() vs. timestamp ordering
  */

@@ -794,7 +764,6 @@ EXPORT_SYMBOL_GPL(ktime_get_resolution_ns);
 
 static ktime_t *offsets[TK_OFFS_MAX] = {
 	[TK_OFFS_REAL]	= &tk_core.timekeeper.offs_real,
-	[TK_OFFS_BOOT]	= &tk_core.timekeeper.offs_boot,
 	[TK_OFFS_TAI]	= &tk_core.timekeeper.offs_tai,
 };

Thomas Gleixner

author

2018-03-01 16:52 UTC #9 lore raw

PATCH 5/7 posix-timers: Conflate clock MONOTONIC and BOOTTIME

In reply to Thomas Gleixner

Now that clock MONOTONIC and BOOTTIME are indentical remove all the special casing. The user space visible interfaces still support both clocks.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/time/posix-timers.c |   23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

kernel/time/posix-timers.c +1 ~~-22~~

@@ -251,12 +251,6 @@ static int posix_get_coarse_res(const cl
 	return 0;
 }
 
-static int posix_get_boottime(const clockid_t which_clock, struct timespec64 *tp)
-{
-	get_monotonic_boottime64(tp);
-	return 0;
-}
-
 static int posix_get_tai(clockid_t which_clock, struct timespec64 *tp)
 {
 	timekeeping_clocktai64(tp);

@@ -1322,21 +1316,6 @@ static const struct k_clock clock_tai =
 	.timer_arm		= common_hrtimer_arm,
 };
 
-static const struct k_clock clock_boottime = {
-	.clock_getres		= posix_get_hrtimer_res,
-	.clock_get		= posix_get_boottime,
-	.nsleep			= common_nsleep,
-	.timer_create		= common_timer_create,
-	.timer_set		= common_timer_set,
-	.timer_get		= common_timer_get,
-	.timer_del		= common_timer_del,
-	.timer_rearm		= common_hrtimer_rearm,
-	.timer_forward		= common_hrtimer_forward,
-	.timer_remaining	= common_hrtimer_remaining,
-	.timer_try_to_cancel	= common_hrtimer_try_to_cancel,
-	.timer_arm		= common_hrtimer_arm,
-};
-
 static const struct k_clock clock_monotonic_active = {
 	.clock_getres		= posix_get_hrtimer_res,
 	.clock_get		= posix_get_monotonic_active,

@@ -1350,7 +1329,7 @@ static const struct k_clock * const posi
 	[CLOCK_MONOTONIC_RAW]		= &clock_monotonic_raw,
 	[CLOCK_REALTIME_COARSE]		= &clock_realtime_coarse,
 	[CLOCK_MONOTONIC_COARSE]	= &clock_monotonic_coarse,
-	[CLOCK_BOOTTIME]		= &clock_boottime,
+	[CLOCK_BOOTTIME]		= &clock_monotonic,
 	[CLOCK_REALTIME_ALARM]		= &alarm_clock,
 	[CLOCK_BOOTTIME_ALARM]		= &alarm_clock,
 	[CLOCK_TAI]			= &clock_tai,

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:09 UTC #10 lore raw

[tip:timers/core] posix-timers: Unify MONOTONIC and BOOTTIME clock behavior

posix-timers: Unify MONOTONIC and BOOTTIME clock behavior

Commit-ID:  7250a4047aa6106006c2c9b5aff91d7d3fb77962
Gitweb:     https://git.kernel.org/tip/7250a4047aa6106006c2c9b5aff91d7d3fb77962
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:36 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:22 +0100

posix-timers: Unify MONOTONIC and BOOTTIME clock behavior

Now that the MONOTONIC and BOOTTIME clocks are indentical remove all the special casing.

The user space visible interfaces still support both clocks, but their behavior is identical.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/time/posix-timers.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

kernel/time/posix-timers.c +1 ~~-22~~

index 556fe02a47a4..8cf95bfee44f 100644
@@ -251,12 +251,6 @@ static int posix_get_coarse_res(const clockid_t which_clock, struct timespec64 *
 	return 0;
 }
 
-static int posix_get_boottime(const clockid_t which_clock, struct timespec64 *tp)
-{
-	get_monotonic_boottime64(tp);
-	return 0;
-}
-
 static int posix_get_tai(clockid_t which_clock, struct timespec64 *tp)
 {
 	timekeeping_clocktai64(tp);

@@ -1322,21 +1316,6 @@ static const struct k_clock clock_tai = {
 	.timer_arm		= common_hrtimer_arm,
 };
 
-static const struct k_clock clock_boottime = {
-	.clock_getres		= posix_get_hrtimer_res,
-	.clock_get		= posix_get_boottime,
-	.nsleep			= common_nsleep,
-	.timer_create		= common_timer_create,
-	.timer_set		= common_timer_set,
-	.timer_get		= common_timer_get,
-	.timer_del		= common_timer_del,
-	.timer_rearm		= common_hrtimer_rearm,
-	.timer_forward		= common_hrtimer_forward,
-	.timer_remaining	= common_hrtimer_remaining,
-	.timer_try_to_cancel	= common_hrtimer_try_to_cancel,
-	.timer_arm		= common_hrtimer_arm,
-};
-
 static const struct k_clock clock_monotonic_active = {
 	.clock_getres		= posix_get_hrtimer_res,
 	.clock_get		= posix_get_monotonic_active,

@@ -1350,7 +1329,7 @@ static const struct k_clock * const posix_clocks[] = {
 	[CLOCK_MONOTONIC_RAW]		= &clock_monotonic_raw,
 	[CLOCK_REALTIME_COARSE]		= &clock_realtime_coarse,
 	[CLOCK_MONOTONIC_COARSE]	= &clock_monotonic_coarse,
-	[CLOCK_BOOTTIME]		= &clock_boottime,
+	[CLOCK_BOOTTIME]		= &clock_monotonic,
 	[CLOCK_REALTIME_ALARM]		= &alarm_clock,
 	[CLOCK_BOOTTIME_ALARM]		= &alarm_clock,
 	[CLOCK_TAI]			= &clock_tai,

Thomas Gleixner

author

2018-03-01 16:52 UTC #11 lore raw

PATCH 6/7 hrtimer: Conflate clock MONOTONIC and BOOTTIME

In reply to Thomas Gleixner

Now that clock MONOTONIC and BOOTTIME are indentical remove all the special casing. The user space visible interfaces still support both clocks.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/hrtimer.h   |    2 --
 kernel/time/hrtimer.c     |   16 ++--------------
 kernel/time/timekeeping.c |    4 +---
 kernel/time/timekeeping.h |    1 -
 4 files changed, 3 insertions(+), 20 deletions(-)

include/linux/hrtimer.h +0 -2

@@ -161,11 +161,9 @@ struct hrtimer_clock_base {
 enum  hrtimer_base_type {
 	HRTIMER_BASE_MONOTONIC,
 	HRTIMER_BASE_REALTIME,
-	HRTIMER_BASE_BOOTTIME,
 	HRTIMER_BASE_TAI,
 	HRTIMER_BASE_MONOTONIC_SOFT,
 	HRTIMER_BASE_REALTIME_SOFT,
-	HRTIMER_BASE_BOOTTIME_SOFT,
 	HRTIMER_BASE_TAI_SOFT,
 	HRTIMER_MAX_CLOCK_BASES,
 };

kernel/time/hrtimer.c +2 ~~-14~~

@@ -91,11 +91,6 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base,
 			.get_time = &ktime_get_real,
 		},
 		{
-			.index = HRTIMER_BASE_BOOTTIME,
-			.clockid = CLOCK_BOOTTIME,
-			.get_time = &ktime_get_boottime,
-		},
-		{
 			.index = HRTIMER_BASE_TAI,
 			.clockid = CLOCK_TAI,
 			.get_time = &ktime_get_clocktai,

@@ -111,11 +106,6 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base,
 			.get_time = &ktime_get_real,
 		},
 		{
-			.index = HRTIMER_BASE_BOOTTIME_SOFT,
-			.clockid = CLOCK_BOOTTIME,
-			.get_time = &ktime_get_boottime,
-		},
-		{
 			.index = HRTIMER_BASE_TAI_SOFT,
 			.clockid = CLOCK_TAI,
 			.get_time = &ktime_get_clocktai,

@@ -129,7 +119,7 @@ static const int hrtimer_clock_to_base_t
 
 	[CLOCK_REALTIME]	= HRTIMER_BASE_REALTIME,
 	[CLOCK_MONOTONIC]	= HRTIMER_BASE_MONOTONIC,
-	[CLOCK_BOOTTIME]	= HRTIMER_BASE_BOOTTIME,
+	[CLOCK_BOOTTIME]	= HRTIMER_BASE_MONOTONIC,
 	[CLOCK_TAI]		= HRTIMER_BASE_TAI,
 };

@@ -565,14 +555,12 @@ static ktime_t
 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;
 	ktime_t *offs_tai = &base->clock_base[HRTIMER_BASE_TAI].offset;
 
 	ktime_t now = ktime_get_update_offsets_now(&base->clock_was_set_seq,
-					    offs_real, offs_boot, offs_tai);
+						   offs_real, offs_tai);
 
 	base->clock_base[HRTIMER_BASE_REALTIME_SOFT].offset = *offs_real;
-	base->clock_base[HRTIMER_BASE_BOOTTIME_SOFT].offset = *offs_boot;
 	base->clock_base[HRTIMER_BASE_TAI_SOFT].offset = *offs_tai;
 
 	return now;

kernel/time/timekeeping.c +1 -3

@@ -2242,7 +2242,6 @@ void do_timer(unsigned long ticks)
  * ktime_get_update_offsets_now - hrtimer helper
  * @cwsseq:	pointer to check and store the clock was set sequence number
  * @offs_real:	pointer to storage for monotonic -> realtime offset
- * @offs_boot:	pointer to storage for monotonic -> boottime offset
  * @offs_tai:	pointer to storage for monotonic -> clock tai offset
  *
  * Returns current monotonic time and updates the offsets if the

@@ -2252,7 +2251,7 @@ void do_timer(unsigned long ticks)
  * Called from hrtimer_interrupt() or retrigger_next_event()
  */
 ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,
-				     ktime_t *offs_boot, ktime_t *offs_tai)
+				     ktime_t *offs_tai)
 {
 	struct timekeeper *tk = &tk_core.timekeeper;
 	unsigned int seq;

@@ -2269,7 +2268,6 @@ ktime_t ktime_get_update_offsets_now(uns
 		if (*cwsseq != tk->clock_was_set_seq) {
 			*cwsseq = tk->clock_was_set_seq;
 			*offs_real = tk->offs_real;
-			*offs_boot = tk->offs_boot;
 			*offs_tai = tk->offs_tai;
 		}

kernel/time/timekeeping.h +0 -1

@@ -6,7 +6,6 @@
  */
 extern ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq,
 					    ktime_t *offs_real,
-					    ktime_t *offs_boot,
 					    ktime_t *offs_tai);
 
 extern int timekeeping_valid_for_hres(void);

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:09 UTC #12 lore raw

[tip:timers/core] hrtimer: Unify MONOTONIC and BOOTTIME clock behavior

hrtimer: Unify MONOTONIC and BOOTTIME clock behavior

Commit-ID:  127bfa5f4342e63d83a0b07ece376c2e8878e4a5
Gitweb:     https://git.kernel.org/tip/127bfa5f4342e63d83a0b07ece376c2e8878e4a5
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:37 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:23 +0100

hrtimer: Unify MONOTONIC and BOOTTIME clock behavior

Now that th MONOTONIC and BOOTTIME clocks are indentical remove all the special casing.

The user space visible interfaces still support both clocks, but their behavior is identical.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/hrtimer.h   |  2 --
 kernel/time/hrtimer.c     | 16 ++--------------
 kernel/time/timekeeping.c |  4 +---
 kernel/time/timekeeping.h |  1 -
 4 files changed, 3 insertions(+), 20 deletions(-)

include/linux/hrtimer.h +0 -2

index c7902ca7c9f4..78f456fcd242 100644
@@ -161,11 +161,9 @@ struct hrtimer_clock_base {
 enum  hrtimer_base_type {
 	HRTIMER_BASE_MONOTONIC,
 	HRTIMER_BASE_REALTIME,
-	HRTIMER_BASE_BOOTTIME,
 	HRTIMER_BASE_TAI,
 	HRTIMER_BASE_MONOTONIC_SOFT,
 	HRTIMER_BASE_REALTIME_SOFT,
-	HRTIMER_BASE_BOOTTIME_SOFT,
 	HRTIMER_BASE_TAI_SOFT,
 	HRTIMER_MAX_CLOCK_BASES,
 };

kernel/time/hrtimer.c +2 ~~-14~~

index 23788100e214..9b082ce86325 100644
@@ -90,11 +90,6 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
 			.clockid = CLOCK_REALTIME,
 			.get_time = &ktime_get_real,
 		},
-		{
-			.index = HRTIMER_BASE_BOOTTIME,
-			.clockid = CLOCK_BOOTTIME,
-			.get_time = &ktime_get_boottime,
-		},
 		{
 			.index = HRTIMER_BASE_TAI,
 			.clockid = CLOCK_TAI,

@@ -110,11 +105,6 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
 			.clockid = CLOCK_REALTIME,
 			.get_time = &ktime_get_real,
 		},
-		{
-			.index = HRTIMER_BASE_BOOTTIME_SOFT,
-			.clockid = CLOCK_BOOTTIME,
-			.get_time = &ktime_get_boottime,
-		},
 		{
 			.index = HRTIMER_BASE_TAI_SOFT,
 			.clockid = CLOCK_TAI,

@@ -129,7 +119,7 @@ static const int hrtimer_clock_to_base_table[MAX_CLOCKS] = {
 
 	[CLOCK_REALTIME]	= HRTIMER_BASE_REALTIME,
 	[CLOCK_MONOTONIC]	= HRTIMER_BASE_MONOTONIC,
-	[CLOCK_BOOTTIME]	= HRTIMER_BASE_BOOTTIME,
+	[CLOCK_BOOTTIME]	= HRTIMER_BASE_MONOTONIC,
 	[CLOCK_TAI]		= HRTIMER_BASE_TAI,
 };

@@ -565,14 +555,12 @@ __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base, unsigned int active_
 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;
 	ktime_t *offs_tai = &base->clock_base[HRTIMER_BASE_TAI].offset;
 
 	ktime_t now = ktime_get_update_offsets_now(&base->clock_was_set_seq,
-					    offs_real, offs_boot, offs_tai);
+						   offs_real, offs_tai);
 
 	base->clock_base[HRTIMER_BASE_REALTIME_SOFT].offset = *offs_real;
-	base->clock_base[HRTIMER_BASE_BOOTTIME_SOFT].offset = *offs_boot;
 	base->clock_base[HRTIMER_BASE_TAI_SOFT].offset = *offs_tai;
 
 	return now;

kernel/time/timekeeping.c +1 -3

index 8355c8803282..ca90219a1e73 100644
@@ -2195,7 +2195,6 @@ void do_timer(unsigned long ticks)
  * ktime_get_update_offsets_now - hrtimer helper
  * @cwsseq:	pointer to check and store the clock was set sequence number
  * @offs_real:	pointer to storage for monotonic -> realtime offset
- * @offs_boot:	pointer to storage for monotonic -> boottime offset
  * @offs_tai:	pointer to storage for monotonic -> clock tai offset
  *
  * Returns current monotonic time and updates the offsets if the

@@ -2205,7 +2204,7 @@ void do_timer(unsigned long ticks)
  * Called from hrtimer_interrupt() or retrigger_next_event()
  */
 ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,
-				     ktime_t *offs_boot, ktime_t *offs_tai)
+				     ktime_t *offs_tai)
 {
 	struct timekeeper *tk = &tk_core.timekeeper;
 	unsigned int seq;

@@ -2222,7 +2221,6 @@ ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,
 		if (*cwsseq != tk->clock_was_set_seq) {
 			*cwsseq = tk->clock_was_set_seq;
 			*offs_real = tk->offs_real;
-			*offs_boot = tk->offs_boot;
 			*offs_tai = tk->offs_tai;
 		}

kernel/time/timekeeping.h +0 -1

index 7a9b4eb7a1d5..79b67f5e0343 100644
@@ -6,7 +6,6 @@
  */
 extern ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq,
 					    ktime_t *offs_real,
-					    ktime_t *offs_boot,
 					    ktime_t *offs_tai);
 
 extern int timekeeping_valid_for_hres(void);

Thomas Gleixner

author

2018-03-01 16:57 UTC #13 lore raw

PATCH 7/7 tracing: Conflate boot and monotonic clock

In reply to Thomas Gleixner

Conflate boot and mono trace clocks and document the new behaviour.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Steven Rostedt <[email protected]>

---
 Documentation/trace/ftrace.txt |   14 +++-----------
 include/linux/timekeeping.h    |    6 ------
 kernel/trace/trace.c           |    2 +-
 3 files changed, 4 insertions(+), 18 deletions(-)

Documentation/trace/ftrace.txt +3 ~~-11~~

@@ -449,17 +449,9 @@ After mounting tracefs you will have acc
 		which is montonic but is not subject to any rate adjustments
 		and ticks at the same rate as the hardware clocksource.
 
-	  boot: This is the boot clock (CLOCK_BOOTTIME) and is based on the
-		fast monotonic clock, but also accounts for time spent in
-		suspend. Since the clock access is designed for use in
-		tracing in the suspend path, some side effects are possible
-		if clock is accessed after the suspend time is accounted before
-		the fast mono clock is updated. In this case, the clock update
-		appears to happen slightly sooner than it normally would have.
-		Also on 32-bit systems, it's possible that the 64-bit boot offset
-		sees a partial update. These effects are rare and post
-		processing should be able to handle them. See comments in the
-		ktime_get_boot_fast_ns() function for more information.
+	  boot: Same as mono. Used to be a separate clock which accounted
+	  	for the time spent in suspend while CLOCK_MONOTONIC did
+		not.
 
 	To set a clock, simply echo the clock name into this file.

include/linux/timekeeping.h +0 -6

@@ -113,12 +113,6 @@ extern u64 ktime_get_mono_fast_ns(void);
 extern u64 ktime_get_raw_fast_ns(void);
 extern u64 ktime_get_real_fast_ns(void);
 
-/* Clock BOOTTIME compatibility wrappers */
-static inline u64 ktime_get_boot_fast_ns(void)
-{
-	return ktime_get_mono_fast_ns();
-}
-
 /*
  * timespec64 interfaces utilizing the ktime based ones
  */

kernel/trace/trace.c +1 -1

@@ -1164,7 +1164,7 @@ static struct {
 	{ trace_clock,			"perf",		1 },
 	{ ktime_get_mono_fast_ns,	"mono",		1 },
 	{ ktime_get_raw_fast_ns,	"mono_raw",	1 },
-	{ ktime_get_boot_fast_ns,	"boot",		1 },
+	{ ktime_get_mono_fast_ns,	"boot",		1 },
 	ARCH_TRACE_CLOCKS
 };

tip-bot for Thomas Gleixner

merge bot

2018-03-13 07:10 UTC #14 lore raw

[tip:timers/core] tracing: Unify the "boot" and "mono" tracing clocks

tracing: Unify the "boot" and "mono" tracing clocks

Commit-ID:  92af4dcb4e1c5f58dc337bc97bdffd4e853dbc93
Gitweb:     https://git.kernel.org/tip/92af4dcb4e1c5f58dc337bc97bdffd4e853dbc93
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Thu, 1 Mar 2018 17:33:38 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Mar 2018 07:34:23 +0100

tracing: Unify the "boot" and "mono" tracing clocks

Unify the "boot" and "mono" tracing clocks and document the new behaviour.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc John Stultz <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Kevin Easton <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mark Salyzyn <[email protected]>

Cc Michael Kerrisk <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Petr Mladek <[email protected]>

Cc Prarit Bhargava <[email protected]>

Cc Sergey Senozhatsky <[email protected]>

Cc Steven Rostedt <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 Documentation/trace/ftrace.txt | 14 +++-----------
 include/linux/timekeeping.h    |  6 ------
 kernel/trace/trace.c           |  2 +-
 3 files changed, 4 insertions(+), 18 deletions(-)

Documentation/trace/ftrace.txt +3 ~~-11~~

index d4601df6e72e..bf89f98bfdb9 100644
@@ -449,17 +449,9 @@ of ftrace. Here is a list of some of the key files:
 		which is montonic but is not subject to any rate adjustments
 		and ticks at the same rate as the hardware clocksource.
 
-	  boot: This is the boot clock (CLOCK_BOOTTIME) and is based on the
-		fast monotonic clock, but also accounts for time spent in
-		suspend. Since the clock access is designed for use in
-		tracing in the suspend path, some side effects are possible
-		if clock is accessed after the suspend time is accounted before
-		the fast mono clock is updated. In this case, the clock update
-		appears to happen slightly sooner than it normally would have.
-		Also on 32-bit systems, it's possible that the 64-bit boot offset
-		sees a partial update. These effects are rare and post
-		processing should be able to handle them. See comments in the
-		ktime_get_boot_fast_ns() function for more information.
+	  boot: Same as mono. Used to be a separate clock which accounted
+	  	for the time spent in suspend while CLOCK_MONOTONIC did
+		not.
 
 	To set a clock, simply echo the clock name into this file.

include/linux/timekeeping.h +0 -6

index abb396731332..82c219dfd3bb 100644
@@ -113,12 +113,6 @@ extern u64 ktime_get_mono_fast_ns(void);
 extern u64 ktime_get_raw_fast_ns(void);
 extern u64 ktime_get_real_fast_ns(void);
 
-/* Clock BOOTTIME compatibility wrappers */
-static inline u64 ktime_get_boot_fast_ns(void)
-{
-	return ktime_get_mono_fast_ns();
-}
-
 /*
  * timespec64 interfaces utilizing the ktime based ones
  */

kernel/trace/trace.c +1 -1

index 20a2300ae4e8..300f4ea39646 100644
@@ -1164,7 +1164,7 @@ static struct {
 	{ trace_clock,			"perf",		1 },
 	{ ktime_get_mono_fast_ns,	"mono",		1 },
 	{ ktime_get_raw_fast_ns,	"mono_raw",	1 },
-	{ ktime_get_boot_fast_ns,	"boot",		1 },
+	{ ktime_get_mono_fast_ns,	"boot",		1 },
 	ARCH_TRACE_CLOCKS
 };

Linus Torvalds

2018-03-01 17:23 UTC #15 lore raw

In reply to Thomas Gleixner

Quoted 3 lines · Thomas Gleixner

This really needs lot of testing, documentation updates and more input from userspace folks to make a final decision.

Honestly, I don't think we'd get the testing this kind of change needs except by just trying it.

I'm willing to merge this in the 4.17 merge window, with the understanding that if people end up reporting issues, we may just have to revert it all, and chalk it up to a learning experience - and add the appropriate commentary in the kernel code about exactly what it was that depended on that MONO/BOOT difference.

One non-technical thing I would ask: use some other word than "conflate". Maybe just "combine". Or better yet, "unify".

"Conflate" technically and historically means the same thing as combine, but has very much gathered a side meaning of "confuse".

So yes, "conflate" is indeed about mixing or combining, but it's typically used in the sense of a *bad* combination or mixing. So "trying to conflate two issues" means "trying to mix two issues that are not the same into one".

So "unify" and "conflate" mean both the same thing and almost exactly the opposite at the same time.

And yes, you will find dictionaries (and linguists) that hold purely to the old meaning. As always, there are fogeys that can't get over the fact that meanings meander and change.

Linus

Thomas Gleixner

author

2018-03-01 18:41 UTC #16 lore raw

Quoted 5 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner
This really needs lot of testing, documentation updates and more input from userspace folks to make a final decision.

Quoted 8 lines

Honestly, I don't think we'd get the testing this kind of change needs except by just trying it.
I'm willing to merge this in the 4.17 merge window, with the understanding that if people end up reporting issues, we may just have to revert it all, and chalk it up to a learning experience - and add the appropriate commentary in the kernel code about exactly what it was that depended on that MONO/BOOT difference.

Fair enough. So we maybe just merge the first two patches and merge the cleanups and consolidation patches when we feel good enough.

I surely can queue the whole lot in next, but from PTI the experience I know how good the test coverage is. 4.14.stable would be the ideal testing ground. /me runs fast and hides

Quoted 11 lines

One non-technical thing I would ask: use some other word than "conflate". Maybe just "combine". Or better yet, "unify".
"Conflate" technically and historically means the same thing as combine, but has very much gathered a side meaning of "confuse".
So yes, "conflate" is indeed about mixing or combining, but it's typically used in the sense of a *bad* combination or mixing. So "trying to conflate two issues" means "trying to mix two issues that are not the same into one".

Quoted 6 lines

So "unify" and "conflate" mean both the same thing and almost exactly the opposite at the same time.
And yes, you will find dictionaries (and linguists) that hold purely to the old meaning. As always, there are fogeys that can't get over the fact that meanings meander and change.

I'm old enough to have learned that conflate means unify or combine, but I'm still not old enough to be stubborn about it :)

Thanks,

tglx

Steven Rostedt

maintainer

2018-03-01 18:50 UTC #17 lore raw

Quoted 2 lines · On Thu, 1 Mar 2018 19:41:35 +0100 (CET) Thomas Gleixner <[email protected]>…

I'm old enough to have learned that conflate means unify or combine, but I'm still not old enough to be stubborn about it :)

You need to watch more American Cable News channels to know what "conflate" means today.

-- Steve

Thomas Gleixner

author

2018-03-01 19:10 UTC #18 lore raw

In reply to Thomas Gleixner

Quoted 15 lines · Thomas Gleixner

Quoted 5 lines · Linus Torvalds
Quoted 3 lines · Thomas Gleixner
This really needs lot of testing, documentation updates and more input from userspace folks to make a final decision.
Quoted 8 lines
Honestly, I don't think we'd get the testing this kind of change needs except by just trying it.
I'm willing to merge this in the 4.17 merge window, with the understanding that if people end up reporting issues, we may just have to revert it all, and chalk it up to a learning experience - and add the appropriate commentary in the kernel code about exactly what it was that depended on that MONO/BOOT difference.

Quoted 6 lines

Fair enough. So we maybe just merge the first two patches and merge the cleanups and consolidation patches when we feel good enough.
I surely can queue the whole lot in next, but from PTI the experience I know how good the test coverage is. 4.14.stable would be the ideal testing ground. /me runs fast and hides

That said, at least the people who are asking for that should provide testing results _before_ this gets applied or merged upstream.

Thanks,

tglx

Ingo Molnar

maintainer

2018-03-13 06:36 UTC #19 lore raw

In reply to Linus Torvalds

Quoted 8 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner
This really needs lot of testing, documentation updates and more input from userspace folks to make a final decision.
Honestly, I don't think we'd get the testing this kind of change needs except by just trying it.

Quoted 8 lines

I'm willing to merge this in the 4.17 merge window, with the understanding that if people end up reporting issues, we may just have to revert it all, and chalk it up to a learning experience - and add the appropriate commentary in the kernel code about exactly what it was that depended on that MONO/BOOT difference.
One non-technical thing I would ask: use some other word than "conflate". Maybe just "combine". Or better yet, "unify".

Ok, I have edited all the changelogs accordingly (and also flipped around the 'clock MONOTONIC' language to the more readable 'the MONOTONIC clock' variant), the resulting titles are (in order):

72199320d49d: timekeeping: Add the new CLOCK_MONOTONIC_ACTIVE clock
d6ed449afdb3: timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock
f2d6fdbfd238: Input: Evdev - unify MONOTONIC and BOOTTIME clock behavior
d6c7270e913d: timekeeping: Remove boot time specific code 7250a4047aa6: posix-timers: Unify MONOTONIC and BOOTTIME clock behavior 127bfa5f4342: hrtimer: Unify MONOTONIC and BOOTTIME clock behavior 92af4dcb4e1c: tracing: Unify the "boot" and "mono" tracing clocks

I'll push these out after testing.

Thanks,

Ingo

John Stultz

2018-03-13 18:11 UTC #20 lore raw

Quoted 32 lines · Ingo Molnar

Quoted 8 lines · Linus Torvalds
Quoted 3 lines · Thomas Gleixner
This really needs lot of testing, documentation updates and more input from userspace folks to make a final decision.
Honestly, I don't think we'd get the testing this kind of change needs except by just trying it.
Quoted 8 lines
I'm willing to merge this in the 4.17 merge window, with the understanding that if people end up reporting issues, we may just have to revert it all, and chalk it up to a learning experience - and add the appropriate commentary in the kernel code about exactly what it was that depended on that MONO/BOOT difference.
One non-technical thing I would ask: use some other word than "conflate". Maybe just "combine". Or better yet, "unify".
Ok, I have edited all the changelogs accordingly (and also flipped around the 'clock MONOTONIC' language to the more readable 'the MONOTONIC clock' variant), the resulting titles are (in order):
72199320d49d: timekeeping: Add the new CLOCK_MONOTONIC_ACTIVE clock
d6ed449afdb3: timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock
f2d6fdbfd238: Input: Evdev - unify MONOTONIC and BOOTTIME clock behavior
d6c7270e913d: timekeeping: Remove boot time specific code 7250a4047aa6: posix-timers: Unify MONOTONIC and BOOTTIME clock behavior 127bfa5f4342: hrtimer: Unify MONOTONIC and BOOTTIME clock behavior 92af4dcb4e1c: tracing: Unify the "boot" and "mono" tracing clocks

Quoted 1 line

I'll push these out after testing.

I'm still anxious about userspace effects given how much I've seen the current behavior documented, and wouldn't pushed for this myself (I'm a worrier), but at least I'm not seeing any failures in initial testing w/ kselftest so far.

thanks
-john

David Herrmann

2018-04-20 04:37 UTC #21 lore raw

Hey

Quoted 13 lines · John Stultz

Quoted 12 lines · Ingo Molnar
Ok, I have edited all the changelogs accordingly (and also flipped around the 'clock MONOTONIC' language to the more readable 'the MONOTONIC clock' variant), the resulting titles are (in order):
72199320d49d: timekeeping: Add the new CLOCK_MONOTONIC_ACTIVE clock
d6ed449afdb3: timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock
f2d6fdbfd238: Input: Evdev - unify MONOTONIC and BOOTTIME clock behavior
d6c7270e913d: timekeeping: Remove boot time specific code 7250a4047aa6: posix-timers: Unify MONOTONIC and BOOTTIME clock behavior 127bfa5f4342: hrtimer: Unify MONOTONIC and BOOTTIME clock behavior 92af4dcb4e1c: tracing: Unify the "boot" and "mono" tracing clocks

Quoted 6 lines

Quoted 1 line
I'll push these out after testing.
I'm still anxious about userspace effects given how much I've seen the current behavior documented, and wouldn't pushed for this myself (I'm a worrier), but at least I'm not seeing any failures in initial testing w/ kselftest so far.

I get lots of timer-errors on Arch-Linux booting current master, after a suspend/resume cycle. Just a selection of errors I see on resume:

systemd[1]: systemd-journald.service: Main process exited, code=dumped, status=6/ABRT
rtkit-daemon[742]: The canary thread is apparently starving. Taking action. systemd[1]: systemd-udevd.service: Watchdog timeout (limit 3min)! systemd[1]: systemd-journald.service: Watchdog timeout (limit 3min)!
kernel: e1000e 0000:00:1f.6: Failed to restore TIMINCA clock rate delta: -22

Lots of crashes with SIGABRT due to these.

I did not bisect it, but it sounds related to me. Also, user-space uses CLOCK_MONOTONIC for watchdog timers. That is, a process is required to respond to a watchdog-request in a given MONOTONIC time-frame. If this jumps during suspend/resume, watchdogs will fire immediately. I don't see how this can work with the new MONOTONIC behavior?

Thanks
David

Sergey Senozhatsky

2018-04-20 05:45 UTC #22 lore raw

Quoted 3 lines · David Herrmann

I get lots of timer-errors on Arch-Linux booting current master, after a suspend/resume cycle. Just a selection of errors I see on resume:

Hello David,
Any chance you can revert the patches in question and test? I'm running ARCH (4.17.0-rc1-dbg-00042-gaa03ddd9c434) and suspend/resume cycle does not trigger any errors. Except for this one

kernel: do_IRQ: 0.55 No irq handler for vector

Quoted 7 lines

systemd[1]: systemd-journald.service: Main process exited, code=dumped, status=6/ABRT
rtkit-daemon[742]: The canary thread is apparently starving. Taking action. systemd[1]: systemd-udevd.service: Watchdog timeout (limit 3min)! systemd[1]: systemd-journald.service: Watchdog timeout (limit 3min)!
kernel: e1000e 0000:00:1f.6: Failed to restore TIMINCA clock rate delta: -22

Quoted 8 lines

Lots of crashes with SIGABRT due to these.
I did not bisect it, but it sounds related to me. Also, user-space uses CLOCK_MONOTONIC for watchdog timers. That is, a process is required to respond to a watchdog-request in a given MONOTONIC time-frame. If this jumps during suspend/resume, watchdogs will fire immediately. I don't see how this can work with the new MONOTONIC behavior?

-ss

David Herrmann

2018-04-20 06:49 UTC #23 lore raw

Quoted 11 lines · Sergey Senozhatsky

Quoted 3 lines · David Herrmann
I get lots of timer-errors on Arch-Linux booting current master, after a suspend/resume cycle. Just a selection of errors I see on resume:
Hello David,
Any chance you can revert the patches in question and test? I'm running ARCH (4.17.0-rc1-dbg-00042-gaa03ddd9c434) and suspend/resume cycle does not trigger any errors. Except for this one
kernel: do_IRQ: 0.55 No irq handler for vector

I can easily reproduce it by sleeping for >5min, so the systemd watchdog timers are triggered. The patches don't revert cleanly, so I didn't look into booting without them, yet. I will try just linking the monotonic clock to the monotonic_active clock later.

Also, doesn't this hunk in 72199320d49d need a 'break;':

kernel/time/posix-stubs.c +0 -0

index b258bee13b02..6259dbc0191a 100644
@@ -73,6 +73,8 @@ int do_clock_gettime(clockid_t which_clock, struct

timespec64 *tp)

        case CLOCK_BOOTTIME:
                get_monotonic_boottime64(tp);
                break;
+       case CLOCK_MONOTONIC_ACTIVE:
+               ktime_get_active_ts64(tp);
        default:
                return -EINVAL;
        }

Genki Sky

2018-04-24 00:49 UTC #24 lore raw

Hello,

I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.

Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!

As a random end user, I see three options to get suspend/resume working again on my laptop:

(A) Change systemd to keep track of the difference between CLOCK_MONOTONIC and CLOCK_MONOTONIC_ACTIVE, using their difference via clock_gettime(). This seems to be what the author of this patch series intends (?).

(B) Implement timerfd_*(2) for CLOCK_MONOTONIC_ACTIVE in the kernel. Do a sed s/CLOCK_MONOTONIC/&_ACTIVE in systemd source code.

(C) Do a 90% reverting of this patch series. Just introduce CLOCK_MONOTONIC_ACTIVE as the "what you should use", document and publicize this fact, and sometime in the future (monotonically speaking :) finally unify CLOCK_MONOTONIC and CLOCK_BOOTTIME.

Thoughts?

Also, agreed, the missing break in do_clock_gettime() should be fixed, as David mentioned in [1]. Is someone already patching this?

[0]: http://lkml.kernel.org/r/CANq1E4Tf27FJHTrO4ZVtWhYK=DLmwzszK=njOpgXZZXqzAOunA@mail.gmail.com
[1]: http://lkml.kernel.org/r/CANq1E4QppGAaU5PYbGpuC00S6wGQcAt70Z7CntZHiLC6748z2A@mail.gmail.com

Genki

Genki Sky

2018-04-24 02:45 UTC #25 lore raw

Quoting Genki Sky (2018/04/23 20:40:36 -0400)

Quoted 9 lines

I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!

This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).

Let me know if I can help in any way.

John Stultz

2018-04-24 03:03 UTC #26 lore raw

Quoted 11 lines · Genki Sky

Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 9 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!

Quoted 6 lines

This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).
Let me know if I can help in any way.

Yea, this is the sort of thing I was worried about.

Thomas: I think reverting this change is needed.

thanks
-john

Thomas Gleixner

author

2018-04-24 08:09 UTC #27 lore raw

Quoted 17 lines · John Stultz

Quoted 16 lines · Genki Sky
Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 9 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!
This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).

Quoted 5 lines

Quoted 1 line
Let me know if I can help in any way.
Yea, this is the sort of thing I was worried about.
Thomas: I think reverting this change is needed.

Sigh. I hoped that something like this would be catched before I sent the pull request by those who were actually interested in this change...

I'll try to distangle it.

Thanks,

tglx

Genki Sky

2018-04-24 12:17 UTC #28 lore raw

Sorry to have been the bearer of bad news :(. Again, I just have my user hat on here. It does seem like this unifying would have been nice to have. And even, more compliant with the POSIX definition of MONOTONIC...

On that note, maybe it is still worth introducing MONOTONIC_ACTIVE, but just as an alias for MONOTONIC for now. It's also more self-documenting. Then sometime in the future, if people switch over, remove BOOTTIME and make MONOTONIC like BOOTTIME. Though this doesn't help simplify the code, I know.

Thanks again,
Genki

Thomas Gleixner

author

2018-04-24 15:01 UTC #29 lore raw

Quoted 1 line · Genki Sky

Sorry to have been the bearer of bad news :(.

No problem. We're not shooting the messengers

Quoted 3 lines

Again, I just have my user hat on here. It does seem like this unifying would have been nice to have. And even, more compliant with the POSIX definition of MONOTONIC...

Yes, that was the idea

Quoted 5 lines

On that note, maybe it is still worth introducing MONOTONIC_ACTIVE, but just as an alias for MONOTONIC for now. It's also more self-documenting. Then sometime in the future, if people switch over, remove BOOTTIME and make MONOTONIC like BOOTTIME. Though this doesn't help simplify the code, I know.

It doesn't and it does not make applications magically make use of MONOTONIC_ACTIVE. We're in a trap here.....

Thanks,

tglx

Pavel Machek

2018-04-25 06:50 UTC #30 lore raw

In reply to Thomas Gleixner

Quoted 19 lines · Thomas Gleixner

Quoted 17 lines · John Stultz
Quoted 16 lines · Genki Sky
Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 9 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!
This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Let me know if I can help in any way.
Yea, this is the sort of thing I was worried about.
Thomas: I think reverting this change is needed.
Sigh. I hoped that something like this would be catched before I sent the pull request by those who were actually interested in this change...

Well, we had two regressions in -next this cycle... I reported both but bisections were not easy and noone was really interested.

See

Re: linux-next on x60: network manager often complains "network is
disabled" after resume

Subject: Re: CLOCK_MONOTONIC, BOOTTIME, suspend and screensaver regression

Can we expect this to be sorted out in v4.17-rc3? -next?
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Rafael J. Wysocki

2018-04-25 08:55 UTC #31 lore raw

Quoted 32 lines · CEST Pavel Machek

--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Quoted 12 lines · Thomas Gleixner
On Mon, 23 Apr 2018, John Stultz wrote:
=20
Quoted 10 lines · Genki Sky
Quoted 9 lines
Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 8 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you=
'd
Quoted 12 lines
Quoted 12 lines
Quoted 8 lines
Quoted 1 line
hope it *would* break it!
This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).
Let me know if I can help in any way.
=20
Yea, this is the sort of thing I was worried about. =20
Thomas: I think reverting this change is needed.

Quoted 6 lines

Quoted 3 lines
=20
Sigh. I hoped that something like this would be catched before I sent the pull request by those who were actually interested in this change...
Well, we had two regressions in -next this cycle... I reported both but bisections were not easy and noone was really interested.

It's more that it was difficult to correlate the reported symptoms with a particular set of kernel changes.

Rafael J. Wysocki

2018-04-25 08:52 UTC #32 lore raw

In reply to Thomas Gleixner

Quoted 19 lines · CEST Thomas Gleixner

Quoted 17 lines · John Stultz
Quoted 16 lines · Genki Sky
Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 9 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!
This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Let me know if I can help in any way.
Yea, this is the sort of thing I was worried about.
Thomas: I think reverting this change is needed.
Sigh. I hoped that something like this would be catched before I sent the pull request by those who were actually interested in this change...

The "git revert -m 1 680014d6d1da" makes resume issues on my Aspire S5 go away (cf. https://marc.info/?l=linux-kernel&m=152460804018920&w=2).

I'll try with just the "monotonic" vs "boottime" clock changes reverted.

Quoted 1 line

I'll try to distangle it.

Cool.

Please let me know if you need any help.

Rafael J. Wysocki

2018-04-25 09:49 UTC #33 lore raw

Quoted 26 lines · CEST Rafael J. Wysocki

Quoted 25 lines · CEST Thomas Gleixner
Quoted 22 lines · John Stultz
Quoted 11 lines · Genki Sky
Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 9 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!
Quoted 6 lines
This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).
Let me know if I can help in any way.
Yea, this is the sort of thing I was worried about.
Thomas: I think reverting this change is needed.

Quoted 7 lines

Quoted 2 lines
Sigh. I hoped that something like this would be catched before I sent the pull request by those who were actually interested in this change...
The "git revert -m 1 680014d6d1da" makes resume issues on my Aspire S5 go away (cf. https://marc.info/?l=linux-kernel&m=152460804018920&w=2).
I'll try with just the "monotonic" vs "boottime" clock changes reverted.

FWICS (so far) system resume still works here with the commits between d6ed449afdb3 (timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock) and 127bfa5f4342 (hrtimer: Unify MONOTONIC and BOOTTIME clock behavior) inclusive reverted on top of 4.17-rc2. [I probably should revert commit 92af4dcb4e1c (tracing: Unify the "boot" and "mono" tracing clocks) too, but it doesn't revert cleanly and it doesn't apprear to affect things here too.]

Thomas Gleixner

author

2018-04-25 13:03 UTC #34 lore raw

Quoted 33 lines · Rafael J. Wysocki

Quoted 32 lines · CEST Rafael J. Wysocki
Quoted 19 lines · CEST Thomas Gleixner
Quoted 17 lines · John Stultz
Quoted 16 lines · Genki Sky
Quoting Genki Sky (2018/04/23 20:40:36 -0400)
Quoted 9 lines
I came across this thread for same reason as [0]: Daemons getting killed by systemd on resume (after >WatchdogSec seconds of suspending). I'm using master branch of systemd and the kernel. As mentioned, systemd uses CLOCK_MONOTONIC, originally expecting it to not include suspend time.
Correct me if I'm mistaken, but I don't see the ambiguity of whether this patch series breaks systemd. If it's implemented correctly, you'd hope it *would* break it!
This sounded a little weak on re-reading, sorry. So, I just confirmed that after booting a "git revert -m 1 680014d6d1da", the issue no longer appears. (I.e., a suspend for >WatchDog sec doesn't result in any daemon getting killed).
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Let me know if I can help in any way.
Yea, this is the sort of thing I was worried about.
Thomas: I think reverting this change is needed.
Sigh. I hoped that something like this would be catched before I sent the pull request by those who were actually interested in this change...
The "git revert -m 1 680014d6d1da" makes resume issues on my Aspire S5 go away (cf. https://marc.info/?l=linux-kernel&m=152460804018920&w=2).

Quoted 8 lines

Quoted 1 line
I'll try with just the "monotonic" vs "boottime" clock changes reverted.
FWICS (so far) system resume still works here with the commits between d6ed449afdb3 (timekeeping: Make the MONOTONIC clock behave like the BOOTTIME clock) and 127bfa5f4342 (hrtimer: Unify MONOTONIC and BOOTTIME clock behavior) inclusive reverted on top of 4.17-rc2. [I probably should revert commit 92af4dcb4e1c (tracing: Unify the "boot" and "mono" tracing clocks) too, but it doesn't revert cleanly and it doesn't apprear to affect things here too.]

Right, it does not matter. The real interesting one is d6ed449afdb3.

Thanks,

tglx

Mike Galbraith

2018-04-26 07:03 UTC #35 lore raw

Quoted 1 line · Thomas Gleixner

Right, it does not matter. The real interesting one is d6ed449afdb3.

FWIW, three boxen here suspend/resume fine, but repeatably exhibit the below after a very few minute suspend, and a short bisect fingered your suspect. Distro is opensuse 42.3.

[  211.113902] Restarting tasks ... done.
[  211.114817] PM: suspend exit
[  212.312993] systemd-journald[7266]: File /var/log/journal/016627c3c4784cd4812d4b7e96a34226/system.journal corrupted or uncleanly shut down, renaming and replacing.
[  212.313363] systemd-coredump[7264]: Detected coredump of the journal daemon itself, diverted to /var/lib/systemd/coredump/core.systemd-journal.0.0aa39276decf4f1ab6fda3464e31f9dd.582.1524720954000000.

Thomas Gleixner

author

2018-04-26 07:42 UTC #36 lore raw

Quoted 12 lines · Mike Galbraith

Quoted 1 line · Thomas Gleixner
Right, it does not matter. The real interesting one is d6ed449afdb3.
FWIW, three boxen here suspend/resume fine, but repeatably exhibit the below after a very few minute suspend, and a short bisect fingered your suspect. Distro is opensuse 42.3.
[  211.113902] Restarting tasks ... done.
[  211.114817] PM: suspend exit
[  212.312993] systemd-journald[7266]: File /var/log/journal/016627c3c4784cd4812d4b7e96a34226/system.journal corrupted or uncleanly shut down, renaming and replacing.
[  212.313363] systemd-coredump[7264]: Detected coredump of the journal daemon itself, diverted to /var/lib/systemd/coredump/core.systemd-journal.0.0aa39276decf4f1ab6fda3464e31f9dd.582.1524720954000000.

Huch, that rather looks like a genuine application bug.

Thanks,

tglx

Rafael J. Wysocki

2018-04-26 08:36 UTC #37 lore raw

Quoted 8 lines · Thomas Gleixner

Quoted 7 lines · Mike Galbraith
Quoted 1 line · Thomas Gleixner
Right, it does not matter. The real interesting one is d6ed449afdb3.
FWIW, three boxen here suspend/resume fine, but repeatably exhibit the below after a very few minute suspend, and a short bisect fingered your suspect. Distro is opensuse 42.3.

Quoted 7 lines

Quoted 5 lines

[  211.113902] Restarting tasks ... done.
[  211.114817] PM: suspend exit
[  212.312993] systemd-journald[7266]: File /var/log/journal/016627c3c4784cd4812d4b7e96a34226/system.journal corrupted or uncleanly shut down, renaming and replacing.
[  212.313363] systemd-coredump[7264]: Detected coredump of the journal daemon itself, diverted to /var/lib/systemd/coredump/core.systemd-journal.0.0aa39276decf4f1ab6fda3464e31f9dd.582.1524720954000000.

Huch, that rather looks like a genuine application bug.

Well, say you set a timer to wake you up in X seconds. When you wake up, you look at a clock and see that Y seconds have passed and Y is much greater than X. I guess you'd think that something's wrong. :-)

Thomas Gleixner

author

2018-04-26 08:51 UTC #38 lore raw

Quoted 15 lines · Rafael J. Wysocki

Quoted 14 lines · Thomas Gleixner
Quoted 12 lines · Mike Galbraith
Quoted 1 line · Thomas Gleixner
Right, it does not matter. The real interesting one is d6ed449afdb3.
FWIW, three boxen here suspend/resume fine, but repeatably exhibit the below after a very few minute suspend, and a short bisect fingered your suspect. Distro is opensuse 42.3.
[  211.113902] Restarting tasks ... done.
[  211.114817] PM: suspend exit
[  212.312993] systemd-journald[7266]: File /var/log/journal/016627c3c4784cd4812d4b7e96a34226/system.journal corrupted or uncleanly shut down, renaming and replacing.
[  212.313363] systemd-coredump[7264]: Detected coredump of the journal daemon itself, diverted to /var/lib/systemd/coredump/core.systemd-journal.0.0aa39276decf4f1ab6fda3464e31f9dd.582.1524720954000000.

Quoted 5 lines

Quoted 1 line
Huch, that rather looks like a genuine application bug.
Well, say you set a timer to wake you up in X seconds. When you wake up, you look at a clock and see that Y seconds have passed and Y is much greater than X. I guess you'd think that something's wrong. :-)

And that makes you coredump, right? Brilliant choice.

Thanks,

tglx

Rafael J. Wysocki

2018-04-26 09:03 UTC #39 lore raw

Quoted 17 lines · Thomas Gleixner

Quoted 15 lines · Rafael J. Wysocki
Quoted 14 lines · Thomas Gleixner
Quoted 12 lines · Mike Galbraith
Quoted 1 line · Thomas Gleixner
Right, it does not matter. The real interesting one is d6ed449afdb3.
FWIW, three boxen here suspend/resume fine, but repeatably exhibit the below after a very few minute suspend, and a short bisect fingered your suspect. Distro is opensuse 42.3.
[  211.113902] Restarting tasks ... done.
[  211.114817] PM: suspend exit
[  212.312993] systemd-journald[7266]: File /var/log/journal/016627c3c4784cd4812d4b7e96a34226/system.journal corrupted or uncleanly shut down, renaming and replacing.
[  212.313363] systemd-coredump[7264]: Detected coredump of the journal daemon itself, diverted to /var/lib/systemd/coredump/core.systemd-journal.0.0aa39276decf4f1ab6fda3464e31f9dd.582.1524720954000000.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Huch, that rather looks like a genuine application bug.
Well, say you set a timer to wake you up in X seconds. When you wake up, you look at a clock and see that Y seconds have passed and Y is much greater than X. I guess you'd think that something's wrong. :-)
And that makes you coredump, right? Brilliant choice.

That wouldn't be my choice, but some people do make choices like that even in their personal lives ...