[patch 0/3] timekeeping: Janitorial updates

[patch 0/3] timekeeping: Janitorial updates

[Thomas Gleixner]

The following series provides a few minor fixlets and consolidation:

   - Add a missing data_race() annotation

   - Ensure that the clock readout is always inlined

   - Reduce the copy & pasta in the NMI safe accessors

Thanks,

	tglx
---
 timekeeping.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

[patch 1/3] timekeeping: Annotate ktime_get_boot_fast_ns() with data_race()

[Thomas Gleixner]

Accessing timekeeper::offset_boot in ktime_get_boot_fast_ns() is an
intended data race as the reader side cannot synchronize with a writer and
there is no space in struct tk_read_base of the NMI safe timekeeper.

Mark it so.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -528,7 +528,7 @@ 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));
+	return (ktime_get_mono_fast_ns() + ktime_to_ns(data_race(tk->offs_boot)));
 }
 EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);
 

Re: [patch 1/3] timekeeping: Annotate ktime_get_boot_fast_ns() with data_race()

[Peter Zijlstra]

On Fri, Apr 15, 2022 at 11:19:35AM +0200, Thomas Gleixner wrote:
> Accessing timekeeper::offset_boot in ktime_get_boot_fast_ns() is an
> intended data race as the reader side cannot synchronize with a writer and
> there is no space in struct tk_read_base of the NMI safe timekeeper.
> 
> Mark it so.

If offs_boot actually ever changed?

Re: [patch 1/3] timekeeping: Annotate ktime_get_boot_fast_ns() with data_race()

[Thomas Gleixner]

On Fri, Apr 15 2022 at 14:07, Peter Zijlstra wrote:
> On Fri, Apr 15, 2022 at 11:19:35AM +0200, Thomas Gleixner wrote:
>> Accessing timekeeper::offset_boot in ktime_get_boot_fast_ns() is an
>> intended data race as the reader side cannot synchronize with a writer and
>> there is no space in struct tk_read_base of the NMI safe timekeeper.
>> 
>> Mark it so.
>
> If offs_boot actually ever changed?

Yes, during resume to compensate for the time spent in suspend. So, yes
the data_race() is more of documentary value.

Thanks,

        tglx

[tip: timers/core] timekeeping: Annotate ktime_get_boot_fast_ns() with data_race()

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     eff4849f928f2b90402907e06a6de1619cf16b1a
Gitweb:        https://git.kernel.org/tip/eff4849f928f2b90402907e06a6de1619cf16b1a
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Fri, 15 Apr 2022 11:19:35 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 02 May 2022 14:00:20 +02:00

timekeeping: Annotate ktime_get_boot_fast_ns() with data_race()

Accessing timekeeper::offset_boot in ktime_get_boot_fast_ns() is an
intended data race as the reader side cannot synchronize with a writer and
there is no space in struct tk_read_base of the NMI safe timekeeper.

Mark it so.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20220415091920.956045162@linutronix.de

---
 kernel/time/timekeeping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 2c22023..3479804 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -528,7 +528,7 @@ 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));
+	return (ktime_get_mono_fast_ns() + ktime_to_ns(data_race(tk->offs_boot)));
 }
 EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);
 

[patch 2/3] timekeeping: Mark tk_clock_read() __always_inline

[Thomas Gleixner]

