The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] hrtimers: skip cpu_base->lock in hrtimer_get_next_event() when hres_active
@ 2026-07-07 15:38 Usama Arif
  2026-07-07 21:32 ` [tip: timers/core] hrtimer: Don't take cpu_base::lock " tip-bot2 for Usama Arif
  0 siblings, 1 reply; 2+ messages in thread
From: Usama Arif @ 2026-07-07 15:38 UTC (permalink / raw)
  To: anna-maria, frederic, linux-kernel, tglx, d, shakeel.butt, hannes,
	riel, kernel-team
  Cc: Usama Arif

hrtimer_get_next_event() runs on every tick-stop decision via
get_next_timer_interrupt() -> cmp_next_hrtimer_event().  When high
resolution timers are active it must return KTIME_MAX -- the caller
documents and depends on this.

Yet the current body takes cpu_base->lock, checks hres_active, and
returns KTIME_MAX anyway.  On any modern kernel HRES is on continuously
after boot, so the lock acquisition is pure overhead on every
idle-decision path.

On a 176-thread AMD EPYC 9D64 running a Meta production workload, bucketing
callers of native_queued_spin_lock_slowpath by the _raw_spin_lock*, the
slowpath had 199 samples total in total, of which 54 are attributed to
hrtimer_get_next_event+0x2b (the guard() lock take), i.e. this lock take
accounts for ~27% of irqsave slowpath hits on this specific workload.

cpu_base->hres_active is only written by the local CPU
(hrtimer_switch_to_hres() from hardirq context, hrtimers_cpu_starting()
during bring-up).  All callers of hrtimer_get_next_event() reach it
from the tick-stop / cpuidle paths with interrupts disabled on that
CPU (__get_next_timer_interrupt() itself takes base_local->lock with
raw_spin_lock(), not raw_spin_lock_irqsave()).  No writer can therefore
race with the read, so an unlocked hres_active check is stable and the
lock can be skipped in the common case.

Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 kernel/time/hrtimer.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 313dcea127fe..5be2cb035b86 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1786,13 +1786,19 @@ EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
 ktime_t hrtimer_get_next_event(void)
 {
 	struct hrtimer_cpu_base *cpu_base = this_cpu_ptr(&hrtimer_bases);
-	ktime_t expires = KTIME_MAX;
 
-	guard(raw_spinlock_irqsave)(&cpu_base->lock);
-	if (!hrtimer_hres_active(cpu_base))
-		expires = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_ALL);
+	/*
+	 * When HRES is active the caller (cmp_next_hrtimer_event()) expects
+	 * KTIME_MAX.  cpu_base->hres_active is written only by the local CPU
+	 * (hrtimer_switch_to_hres() from hardirq, hrtimers_cpu_starting()
+	 * during bring-up), and all callers reach this with IRQs disabled on
+	 * the same CPU, so an unlocked read is stable across the lock below.
+	 */
+	if (hrtimer_hres_active(cpu_base))
+		return KTIME_MAX;
 
-	return expires;
+	guard(raw_spinlock_irqsave)(&cpu_base->lock);
+	return __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_ALL);
 }
 
 /**
-- 
2.53.0-Meta


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

* [tip: timers/core] hrtimer: Don't take cpu_base::lock in hrtimer_get_next_event() when hres_active
  2026-07-07 15:38 [PATCH] hrtimers: skip cpu_base->lock in hrtimer_get_next_event() when hres_active Usama Arif
@ 2026-07-07 21:32 ` tip-bot2 for Usama Arif
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot2 for Usama Arif @ 2026-07-07 21:32 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Usama Arif, Thomas Gleixner, x86, linux-kernel

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

Commit-ID:     c4415c993fc2c8bdf2cf0bfbcbb1ac0e0f7a9eaf
Gitweb:        https://git.kernel.org/tip/c4415c993fc2c8bdf2cf0bfbcbb1ac0e0f7a9eaf
Author:        Usama Arif <usama.arif@linux.dev>
AuthorDate:    Tue, 07 Jul 2026 08:38:00 -07:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Tue, 07 Jul 2026 23:26:58 +02:00

hrtimer: Don't take cpu_base::lock in hrtimer_get_next_event() when hres_active

hrtimer_get_next_event() runs on every tick-stop decision via
get_next_timer_interrupt() -> cmp_next_hrtimer_event().  When high
resolution timers are active it must return KTIME_MAX -- the caller
documents and depends on this.

The function takes cpu_base->lock, checks hres_active, and returns
KTIME_MAX.

Taking the lock is not required because cpu_base->hres_active is only
written by the local CPU in hrtimer_switch_to_hres() from hard interrupt
context and in hrtimers_cpu_starting() during bring-up.

All callers of hrtimer_get_next_event() reach it from the tick-stop /
cpuidle paths with interrupts disabled on that CPU. No writer can therefore
race with the read, so an unlocked hres_active check is stable and the lock
can be skipped in this case.

On a 176-thread AMD EPYC 9D64 running a production workload, bucketing
callers of native_queued_spin_lock_slowpath(), the slowpath had 199 samples
in total, of which 54 are attributed to hrtimer_get_next_event(), i.e. this
accounts for ~27% of slowpath hits on this specific workload.

Move the hres_active check before the lock guard region to address this.

[ tglx: Massaged change log and comments ]

Signed-off-by: Usama Arif <usama.arif@linux.dev>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20260707153800.542394-1-usama.arif@linux.dev
---
 kernel/time/hrtimer.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 9c320ab..697816d 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1786,13 +1786,21 @@ EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
 ktime_t hrtimer_get_next_event(void)
 {
 	struct hrtimer_cpu_base *cpu_base = this_cpu_ptr(&hrtimer_bases);
-	ktime_t expires = KTIME_MAX;
 
-	guard(raw_spinlock_irqsave)(&cpu_base->lock);
-	if (!hrtimer_hres_active(cpu_base))
-		expires = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_ALL);
+	/*
+	 * When HRES is active cmp_next_hrtimer_event() expects KTIME_MAX.
+	 *
+	 * cpu_base->hres_active is written only by the local CPU in
+	 * hrtimer_switch_to_hres() from hard interrupt context and in
+	 * hrtimers_cpu_starting() during CPU bring-up, and all callers reach
+	 * this with interrupts disabled on the same CPU, so an unlocked read is
+	 * stable without holding the lock.
+	 */
+	if (hrtimer_hres_active(cpu_base))
+		return KTIME_MAX;
 
-	return expires;
+	guard(raw_spinlock_irqsave)(&cpu_base->lock);
+	return __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_ALL);
 }
 
 /**

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

end of thread, other threads:[~2026-07-07 21:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 15:38 [PATCH] hrtimers: skip cpu_base->lock in hrtimer_get_next_event() when hres_active Usama Arif
2026-07-07 21:32 ` [tip: timers/core] hrtimer: Don't take cpu_base::lock " tip-bot2 for Usama Arif

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox