[PATCH 0/6] hrtimer: add an expiry injecting callback variant

[PATCH 0/6] hrtimer: add an expiry injecting callback variant

[Andreas Hindborg]

An hrtimer callback runs after __run_hrtimer() has dropped the
cpu_base->lock, and a timer can be armed from any CPU at any time,
including while its callback is running. A callback that adjusts its
own expiry with hrtimer_forward() therefore races with a concurrent
hrtimer_start_range_ns():

- The read-modify-write of node.expires in hrtimer_forward() is
  unsynchronized against the write in the start path, which happens
  under the base lock.

- The is_queued check in hrtimer_forward() is a
  time-of-check-to-time-of-use bug. A concurrent start can enqueue
  the timer right after the check, and forwarding a queued timer
  changes the expiry of a node inside the timerqueue without
  re-sorting it, leaving the tree unordered.

Users that both forward from the callback and arm the timer from
other contexts have to serialize the two themselves, as perf does
with cpc->hrtimer_lock and the hrtimer_active flag, see 4cfafd3082af
("sched,perf: Fix periodic timers"). The requirement is subtle. i915_pmu
and taprio do not honor it today.

For the Rust hrtimer abstraction this is a soundness problem rather than
a documentation problem: safe Rust code can arm a timer whose callback
is running, because Arc<T> is Clone and Pin<&T> is Copy, so a callback
context forward() cannot be offered as safe API at all. Making arming
exclusive in the Rust type system was tried [1] and abandoned. It adds
complexity to the Arc based API, and it leaves the C interface as the
same trap for C users.

Gary suggested [2] removing the race structurally instead: snapshot
the expiry under the base lock, hand it to the callback by value, and
have the callback request the forward and the requeue instead of
performing them itself. This series implements that suggestion.

Patch 1 adds the expiry injecting callback variant to the hrtimer core.
Such a callback is installed with hrtimer_setup_ext() and receives
the expiry snapshotted under the base lock. To restart the timer it
fills a struct hrtimer_forward_args and returns HRTIMER_RESTART, and
__run_hrtimer() then applies the forward and the enqueue with the base
lock held. The callback never touches live timer state. If a concurrent
start enqueued the timer while the callback ran, the restart request
is discarded and the start wins, which matches how we already treat a
restart of a timer that was requeued behind the callback's back. The
new callback pointer shares storage with the classic one in an anonymous
union and is discriminated by a flag placed in existing padding, so
struct hrtimer does not grow and the classic callback path is untouched.

Patch 2 converts i915_pmu, which forwards from its sampling callback
while gt park/unpark can start the timer from another CPU. The
conversion closes that window without adding locking to the sampling
path, and demonstrates that the new variant is not Rust-only
plumbing.

Patches 3 to 6 are the Rust side. Patch 3 moves the abstraction to
the new callback variant: HrTimerCallback::run() receives the expiry
snapshot and returns HrTimerRestart::Forward { now, interval }, and
HrTimerCallbackContext with its forward()/forward_now() methods is
removed. Patch 4 is Tomonori's expires() fix rebased on top, now
justified by exclusive access rather than by callback context. Patch
5 documents the pre-existing hazard that starting a timer from within
its own handler self-deadlocks when the returned handle is dropped
there.

The i915 patch is compile tested only, I have no hardware for it.

[1]: https://lore.kernel.org/rust-for-linux/20260813134834.1562995-1-tomo@flapping.org/
[2]: https://lore.kernel.org/rust-for-linux/DKNVOU9JC15P.3DEBNZ56QK20E@garyguo.net/

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Andreas Hindborg (4):
      hrtimer: add expiry injecting callback variant
      drm/i915/pmu: use the expiry injecting hrtimer callback
      rust: hrtimer: use the expiry injecting callback variant
      rust: hrtimer: document deadlock when starting a timer in its handler

FUJITA Tomonori (2):
      rust: hrtimer: restrict expires() to exclusive access
      rust: hrtimer: Make HrTimer repr(transparent)

 drivers/gpu/drm/i915/i915_pmu.c     |   8 +-
 include/linux/hrtimer.h             |   7 +
 include/linux/hrtimer_types.h       |  34 ++++-
 kernel/time/hrtimer.c               | 112 +++++++++++++++-
 rust/helpers/time.c                 |   6 +
 rust/kernel/time/hrtimer.rs         | 257 +++++++++++++++++++-----------------
 rust/kernel/time/hrtimer/arc.rs     |  23 ++--
 rust/kernel/time/hrtimer/pin.rs     |  23 ++--
 rust/kernel/time/hrtimer/pin_mut.rs |  26 ++--
 rust/kernel/time/hrtimer/tbox.rs    |  23 ++--
 10 files changed, 352 insertions(+), 167 deletions(-)
---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260825-expires-v2-0764adf4c466

Best regards,
--  
Andreas Hindborg <a.hindborg@kernel.org>

[PATCH 1/6] hrtimer: add expiry injecting callback variant

[Andreas Hindborg]

A hrtimer callback may modify its own expiry with hrtimer_forward()
and read it with hrtimer_get_expires(). Both run after
__run_hrtimer() has dropped the cpu_base->lock, so they race with a
concurrent hrtimer_start_range_ns() on another CPU, which rewrites
node.expires under the base lock and requeues the timer:

- The unlocked read-modify-write of node.expires in
  hrtimer_forward() is a data race against the locked write in the
  start path.

- The is_queued check in hrtimer_forward() is racy with
  a time-of-check-time-of-use bug as well: a concurrent
  start can enqueue the timer between the check and the expiry
  update, and forwarding an already queued timer changes the expiry
  of a node inside the timerqueue without re-sorting, leaving the
  tree unordered.

Timer users which both forward in the callback and arm from other contexts
must provide their own serialization, e.g. perf's cpc->hrtimer_lock plus
hrtimer_active flag, see commit 4cfafd3082af ("sched,perf: Fix periodic
timers"). The requirement is subtle and not enforced; i915_pmu and taprio
currently get it wrong. For the Rust hrtimer abstraction it is a soundness
problem: safe code can arm a timer whose callback is running, so callback
context forward and expiry reads cannot be offered as safe API.

Add an alternative callback variant that removes the race
structurally instead of requiring serialization. An expiry injecting
callback receives the expiry snapshotted under the base lock by
value and, to restart the timer, fills a struct hrtimer_forward_args
and returns HRTIMER_RESTART. __run_hrtimer() then applies the
forward and the enqueue with the base lock held. The callback never
accesses live timer state.

If a concurrent start enqueued the timer while the callback ran, the
restart request is discarded and the start wins, matching the
existing "restart == HRTIMER_RESTART && !timer->is_queued" handling
for classic callbacks. The is_queued check is reliable here: while
base->running == timer, hrtimer_try_to_cancel() bails out before
remove_hrtimer() and the timer cannot switch bases, so only a
concurrent start can enqueue it, and the start path writes the
expiry and is_queued in the same critical section. Thus !is_queued
at requeue time guarantees the expiry still equals the snapshot
handed to the callback: the deferred hrtimer_forward() cannot hit
its concurrent start check, and an overrun count the callback
derived from the snapshot is consistent with the forward that is
applied.

The new callback pointer shares storage with the classic one in an
anonymous union, discriminated by a new is_ext flag placed in
existing padding; sizeof(struct hrtimer) is unchanged and the
classic callback path is unaffected. hrtimer_update_function()
rejects timers with an expiry injecting callback.

Link: https://lore.kernel.org/r/87h5kp88uy.fsf@kernel.org
Suggested-by: Gary Guo <gary@garyguo.net>
Assisted-by: claude-code:claude-fable-5
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 include/linux/hrtimer.h       |   7 +++
 include/linux/hrtimer_types.h |  34 ++++++++++++-
 kernel/time/hrtimer.c         | 112 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 145 insertions(+), 8 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 6862dea0acc52..a45a0677238c9 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -190,6 +190,10 @@ static inline enum hrtimer_restart hrtimer_dummy_timeout(struct hrtimer *unused)
 /* Initialize timers: */
 extern void hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*function)(struct hrtimer *),
 			  clockid_t clock_id, enum hrtimer_mode mode);