Compilers can uninline this which makes the notrace annotation of the NMI
safe accessors moot.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -186,7 +186,7 @@ static inline void tk_update_sleep_time(
  * a read of the fast-timekeeper tkrs (which is protected by its own locking
  * and update logic).
  */
-static inline u64 tk_clock_read(const struct tk_read_base *tkr)
+static __always_inline u64 tk_clock_read(const struct tk_read_base *tkr)
 {
 	struct clocksource *clock = READ_ONCE(tkr->clock);
 

Re: [patch 2/3] timekeeping: Mark tk_clock_read() __always_inline

[Peter Zijlstra]

On Fri, Apr 15, 2022 at 11:19:36AM +0200, Thomas Gleixner wrote:
> Compilers can uninline this which makes the notrace annotation of the NMI
> safe accessors moot.

inline already implies notrace.

No objection to making it __always_inline, but this reason doesn't
really work.

> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/time/timekeeping.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -186,7 +186,7 @@ static inline void tk_update_sleep_time(
>   * a read of the fast-timekeeper tkrs (which is protected by its own locking
>   * and update logic).
>   */
> -static inline u64 tk_clock_read(const struct tk_read_base *tkr)
> +static __always_inline u64 tk_clock_read(const struct tk_read_base *tkr)
>  {
>  	struct clocksource *clock = READ_ONCE(tkr->clock);
>  
> 

Re: [patch 2/3] timekeeping: Mark tk_clock_read() __always_inline

[Thomas Gleixner]

On Fri, Apr 15 2022 at 14:09, Peter Zijlstra wrote:

> On Fri, Apr 15, 2022 at 11:19:36AM +0200, Thomas Gleixner wrote:
>> Compilers can uninline this which makes the notrace annotation of the NMI
>> safe accessors moot.
>
> inline already implies notrace.

Bah. Confused myself vs. noinstr. We have too many constraints...

> No objection to making it __always_inline, but this reason doesn't
> really work.

Let me come up with a more coherent argument.

Thanks,

        tglx

[patch 3/3] timekeeping: Consolidate fast timekeeper

[Thomas Gleixner]

Provide a inline function which replaces the copy & pasta.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -429,6 +429,14 @@ static void update_fast_timekeeper(const
 	memcpy(base + 1, base, sizeof(*base));
 }
 
+static __always_inline u64 fast_tk_get_delta_ns(struct tk_read_base *tkr)
+{
+	u64 delta, cycles = tk_clock_read(tkr);
+
+	delta = clocksource_delta(cycles, tkr->cycle_last, tkr->mask);
+	return timekeeping_delta_to_ns(tkr, delta);
+}
+
 static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf)
 {
 	struct tk_read_base *tkr;
@@ -439,12 +447,7 @@ static __always_inline u64 __ktime_get_f
 		seq = raw_read_seqcount_latch(&tkf->seq);
 		tkr = tkf->base + (seq & 0x01);
 		now = ktime_to_ns(tkr->base);
-
-		now += timekeeping_delta_to_ns(tkr,
-				clocksource_delta(
-					tk_clock_read(tkr),
-					tkr->cycle_last,
-					tkr->mask));
+		now += fast_tk_get_delta_ns(tkr);
 	} while (read_seqcount_latch_retry(&tkf->seq, seq));
 
 	return now;
@@ -560,10 +563,7 @@ static __always_inline u64 __ktime_get_r
 		tkr = tkf->base + (seq & 0x01);
 		basem = ktime_to_ns(tkr->base);
 		baser = ktime_to_ns(tkr->base_real);
-
-		delta = timekeeping_delta_to_ns(tkr,
-				clocksource_delta(tk_clock_read(tkr),
-				tkr->cycle_last, tkr->mask));
+		delta = fast_tk_get_delta_ns(tkr);
 	} while (read_seqcount_latch_retry(&tkf->seq, seq));
 
 	if (mono)

Re: [patch 3/3] timekeeping: Consolidate fast timekeeper

[Peter Zijlstra]

On Fri, Apr 15, 2022 at 11:19:38AM +0200, Thomas Gleixner wrote:
> Provide a inline function which replaces the copy & pasta.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[tip: timers/core] timekeeping: Consolidate fast timekeeper

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     90be8d6c1f91e1e5121c219726524c91b52bfc20
Gitweb:        https://git.kernel.org/tip/90be8d6c1f91e1e5121c219726524c91b52bfc20
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Fri, 15 Apr 2022 11:19:38 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 02 May 2022 14:00:20 +02:00

timekeeping: Consolidate fast timekeeper

Provide a inline function which replaces the copy & pasta.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220415091921.072296632@linutronix.de

---
 kernel/time/timekeeping.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 3479804..8895ff2 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -429,6 +429,14 @@ static void update_fast_timekeeper(const struct tk_read_base *tkr,
 	memcpy(base + 1, base, sizeof(*base));
 }
 
+static __always_inline u64 fast_tk_get_delta_ns(struct tk_read_base *tkr)
+{
+	u64 delta, cycles = tk_clock_read(tkr);
+
+	delta = clocksource_delta(cycles, tkr->cycle_last, tkr->mask);
+	return timekeeping_delta_to_ns(tkr, delta);
+}
+
 static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf)
 {
 	struct tk_read_base *tkr;
@@ -439,12 +447,7 @@ static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf)
 		seq = raw_read_seqcount_latch(&tkf->seq);
 		tkr = tkf->base + (seq & 0x01);
 		now = ktime_to_ns(tkr->base);
-
-		now += timekeeping_delta_to_ns(tkr,
-				clocksource_delta(
-					tk_clock_read(tkr),
-					tkr->cycle_last,
-					tkr->mask));
+		now += fast_tk_get_delta_ns(tkr);
 	} while (read_seqcount_latch_retry(&tkf->seq, seq));
 
 	return now;
@@ -560,10 +563,7 @@ static __always_inline u64 __ktime_get_real_fast(struct tk_fast *tkf, u64 *mono)
 		tkr = tkf->base + (seq & 0x01);
 		basem = ktime_to_ns(tkr->base);
 		baser = ktime_to_ns(tkr->base_real);
-
-		delta = timekeeping_delta_to_ns(tkr,
-				clocksource_delta(tk_clock_read(tkr),
-				tkr->cycle_last, tkr->mask));
+		delta = fast_tk_get_delta_ns(tkr);
 	} while (read_seqcount_latch_retry(&tkf->seq, seq));
 
 	if (mono)