+
+extern void hrtimer_setup_ext(struct hrtimer *timer, hrtimer_ext_func_t function_ext,
+			      clockid_t clock_id, enum hrtimer_mode mode);
+
 extern void hrtimer_setup_on_stack(struct hrtimer *timer,
 				   enum hrtimer_restart (*function)(struct hrtimer *),
 				   clockid_t clock_id, enum hrtimer_mode mode);
@@ -307,6 +311,9 @@ static inline int hrtimer_callback_running(struct hrtimer *timer)
 static inline void hrtimer_update_function(struct hrtimer *timer,
 					   enum hrtimer_restart (*function)(struct hrtimer *))
 {
+	if (WARN_ON_ONCE(timer->is_ext))
+		return;
+
 #ifdef CONFIG_PROVE_LOCKING
 	guard(raw_spinlock_irqsave)(&timer->base->cpu_base->lock);
 
diff --git a/include/linux/hrtimer_types.h b/include/linux/hrtimer_types.h
index b5dacc8271a4a..7c7e03dffcae7 100644
--- a/include/linux/hrtimer_types.h
+++ b/include/linux/hrtimer_types.h
@@ -5,6 +5,7 @@
 #include <linux/types.h>
 #include <linux/timerqueue_types.h>
 
+struct hrtimer;
 struct hrtimer_clock_base;
 
 /*
@@ -15,6 +16,26 @@ enum hrtimer_restart {
 	HRTIMER_RESTART,	/* Timer must be restarted */
 };
 
+/**
+ * struct hrtimer_forward_args - deferred forward request of an expiry
+ *				 injecting callback
+ * @now:	forward past this time
+ * @interval:	the interval to forward by
+ *
+ * Filled in by an expiry injecting callback (see hrtimer_setup_ext())
+ * which returns HRTIMER_RESTART. The hrtimer core applies the forward
+ * with the timer base lock held before requeueing the timer, i.e.
+ * hrtimer_forward(timer, now, interval).
+ */
+struct hrtimer_forward_args {
+	ktime_t				now;
+	ktime_t				interval;
+};
+
+/* Callback function of an expiry injecting timer, see hrtimer_setup_ext() */
+typedef enum hrtimer_restart (*hrtimer_ext_func_t)(struct hrtimer *timer, ktime_t expires,
+						   struct hrtimer_forward_args *fwd);
+
 /**
  * struct hrtimer - the basic hrtimer structure
  * @node:	Linked timerqueue node, which also manages node.expires,
@@ -27,6 +48,9 @@ enum hrtimer_restart {
  *		The time which was given as expiry time when the timer
  *		was armed.
  * @function:	timer expiry callback function
+ * @function_ext: expiry injecting timer callback function, receives the
+ *		expiry snapshot and returns a forward request instead of
+ *		modifying the expiry itself. Valid if @is_ext is set.
  * @base:	pointer to the timer base (per cpu and per clock)
  * @is_queued:	Indicates whether a timer is enqueued or not
  * @is_rel:	Set if the timer was armed relative
@@ -35,8 +59,10 @@ enum hrtimer_restart {
  *		even on RT.
  * @is_lazy:	Set if the timer is frequently rearmed to avoid updates
  *		of the clock event device
+ * @is_ext:	Set if @function_ext is valid instead of @function
  *
- * The hrtimer structure must be initialized by hrtimer_setup()
+ * The hrtimer structure must be initialized by hrtimer_setup() or
+ * hrtimer_setup_ext()
  */
 struct hrtimer {
 	struct timerqueue_linked_node	node;
@@ -46,8 +72,12 @@ struct hrtimer {
 	bool				is_soft;
 	bool				is_hard;
 	bool				is_lazy;
+	bool				is_ext;
 	ktime_t				_softexpires;
-	enum hrtimer_restart		(*__private function)(struct hrtimer *);
+	union {
+		enum hrtimer_restart	(*__private function)(struct hrtimer *);
+		hrtimer_ext_func_t	__private function_ext;
+	};
 };
 
 #endif /* _LINUX_HRTIMER_TYPES_H */
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 313dcea127fe4..e718dd0e05195 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1863,8 +1863,7 @@ ktime_t hrtimer_cb_get_time(const struct hrtimer *timer)
 }
 EXPORT_SYMBOL_GPL(hrtimer_cb_get_time);
 
-static void __hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*fn)(struct hrtimer *),
-			    clockid_t clock_id, enum hrtimer_mode mode)
+static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id, enum hrtimer_mode mode)
 {
 	bool softtimer = !!(mode & HRTIMER_MODE_SOFT);
 	struct hrtimer_cpu_base *cpu_base;
@@ -1898,6 +1897,12 @@ static void __hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*fn)(st
 	timer->is_lazy = !!(mode & HRTIMER_MODE_LAZY_REARM);
 	timer->base = &cpu_base->clock_base[base];
 	timerqueue_linked_init(&timer->node);
+}
+
+static void __hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*fn)(struct hrtimer *),
+			    clockid_t clock_id, enum hrtimer_mode mode)
+{
+	__hrtimer_init(timer, clock_id, mode);
 
 	if (WARN_ON_ONCE(!fn))
 		ACCESS_PRIVATE(timer, function) = hrtimer_dummy_timeout;
@@ -1905,6 +1910,20 @@ static void __hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*fn)(st
 		ACCESS_PRIVATE(timer, function) = fn;
 }
 
+static void __hrtimer_setup_ext(struct hrtimer *timer, hrtimer_ext_func_t fn,
+				clockid_t clock_id, enum hrtimer_mode mode)
+{
+	__hrtimer_init(timer, clock_id, mode);
+
+	if (WARN_ON_ONCE(!fn)) {
+		ACCESS_PRIVATE(timer, function) = hrtimer_dummy_timeout;
+		return;
+	}
+
+	ACCESS_PRIVATE(timer, function_ext) = fn;
+	timer->is_ext = true;
+}
+
 /**
  * hrtimer_setup - initialize a timer to the given clock
  * @timer:	the timer to be initialized
@@ -1926,6 +1945,45 @@ void hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*function)(struc
 }
 EXPORT_SYMBOL_GPL(hrtimer_setup);
 
+/**
+ * hrtimer_setup_ext - initialize a timer with an expiry injecting callback
+ * @timer:	the timer to be initialized
+ * @function_ext: the expiry injecting callback function
+ * @clock_id:	the clock to be used
+ * @mode:       The modes which are relevant for initialization:
+ *              HRTIMER_MODE_ABS, HRTIMER_MODE_REL, HRTIMER_MODE_ABS_SOFT,
+ *              HRTIMER_MODE_REL_SOFT
+ *
+ *              The PINNED variants of the above can be handed in,
+ *              but the PINNED bit is ignored as pinning happens
+ *              when the hrtimer is started
+ *
+ * In contrast to a callback installed by hrtimer_setup(), an expiry
+ * injecting callback does not access the expiry of the timer itself.
+ * The expiry is snapshotted under the timer base lock and handed into
+ * the callback by value. To restart the timer, the callback fills @fwd
+ * and returns HRTIMER_RESTART; the core then applies
+ * hrtimer_forward(timer, fwd->now, fwd->interval) and requeues the
+ * timer, both under the timer base lock.
+ *
+ * This closes the race between hrtimer_forward()/expiry reads in
+ * callback context and a concurrent hrtimer_start() on another CPU,
+ * without requiring the timer user to provide serialization: if a
+ * concurrent start requeued the timer while the callback ran, the
+ * restart request is discarded and the concurrent start wins.
+ *
+ * The callback must not call hrtimer_forward() or modify the expiry
+ * itself, and a callback returning HRTIMER_RESTART must fill @fwd with
+ * a non zero interval.
+ */
+void hrtimer_setup_ext(struct hrtimer *timer, hrtimer_ext_func_t function_ext,
+		       clockid_t clock_id, enum hrtimer_mode mode)
+{
+	debug_setup(timer, clock_id, mode);
+	__hrtimer_setup_ext(timer, function_ext, clock_id, mode);
+}
+EXPORT_SYMBOL_GPL(hrtimer_setup_ext);
+
 /**
  * hrtimer_setup_on_stack - initialize a timer on stack memory
  * @timer:	The timer to be initialized
@@ -1991,8 +2049,11 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, struct hrtimer_cloc
 			  struct hrtimer *timer, ktime_t now, unsigned long flags)
 	__must_hold(&cpu_base->lock)
 {
-	enum hrtimer_restart (*fn)(struct hrtimer *);
+	enum hrtimer_restart (*fn)(struct hrtimer *) = NULL;
+	struct hrtimer_forward_args fwd = { };
+	hrtimer_ext_func_t fn_ext = NULL;
 	bool expires_in_hardirq;
+	ktime_t expires = 0;
 	int restart;
 
 	lockdep_assert_held(&cpu_base->lock);
@@ -2010,7 +2071,20 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, struct hrtimer_cloc
 	raw_write_seqcount_barrier(&base->seq);
 
 	__remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, false);
-	fn = ACCESS_PRIVATE(timer, function);
+
+	/*
+	 * Snapshot the expiry for an expiry injecting callback while the
+	 * base lock is still held. The callback gets the snapshot by
+	 * value and must not access timer->node.expires, which a
+	 * concurrent hrtimer_start_range_ns() can modify once the lock is
+	 * dropped.
+	 */
+	if (timer->is_ext) {
+		fn_ext = ACCESS_PRIVATE(timer, function_ext);
+		expires = hrtimer_get_expires(timer);
+	} else {
+		fn = ACCESS_PRIVATE(timer, function);
+	}
 
 	/*
 	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
@@ -2029,12 +2103,19 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, struct hrtimer_cloc
 	trace_hrtimer_expire_entry(timer, now);
 	expires_in_hardirq = lockdep_hrtimer_enter(timer);
 
-	restart = fn(timer);
+	if (fn_ext)
+		restart = fn_ext(timer, expires, &fwd);
+	else
+		restart = fn(timer);
 
 	lockdep_hrtimer_exit(expires_in_hardirq);
 	trace_hrtimer_expire_exit(timer);
 	raw_spin_lock_irq(&cpu_base->lock);
 
+	/* An expiry injecting callback requesting a restart must forward. */
+	if (fn_ext && restart == HRTIMER_RESTART && WARN_ON_ONCE(!fwd.interval))
+		restart = HRTIMER_NORESTART;
+
 	/*
 	 * Note: We clear the running state after enqueue_hrtimer and
 	 * we do not reprogram the event hardware. Happens either in
@@ -2044,8 +2125,27 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, struct hrtimer_cloc
 	 * hrtimer_start_range_ns() can have popped in and enqueued the timer
 	 * for us already.
 	 */
-	if (restart == HRTIMER_RESTART && !timer->is_queued)
+	if (restart == HRTIMER_RESTART && !timer->is_queued) {
+		/*
+		 * Apply the deferred forward of an expiry injecting
+		 * callback with the base lock held.
+		 *
+		 * While base->running == timer, hrtimer_try_to_cancel()
+		 * bails out before remove_hrtimer() and the timer cannot
+		 * switch bases, so only a concurrent start can have
+		 * enqueued the timer and it writes the expiry and
+		 * is_queued in the same critical section. Thus !is_queued
+		 * here guarantees that the expiry is still equal to the
+		 * snapshot handed to the callback, and the concurrent
+		 * start check in hrtimer_forward() cannot trigger. If a
+		 * concurrent start enqueued the timer, is_queued is set
+		 * and the restart request is discarded - the concurrent
+		 * start expressed newer intent and wins.
+		 */
+		if (fn_ext)
+			hrtimer_forward(timer, fwd.now, fwd.interval);
 		enqueue_hrtimer(timer, base, HRTIMER_MODE_ABS, false);
+	}
 
 	/*
 	 * Separate the ->running assignment from the ->is_queued assignment.

-- 
2.51.2

[PATCH 2/6] drm/i915/pmu: use the expiry injecting hrtimer callback

[Andreas Hindborg]

The sampling timer forwards itself from its callback:

  i915_sample()
    hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));

hrtimer callbacks run with the timer base lock dropped, so this
read-modify-write of the expiry races with a concurrent restart of
the timer. Such a restart is possible here: i915_pmu_gt_parked()
clears pmu->timer_enabled, and a subsequent
__i915_pmu_maybe_start_timer() from i915_pmu_gt_unparked() or event
enable on another CPU sees the timer disabled and calls
hrtimer_start_range_ns() - also while the callback is running, since
i915_sample() checks timer_enabled only once at entry and takes no
lock. hrtimer_forward() then operates on an already requeued timer:
it warns and, in the worst case, rewrites the expiry of an enqueued
timer without the base lock.

Convert the timer to the expiry injecting callback variant. The
callback returns the forward request instead of applying it, and the
hrtimer core applies it under the timer base lock. If a concurrent
start requeued the timer while the callback ran, the core discards
the callback's restart request and the start wins, which closes the
park/unpark race without adding any locking to the sampling path.

No functional change in the common case: the timer still forwards by
PERIOD past the sampling timestamp.

Assisted-by: claude-code:claude-fable-5
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/gpu/drm/i915/i915_pmu.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 1c3bafda9c708..c18587e200c9f 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -502,7 +502,8 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 	intel_gt_pm_put_async(gt, wakeref);
 }
 
-static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
+static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer, ktime_t expires,
+					struct hrtimer_forward_args *fwd)
 {
 	struct i915_pmu *pmu = container_of(hrtimer, struct i915_pmu, timer);
 	struct drm_i915_private *i915 = pmu_to_i915(pmu);
@@ -533,7 +534,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 		frequency_sample(gt, period_ns);
 	}
 
-	hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));
+	fwd->now = now;
+	fwd->interval = ns_to_ktime(PERIOD);
 
 	return HRTIMER_RESTART;
 }
@@ -1157,7 +1159,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	int ret = -ENOMEM;
 
 	spin_lock_init(&pmu->lock);
-	hrtimer_setup(&pmu->timer, i915_sample, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_setup_ext(&pmu->timer, i915_sample, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	init_rc6(pmu);
 
 	if (IS_DGFX(i915)) {

-- 
2.51.2

[PATCH 3/6] rust: hrtimer: use the expiry injecting callback variant

[Andreas Hindborg]

A timer callback could modify the expiry of its timer with
HrTimerCallbackContext::forward(). The C side runs callbacks with the
timer base lock dropped, and a concurrent start operation - reachable
from safe code, since Arc<T> is Clone and Pin<&T> is Copy - rewrites
the expiry under the base lock and requeues the timer. The unlocked
read-modify-write of the expiry in hrtimer_forward() is a data race
with such a concurrent start, and forwarding a requeued timer changes
the expiry of a node inside the timerqueue without re-sorting it,
leaving the tree unordered.

Switch the abstraction to the expiry injecting callback variant
provided by hrtimer_setup_ext(). The callback receives the expiry by
value, snapshotted under the timer base lock, and requests a restart
by returning HrTimerRestart::Forward { now, interval }. The timer
core applies the forward and requeues the timer under the timer base
lock after the callback has returned. If a concurrent start operation
requeued the timer while the callback ran, the request is discarded
and the concurrent start operation wins.

The callback never accesses live timer state, so the race is removed
structurally while concurrent start operations remain allowed:

- HrTimerCallback::run() receives the expiry snapshot instead of a
  HrTimerCallbackContext and returns HrTimerRestart<T>, which now
  carries the forward request.

- HrTimerCallbackContext is removed together with its forward() and
  forward_now() methods. HrTimer::forward() on an exclusive reference
  remains available.

- The callback trampolines of the four pointer types translate the
  expiry snapshot and the forward request across the FFI boundary.

Link: https://lore.kernel.org/r/87h5kp88uy.fsf@kernel.org
Suggested-by: Gary Guo <gary@garyguo.net>
Assisted-by: claude-code:claude-fable-5
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/time/hrtimer.rs         | 212 ++++++++++++++++++------------------
 rust/kernel/time/hrtimer/arc.rs     |  21 ++--
 rust/kernel/time/hrtimer/pin.rs     |  21 ++--
 rust/kernel/time/hrtimer/pin_mut.rs |  24 ++--
 rust/kernel/time/hrtimer/tbox.rs    |  21 ++--
 5 files changed, 162 insertions(+), 137 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 2d7f1131a8131..e6570a6162035 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -60,16 +60,23 @@
 //! by the `cancel` operation. A timer that is cancelled enters the **stopped**
 //! state.
 //!
-//! A `cancel` or `restart` operation on a timer in the **running** state takes
-//! effect after the handler has returned and the timer has transitioned
-//! out of the **running** state.
+//! The handler receives the expiry time that was in effect when the timer fired. To restart the
+//! timer, the handler returns a forward request; the timer core applies the request and requeues
+//! the timer. A concurrent `start` operation overrides the restart request of the timer handler.
+//!
+//! A `cancel` operation on a timer in the **running** state takes effect after
+//! the handler has returned and the timer has transitioned out of the
+//! **running** state.
 //!
 //! A `restart` operation on a timer in the **stopped** state is equivalent to a
 //! `start` operation.
 //!
-//! When a type implements both `HrTimerPointer` and `Clone`, it is possible to
-//! issue the `start` operation while the timer is in the **started** state. In
-//! this case the `start` operation is equivalent to the `restart` operation.
+//! When a type implements both `HrTimerPointer` and `Clone`, it is possible to issue the `start`
+//! operation while the timer is in the **started** or **running** state. In this case the `start`
+//! operation is equivalent to the `restart` operation. A `restart` operation on a timer in the
+//! **running** state takes effect immediately: the timer re-enters the **started** state before the
+//! handler returns, and a restart requested by the return value of the handler is discarded in
+//! favor of the `restart` operation.
 //!
 //! # Examples
 //!
@@ -87,8 +94,8 @@
 //! #     },
 //! #     time::{
 //! #         hrtimer::{
-//! #             RelativeMode, HrTimer, HrTimerCallback, HrTimerPointer,
-//! #             HrTimerRestart, HrTimerCallbackContext
+//! #             RelativeMode, HrTimer, HrTimerCallback, HrTimerInstant,
+//! #             HrTimerPointer, HrTimerRestart
 //! #         },
 //! #         Delta, Monotonic,
 //! #     },
@@ -130,7 +137,7 @@
 //! impl HrTimerCallback for BoxIntrusiveHrTimer {
 //!     type Pointer<'a> = Pin<KBox<Self>>;
 //!
-//!     fn run(this: Pin<&mut Self>, _ctx: HrTimerCallbackContext<'_, Self>) -> HrTimerRestart {
+//!     fn run(this: Pin<&mut Self>, _expires: HrTimerInstant<Self>) -> HrTimerRestart<Self> {
 //!         pr_info!("Timer called\n");
 //!
 //!         let flag = this.shared.flag.fetch_add(1, ordering::Full);
@@ -139,7 +146,7 @@
 //!         if flag == 4 {
 //!             HrTimerRestart::NoRestart
 //!         } else {
-//!             HrTimerRestart::Restart
+//!             HrTimerRestart::forward_now(Delta::from_micros(200))
 //!         }
 //!     }
 //! }
@@ -176,8 +183,8 @@
 //! #     },
 //! #     time::{
 //! #         hrtimer::{
-//! #             RelativeMode, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
-//! #             HasHrTimer, HrTimerCallbackContext
+//! #             RelativeMode, HrTimer, HrTimerCallback, HrTimerInstant, HrTimerPointer,
+//! #             HrTimerRestart, HasHrTimer
 //! #         },
 //! #         Delta, Monotonic,
 //! #     },
@@ -208,8 +215,8 @@
 //!
 //!     fn run(
 //!         this: ArcBorrow<'_, Self>,
-//!         _ctx: HrTimerCallbackContext<'_, Self>,
-//!     ) -> HrTimerRestart {
+//!         _expires: HrTimerInstant<Self>,
+//!     ) -> HrTimerRestart<Self> {
 //!         pr_info!("Timer called\n");
 //!
 //!         let flag = this.flag.fetch_add(1, ordering::Full);
@@ -218,7 +225,7 @@
 //!         if flag == 4 {
 //!             HrTimerRestart::NoRestart
 //!         } else {
-//!             HrTimerRestart::Restart
+//!             HrTimerRestart::forward_now(Delta::from_micros(200))
 //!         }
 //!     }
 //! }
@@ -252,8 +259,8 @@
 //! #     },
 //! #     time::{
 //! #         hrtimer::{
-//! #             ScopedHrTimerPointer, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
-//! #             HasHrTimer, RelativeMode, HrTimerCallbackContext
+//! #             ScopedHrTimerPointer, HrTimer, HrTimerCallback, HrTimerInstant,
+//! #             HrTimerPointer, HrTimerRestart, HasHrTimer, RelativeMode
 //! #         },
 //! #         Delta, Monotonic,
 //! #     },
@@ -283,7 +290,7 @@
 //! impl HrTimerCallback for IntrusiveHrTimer {
 //!     type Pointer<'a> = Pin<&'a Self>;
 //!
-//!     fn run(this: Pin<&Self>, _ctx: HrTimerCallbackContext<'_, Self>) -> HrTimerRestart {
+//!     fn run(this: Pin<&Self>, _expires: HrTimerInstant<Self>) -> HrTimerRestart<Self> {
 //!         pr_info!("Timer called\n");
 //!
 //!         this.flag.store(1, ordering::Release);
@@ -324,8 +331,8 @@
 //! #     },
 //! #     time::{
 //! #         hrtimer::{
-//! #             ScopedHrTimerPointer, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
-//! #             HasHrTimer, RelativeMode, HrTimerCallbackContext
+//! #             ScopedHrTimerPointer, HrTimer, HrTimerCallback, HrTimerInstant,
+//! #             HrTimerPointer, HrTimerRestart, HasHrTimer, RelativeMode
 //! #         },
 //! #         Delta, Monotonic,
 //! #     },
@@ -368,7 +375,7 @@
 //! impl HrTimerCallback for IntrusiveHrTimer {
 //!     type Pointer<'a> = Pin<&'a mut Self>;
 //!
-//!     fn run(this: Pin<&mut Self>, _ctx: HrTimerCallbackContext<'_, Self>) -> HrTimerRestart {
+//!     fn run(this: Pin<&mut Self>, _expires: HrTimerInstant<Self>) -> HrTimerRestart<Self> {
 //!         pr_info!("Timer called\n");
 //!
 //!         let flag = this.shared.flag.fetch_add(1, ordering::Full);
@@ -377,7 +384,7 @@
 //!         if flag == 4 {
 //!             HrTimerRestart::NoRestart
 //!         } else {
-//!             HrTimerRestart::Restart
+//!             HrTimerRestart::forward_now(Delta::from_micros(200))
 //!         }
 //!     }
 //! }
@@ -405,7 +412,7 @@
 
 use super::{ClockSource, Delta, Instant};
 use crate::{prelude::*, types::Opaque};
-use core::{marker::PhantomData, ptr::NonNull};
+use core::marker::PhantomData;
 use pin_init::PinInit;
 
 /// A type-alias to refer to the [`Instant<C>`] for a given `T` from [`HrTimer<T>`].
@@ -417,7 +424,7 @@
 ///
 /// # Invariants
 ///
-/// * `self.timer` is initialized by `bindings::hrtimer_setup`.
+/// * `self.timer` is initialized by `bindings::hrtimer_setup_ext`.
 #[pin_data]
 #[repr(C)]
 pub struct HrTimer<T> {
@@ -442,13 +449,14 @@ pub fn new() -> impl PinInit<Self>
         T: HasHrTimer<T>,
     {
         pin_init!(Self {
-            // INVARIANT: We initialize `timer` with `hrtimer_setup` below.
+            // INVARIANT: We initialize `timer` with `hrtimer_setup_ext` below.
             timer <- Opaque::ffi_init(move |place: *mut bindings::hrtimer| {
                 // SAFETY: By design of `pin_init!`, `place` is a pointer to a
-                // live allocation. hrtimer_setup will initialize `place` and
-                // does not require `place` to be initialized prior to the call.
+                // live allocation. hrtimer_setup_ext will initialize `place`
+                // and does not require `place` to be initialized prior to the
+                // call.
                 unsafe {
-                    bindings::hrtimer_setup(
+                    bindings::hrtimer_setup_ext(
                         place,
                         Some(T::Pointer::run),
                         <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Clock::ID,
@@ -510,8 +518,7 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
     /// # Safety
     ///
     /// - `self_ptr` must point to a valid `Self`.
-    /// - The caller must either have exclusive access to the data pointed at by `self_ptr`, or be
-    ///   within the context of the timer callback.
+    /// - The caller must have exclusive access to the data pointed at by `self_ptr`.
     #[inline]
     unsafe fn raw_forward(self_ptr: *mut Self, now: HrTimerInstant<T>, interval: Delta) -> u64
     where
@@ -533,8 +540,8 @@ unsafe fn raw_forward(self_ptr: *mut Self, now: HrTimerInstant<T>, interval: Del
     /// `interval`.
     ///
     /// This function is mainly useful for timer types which can provide exclusive access to the
-    /// timer when the timer is not running. For forwarding the timer from within the timer callback
-    /// context, see [`HrTimerCallbackContext::forward()`].
+    /// timer when the timer is not running. To forward the timer from within the timer callback,
+    /// return [`HrTimerRestart::Forward`] from the callback instead.
     ///
     /// Returns the number of overruns that occurred as a result of the timer expiry change.
     pub fn forward(self: Pin<&mut Self>, now: HrTimerInstant<T>, interval: Delta) -> u64
@@ -707,9 +714,16 @@ pub trait RawHrTimerCallback {
     ///
     /// # Safety
     ///
-    /// Only to be called by C code in the `hrtimer` subsystem. `this` must point
-    /// to the `bindings::hrtimer` structure that was used to start the timer.
-    unsafe extern "C" fn run(this: *mut bindings::hrtimer) -> bindings::hrtimer_restart;
+    /// Only to be called by C code in the `hrtimer` subsystem. `this` must
+    /// point to the `bindings::hrtimer` structure that was used to start the
+    /// timer, `expires` must be the expiry of the timer snapshotted under the
+    /// timer base lock, and `fwd` must be valid for writing a
+    /// `bindings::hrtimer_forward_args`.
+    unsafe extern "C" fn run(
+        this: *mut bindings::hrtimer,
+        expires: bindings::ktime_t,
+        fwd: *mut bindings::hrtimer_forward_args,
+    ) -> bindings::hrtimer_restart;
 }
 
 /// Implemented by structs that can be the target of a timer callback.
@@ -719,10 +733,14 @@ pub trait HrTimerCallback {
     type Pointer<'a>: RawHrTimerCallback;
 
     /// Called by the timer logic when the timer fires.
+    ///
+    /// `expires` is the expiry time of the timer, read under the timer base
+    /// lock when the timer fired. A concurrent restart of the timer is not
+    /// reflected in `expires`.
     fn run(
         this: <Self::Pointer<'_> as RawHrTimerCallback>::CallbackTarget<'_>,
-        ctx: HrTimerCallbackContext<'_, Self>,
-    ) -> HrTimerRestart
+        expires: HrTimerInstant<Self>,
+    ) -> HrTimerRestart<Self>
     where
         Self: Sized,
         Self: HasHrTimer<Self>;
@@ -829,19 +847,62 @@ unsafe fn start(this: *const Self, expires: <Self::TimerMode as HrTimerMode>::Ex
     }
 }
 
-/// Restart policy for timers.
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
-#[repr(u32)]
-pub enum HrTimerRestart {
+/// Restart policy for timers, as returned by [`HrTimerCallback::run`].
+///
+/// A timer callback requests a restart of its timer by returning
+/// [`HrTimerRestart::Forward`]. The forward is not applied by the callback
+/// itself: the timer core applies it and requeues the timer under the timer
+/// base lock after the callback has returned. If the timer was restarted by a
+/// concurrent start operation while the callback was running, the request is
+/// discarded and the concurrent start operation wins.
+pub enum HrTimerRestart<T: HasHrTimer<T>> {
     /// Timer should not be restarted.
-    NoRestart = bindings::hrtimer_restart_HRTIMER_NORESTART,
-    /// Timer should be restarted.
-    Restart = bindings::hrtimer_restart_HRTIMER_RESTART,
+    NoRestart,
+    /// Forward the timer expiry to lie past `now` in increments of `interval`
+    /// and restart the timer.
+    ///
+    /// `interval` must be a positive time delta.
+    Forward {
+        /// The point in time to forward the expiry past.
+        now: HrTimerInstant<T>,
+        /// The time interval to forward the expiry by.
+        interval: Delta,
+    },
 }
 
-impl HrTimerRestart {
-    fn into_c(self) -> bindings::hrtimer_restart {
-        self as bindings::hrtimer_restart
+impl<T: HasHrTimer<T>> HrTimerRestart<T> {
+    /// Request that the timer be forwarded past the current time by `interval`
+    /// and restarted.
+    pub fn forward_now(interval: Delta) -> Self {
+        Self::Forward {
+            now: HrTimerInstant::<T>::now(),
+            interval,
+        }
+    }
+
+    /// Convert to the C representation, filling `fwd` with the forward
+    /// request.
+    ///
+    /// # Safety
+    ///
+    /// `fwd` must be valid for writing a `bindings::hrtimer_forward_args`.
+    pub(crate) unsafe fn into_c(
+        self,
+        fwd: *mut bindings::hrtimer_forward_args,
+    ) -> bindings::hrtimer_restart {
+        match self {
+            Self::NoRestart => bindings::hrtimer_restart_HRTIMER_NORESTART,
+            Self::Forward { now, interval } => {
+                // SAFETY: By our safety contract, `fwd` is valid for writing.
+                unsafe {
+                    *fwd = bindings::hrtimer_forward_args {
+                        now: now.as_nanos(),
+                        interval: interval.as_nanos(),
+                    }
+                };
+                bindings::hrtimer_restart_HRTIMER_RESTART
+            }
+        }
     }
 }
 
@@ -1010,63 +1071,6 @@ impl<C: ClockSource> HrTimerMode for RelativePinnedHardMode<C> {
     type Expires = Delta;
 }
 
-/// Privileged smart-pointer for a [`HrTimer`] callback context.
-///
-/// Many [`HrTimer`] methods can only be called in two situations:
-///
-/// * When the caller has exclusive access to the `HrTimer` and the `HrTimer` is guaranteed not to
-///   be running.
-/// * From within the context of an `HrTimer`'s callback method.
-///
-/// This type provides access to said methods from within a timer callback context.
-///
-/// # Invariants
-///
-/// * The existence of this type means the caller is currently within the callback for an
-///   [`HrTimer`].
-/// * `self.0` always points to a live instance of [`HrTimer<T>`].
-pub struct HrTimerCallbackContext<'a, T: HasHrTimer<T>>(NonNull<HrTimer<T>>, PhantomData<&'a ()>);
-
-impl<'a, T: HasHrTimer<T>> HrTimerCallbackContext<'a, T> {
-    /// Create a new [`HrTimerCallbackContext`].
-    ///
-    /// # Safety
-    ///
-    /// This function relies on the caller being within the context of a timer callback, so it must
-    /// not be used anywhere except for within implementations of [`RawHrTimerCallback::run`]. The
-    /// caller promises that `timer` points to a valid initialized instance of
-    /// [`bindings::hrtimer`].
-    ///
-    /// The returned `Self` must not outlive the function context of [`RawHrTimerCallback::run`]
-    /// where this function is called.
-    pub(crate) unsafe fn from_raw(timer: *mut HrTimer<T>) -> Self {
-        // SAFETY: The caller guarantees `timer` is a valid pointer to an initialized
-        // `bindings::hrtimer`
-        // INVARIANT: Our safety contract ensures that we're within the context of a timer callback
-        // and that `timer` points to a live instance of `HrTimer<T>`.
-        Self(unsafe { NonNull::new_unchecked(timer) }, PhantomData)
-    }
-
-    /// Conditionally forward the timer.
-    ///
-    /// This function is identical to [`HrTimer::forward()`] except that it may only be used from
-    /// within the context of a [`HrTimer`] callback.
-    pub fn forward(&mut self, now: HrTimerInstant<T>, interval: Delta) -> u64 {
-        // SAFETY:
-        // - We are guaranteed to be within the context of a timer callback by our type invariants
-        // - By our type invariants, `self.0` always points to a valid `HrTimer<T>`
-        unsafe { HrTimer::<T>::raw_forward(self.0.as_ptr(), now, interval) }
-    }
-
-    /// Conditionally forward the timer.
-    ///
-    /// This is a variant of [`HrTimerCallbackContext::forward()`] that uses an interval after the
-    /// current time of the base clock for the [`HrTimer`].
-    pub fn forward_now(&mut self, duration: Delta) -> u64 {
-        self.forward(HrTimerInstant::<T>::now(), duration)
-    }
-}
-
 /// Use to implement the [`HasHrTimer<T>`] trait.
 ///
 /// See [`module`] documentation for an example.
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 7be82bcb352ac..8a9fcb5c69e64 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -3,13 +3,13 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
-use super::HrTimerCallbackContext;
 use super::HrTimerHandle;
 use super::HrTimerMode;
 use super::HrTimerPointer;
 use super::RawHrTimerCallback;
 use crate::sync::Arc;
 use crate::sync::ArcBorrow;
+use crate::time::Instant;
 
 /// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
 /// [`HrTimerPointer::start`].
@@ -79,7 +79,11 @@ impl<T> RawHrTimerCallback for Arc<T>
 {
     type CallbackTarget<'a> = ArcBorrow<'a, T>;
 
-    unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
+    unsafe extern "C" fn run(
+        ptr: *mut bindings::hrtimer,
+        expires: bindings::ktime_t,
+        fwd: *mut bindings::hrtimer_forward_args,
+    ) -> bindings::hrtimer_restart {
         // `HrTimer` is `repr(C)`
         let timer_ptr = ptr.cast::<super::HrTimer<T>>();
 
@@ -100,12 +104,13 @@ impl<T> RawHrTimerCallback for Arc<T>
         //    allocation from other `Arc` clones.
         let receiver = unsafe { ArcBorrow::from_raw(data_ptr) };
 
-        // SAFETY:
-        // - By C API contract `timer_ptr` is the pointer that we passed when queuing the timer, so
-        //   it is a valid pointer to a `HrTimer<T>` embedded in a `T`.
-        // - We are within `RawHrTimerCallback::run`
-        let context = unsafe { HrTimerCallbackContext::from_raw(timer_ptr) };
+        // SAFETY: By C API contract, `expires` is the expiry of the timer
+        // snapshotted under the timer base lock, and timers cannot have
+        // negative expiry times.
+        let expires = unsafe { Instant::from_ktime(expires) };
 
-        T::run(receiver, context).into_c()
+        // SAFETY: By C API contract, `fwd` is valid for writing a
+        // `bindings::hrtimer_forward_args`.
+        unsafe { T::run(receiver, expires).into_c(fwd) }
     }
 }
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index 4d39ef7816971..d1143f278f312 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -3,11 +3,11 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
-use super::HrTimerCallbackContext;
 use super::HrTimerHandle;
 use super::HrTimerMode;
 use super::RawHrTimerCallback;
 use super::UnsafeHrTimerPointer;
+use crate::time::Instant;
 use core::pin::Pin;
 
 /// A handle for a `Pin<&HasHrTimer>`. When the handle exists, the timer might be
@@ -82,7 +82,11 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
 {
     type CallbackTarget<'b> = Self;
 
-    unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
+    unsafe extern "C" fn run(
+        ptr: *mut bindings::hrtimer,
+        expires: bindings::ktime_t,
+        fwd: *mut bindings::hrtimer_forward_args,
+    ) -> bindings::hrtimer_restart {
         // `HrTimer` is `repr(C)`
         let timer_ptr = ptr.cast::<HrTimer<T>>();
 
@@ -104,12 +108,13 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
         // here.
         let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
 
-        // SAFETY:
-        // - By C API contract `timer_ptr` is the pointer that we passed when queuing the timer, so
-        //   it is a valid pointer to a `HrTimer<T>` embedded in a `T`.
-        // - We are within `RawHrTimerCallback::run`
-        let context = unsafe { HrTimerCallbackContext::from_raw(timer_ptr) };
+        // SAFETY: By C API contract, `expires` is the expiry of the timer
+        // snapshotted under the timer base lock, and timers cannot have
+        // negative expiry times.
+        let expires = unsafe { Instant::from_ktime(expires) };
 
-        T::run(receiver_pin, context).into_c()
+        // SAFETY: By C API contract, `fwd` is valid for writing a
+        // `bindings::hrtimer_forward_args`.
+        unsafe { T::run(receiver_pin, expires).into_c(fwd) }
     }
 }
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 9d9447d4d57e8..04f9d8cbddcd2 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -1,9 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{
-    HasHrTimer, HrTimer, HrTimerCallback, HrTimerCallbackContext, HrTimerHandle, HrTimerMode,
-    RawHrTimerCallback, UnsafeHrTimerPointer,
+    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, HrTimerMode, RawHrTimerCallback,
+    UnsafeHrTimerPointer,
 };
+use crate::time::Instant;
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
 
 /// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
@@ -85,7 +86,11 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
 {
     type CallbackTarget<'b> = Self;
 
-    unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
+    unsafe extern "C" fn run(
+        ptr: *mut bindings::hrtimer,
+        expires: bindings::ktime_t,
+        fwd: *mut bindings::hrtimer_forward_args,
+    ) -> bindings::hrtimer_restart {
         // `HrTimer` is `repr(C)`
         let timer_ptr = ptr.cast::<HrTimer<T>>();
 
@@ -107,12 +112,13 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
         // here.
         let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
 
-        // SAFETY:
-        // - By C API contract `timer_ptr` is the pointer that we passed when queuing the timer, so
-        //   it is a valid pointer to a `HrTimer<T>` embedded in a `T`.
-        // - We are within `RawHrTimerCallback::run`
-        let context = unsafe { HrTimerCallbackContext::from_raw(timer_ptr) };
+        // SAFETY: By C API contract, `expires` is the expiry of the timer
+        // snapshotted under the timer base lock, and timers cannot have
+        // negative expiry times.
+        let expires = unsafe { Instant::from_ktime(expires) };
 
-        T::run(receiver_pin, context).into_c()
+        // SAFETY: By C API contract, `fwd` is valid for writing a
+        // `bindings::hrtimer_forward_args`.
+        unsafe { T::run(receiver_pin, expires).into_c(fwd) }
     }
 }
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index aa1ee31a71953..c7f86909e21b5 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -3,12 +3,12 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
-use super::HrTimerCallbackContext;
 use super::HrTimerHandle;
 use super::HrTimerMode;
 use super::HrTimerPointer;
 use super::RawHrTimerCallback;
 use crate::prelude::*;
+use crate::time::Instant;
 use core::ptr::NonNull;
 
 /// A handle for a [`Box<HasHrTimer<T>>`] returned by a call to
@@ -102,7 +102,11 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>>
 {
     type CallbackTarget<'a> = Pin<&'a mut T>;
 
-    unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
+    unsafe extern "C" fn run(
+        ptr: *mut bindings::hrtimer,
+        expires: bindings::ktime_t,
+        fwd: *mut bindings::hrtimer_forward_args,
+    ) -> bindings::hrtimer_restart {
         // `HrTimer` is `repr(C)`
         let timer_ptr = ptr.cast::<super::HrTimer<T>>();
 
@@ -120,12 +124,13 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>>
         //   `data_ptr` exist.
         let data_mut_ref = unsafe { Pin::new_unchecked(&mut *data_ptr) };
 
-        // SAFETY:
-        // - By C API contract `timer_ptr` is the pointer that we passed when queuing the timer, so
-        //   it is a valid pointer to a `HrTimer<T>` embedded in a `T`.
-        // - We are within `RawHrTimerCallback::run`
-        let context = unsafe { HrTimerCallbackContext::from_raw(timer_ptr) };
+        // SAFETY: By C API contract, `expires` is the expiry of the timer
+        // snapshotted under the timer base lock, and timers cannot have
+        // negative expiry times.
+        let expires = unsafe { Instant::from_ktime(expires) };
 
-        T::run(data_mut_ref, context).into_c()
+        // SAFETY: By C API contract, `fwd` is valid for writing a
+        // `bindings::hrtimer_forward_args`.
+        unsafe { T::run(data_mut_ref, expires).into_c(fwd) }
     }
 }

-- 
2.51.2

[PATCH 4/6] rust: hrtimer: restrict expires() to exclusive access

[Andreas Hindborg]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

HrTimer::expires() read node.expires through a volatile load on a
shared reference. The read is unsynchronized: a concurrent start
operation rewrites the expiry under the timer base lock, and the
64-bit load can tear on 32-bit architectures. The volatile idiom
narrows the race but does not remove it.

Change expires() to take Pin<&mut Self>. Wherever an exclusive
reference to the timer is reachable, no start operation can run
concurrently: the timer handles own or borrow the containing object
exclusively for the box and pinned pointer types, and no exclusive
reference is reachable through an Arc. Route the read through
hrtimer_get_expires() via a helper instead of duplicating the field
access on the Rust side, and provide the unsafe expires_unchecked()
for contexts that can guarantee exclusive access by other means.

Reading the expiry from within the timer callback is served by the
expiry snapshot passed to HrTimerCallback::run(), so no callback
context accessor is needed.

Fixes: 4b0147494275 ("rust: hrtimer: Add HrTimer::expires()")
Closes: https://lore.kernel.org/rust-for-linux/87ldi7f4o1.fsf@t14s.mail-host-address-is-not-set/
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Link: https://lore.kernel.org/r/20260813134834.1562995-4-tomo@flapping.org
[ Andreas - Reword commit message and rebase on expiry injection patches. ]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/helpers/time.c         |  6 ++++++
 rust/kernel/time/hrtimer.rs | 37 +++++++++++++++++++++++--------------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/rust/helpers/time.c b/rust/helpers/time.c
index 32f4959704939..205a38839532a 100644
--- a/rust/helpers/time.c
+++ b/rust/helpers/time.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/delay.h>
+#include <linux/hrtimer.h>
 #include <linux/ktime.h>
 #include <linux/timekeeping.h>
 
@@ -38,3 +39,8 @@ __rust_helper void rust_helper_udelay(unsigned long usec)
 {
 	udelay(usec);
 }
+
+__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
+{
+	return hrtimer_get_expires(timer);
+}
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index e6570a6162035..bdb6aaa228396 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -567,27 +567,36 @@ pub fn forward_now(self: Pin<&mut Self>, interval: Delta) -> u64
         self.forward(HrTimerInstant::<T>::now(), interval)
     }
 
+    /// Return the time expiry for this [`HrTimer`].
+    ///
+    /// # Safety
+    ///
+    /// The caller must have exclusive access to `self`.
+    #[inline]
+    unsafe fn expires_unchecked(&self) -> HrTimerInstant<T>
+    where
+        T: HasHrTimer<T>,
+    {
+        // SAFETY:
+        // - The C API requirements for this function are fulfilled by our safety contract.
+        // - Timers cannot have negative `ktime_t` values as their expiration time.
+        unsafe { Instant::from_ktime(bindings::hrtimer_get_expires(Self::raw_get(self))) }
+    }
+
     /// Return the time expiry for this [`HrTimer`].
     ///
     /// This value should only be used as a snapshot, as the actual expiry time could change after
-    /// this function is called.
-    pub fn expires(&self) -> HrTimerInstant<T>
+    /// this function is called. To read the expiry from within the timer callback, use the value
+    /// passed to [`HrTimerCallback::run`] instead.
+    pub fn expires(self: Pin<&mut Self>) -> HrTimerInstant<T>
     where
         T: HasHrTimer<T>,
     {
-        // SAFETY: `self` is an immutable reference and thus always points to a valid `HrTimer`.
-        let c_timer_ptr = unsafe { HrTimer::raw_get(self) };
+        // SAFETY: `expires_unchecked` does not move `Self`.
+        let this = unsafe { self.get_unchecked_mut() };
 
-        // SAFETY:
-        // - Timers cannot have negative ktime_t values as their expiration time.
-        // - There's no actual locking here, a racy read is fine and expected
-        unsafe {
-            Instant::from_ktime(
-                // This `read_volatile` is intended to correspond to a READ_ONCE call.
-                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
-                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
-            )
-        }
+        // SAFETY: By existence of `Pin<&mut Self>`, we have exclusive access to `Self`.
+        unsafe { this.expires_unchecked() }
     }
 }
 

-- 
2.51.2

[PATCH 5/6] rust: hrtimer: document deadlock when starting a timer in its handler

[Andreas Hindborg]

The state machine documentation states that for pointer types that
implement `Clone`, the `start` operation may be issued while the timer
is in the **running** state, and that it is then equivalent to the
`restart` operation. That only holds when the operation is issued from
outside the timer handler.

The `start` operation returns a `HrTimerHandle`, and dropping the
handle cancels the timer with `hrtimer_cancel()`, which blocks until a
running handler has returned. When `start` is issued from within the
handler, the handle is also dropped within the handler, so the cancel
waits for the very handler that issues it, and the handler deadlocks.

Note this in the state machine documentation.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/time/hrtimer.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index bdb6aaa22839..2a9abc9f5d8c 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -78,6 +78,8 @@
 //! handler returns, and a restart requested by the return value of the handler is discarded in
 //! favor of the `restart` operation.
 //!
+//! ⚠️ Issuing the `start` operation from within the timer handler will lead to deadlock.
+//!
 //! # Examples
 //!
 //! ## Using an intrusive timer living in a [`Box`]

-- 
2.51.2

[PATCH 6/6] rust: hrtimer: Make HrTimer repr(transparent)

[Andreas Hindborg]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

HrTimerCallbackContext acquires a &HrTimer<T> from a
NonNull<HrTimer<T>> while a &mut HrTimer<T> can exist at the same
time. This is sound only because HrTimer's sole field is
Opaque<bindings::hrtimer>, which puts every byte behind an UnsafeCell.
Adding a field to HrTimer that is not Opaque would make acquiring that
shared reference unsound.

Make HrTimer repr(transparent), which prevents multiple fields, so that
such a refactor fails to compile instead of silently introducing
unsoundness. This does not guarantee the remaining field stays behind
Opaque, but it rules out the likely way of getting there.

repr(transparent) cannot be combined with repr(C), so drop the latter.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Link: https://msgid.link/20260813134834.1562995-5-tomo@flapping.org
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/time/hrtimer.rs         | 6 +++++-
 rust/kernel/time/hrtimer/arc.rs     | 2 +-
 rust/kernel/time/hrtimer/pin.rs     | 2 +-
 rust/kernel/time/hrtimer/pin_mut.rs | 2 +-
 rust/kernel/time/hrtimer/tbox.rs    | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 2a9abc9f5d8c..ab7c568b8855 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -427,8 +427,12 @@
 /// # Invariants
 ///
 /// * `self.timer` is initialized by `bindings::hrtimer_setup_ext`.
+// `repr(transparent)` is not merely about layout. `HrTimerCallbackContext` acquires a
+// `&HrTimer<T>` while a `&mut HrTimer<T>` may exist, which is sound only because every byte of
+// this type sits inside `Opaque`. Being transparent rejects a second field at compile time,
+// but it does not enforce that the remaining field stays `Opaque`.
 #[pin_data]
-#[repr(C)]
+#[repr(transparent)]
 pub struct HrTimer<T> {
     #[pin]
     timer: Opaque<bindings::hrtimer>,
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 8a9fcb5c69e6..46ccff9e0024 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -84,7 +84,7 @@ impl<T> RawHrTimerCallback for Arc<T>
         expires: bindings::ktime_t,
         fwd: *mut bindings::hrtimer_forward_args,
     ) -> bindings::hrtimer_restart {
-        // `HrTimer` is `repr(C)`
+        // `HrTimer` is `repr(transparent)`
         let timer_ptr = ptr.cast::<super::HrTimer<T>>();
 
         // SAFETY: By C API contract `ptr` is the pointer we passed when
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index d1143f278f31..5fd374fdc480 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -87,7 +87,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
         expires: bindings::ktime_t,
         fwd: *mut bindings::hrtimer_forward_args,
     ) -> bindings::hrtimer_restart {
-        // `HrTimer` is `repr(C)`
+        // `HrTimer` is `repr(transparent)`
         let timer_ptr = ptr.cast::<HrTimer<T>>();
 
         // SAFETY: By the safety requirement of this function, `timer_ptr`
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 04f9d8cbddcd..2bba3c41d6e9 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -91,7 +91,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
         expires: bindings::ktime_t,
         fwd: *mut bindings::hrtimer_forward_args,
     ) -> bindings::hrtimer_restart {
-        // `HrTimer` is `repr(C)`
+        // `HrTimer` is `repr(transparent)`
         let timer_ptr = ptr.cast::<HrTimer<T>>();
 
         // SAFETY: By the safety requirement of this function, `timer_ptr`
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index c7f86909e21b..399ad7677043 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -107,7 +107,7 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>>
         expires: bindings::ktime_t,
         fwd: *mut bindings::hrtimer_forward_args,
     ) -> bindings::hrtimer_restart {
-        // `HrTimer` is `repr(C)`
+        // `HrTimer` is `repr(transparent)`
         let timer_ptr = ptr.cast::<super::HrTimer<T>>();
 
         // SAFETY: By C API contract `ptr` is the pointer we passed when

-- 
2.51.2

Re: [PATCH 6/6] rust: hrtimer: Make HrTimer repr(transparent)

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Tue Aug 25, 2026 at 1:16 PM BST, Andreas Hindborg wrote:
>> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>
>> HrTimerCallbackContext acquires a &HrTimer<T> from a
>> NonNull<HrTimer<T>> while a &mut HrTimer<T> can exist at the same
>> time. This is sound only because HrTimer's sole field is
>> Opaque<bindings::hrtimer>, which puts every byte behind an UnsafeCell.
>> Adding a field to HrTimer that is not Opaque would make acquiring that
>> shared reference unsound.
>
> HrTimerCallbackContext is removed in patch 4, though?
>
> Still worh preferring `#[repr(transparent)]` over `#[repr(C)]`, but the
> motivation should be reworded and the comment on `HrTimer` should be removed.

Yes, I was a little too fast when I added this patch. I'd like to keep
it as well, but it needs some edits.

Best regards,
Andreas Hindborg

Re: [PATCH 5/6] rust: hrtimer: document deadlock when starting a timer in its handler

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Tue Aug 25, 2026 at 1:16 PM BST, Andreas Hindborg wrote:
>> The state machine documentation states that for pointer types that
>> implement `Clone`, the `start` operation may be issued while the timer
>> is in the **running** state, and that it is then equivalent to the
>> `restart` operation. That only holds when the operation is issued from
>> outside the timer handler.
>>
>> The `start` operation returns a `HrTimerHandle`, and dropping the
>> handle cancels the timer with `hrtimer_cancel()`, which blocks until a
>> running handler has returned. When `start` is issued from within the
>> handler, the handle is also dropped within the handler, so the cancel
>> waits for the very handler that issues it, and the handler deadlocks.
>
> That's not always true, you can start timer and store its handle elsewhere.
>
> I think the proper wording is that "cancelling a timer from within the timer
> handler will lead to deadlock".

Right, I'll update the wording and comment.


Best regards,
Andreas Hindborg