linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 1/7] rust: hrtimer: Document the return value for HrTimerHandle::cancel()
       [not found] <20250821193259.964504-1-lyude@redhat.com>
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 2/7] rust: hrtimer: Add HrTimerInstant Lyude Paul
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Andreas Hindborg, Daniel Almeida, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

Just a drive-by fix I noticed: we don't actually document what the return
value from cancel() does, so do that.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

---
V4:
* Reword to "Returns `true` if the timer was running."

 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 144e3b57cc780..6bfc0223f4f57 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -324,6 +324,8 @@ pub unsafe trait HrTimerHandle {
     /// Note that the timer might be started by a concurrent start operation. If
     /// so, the timer might not be in the **stopped** state when this function
     /// returns.
+    ///
+    /// Returns `true` if the timer was running.
     fn cancel(&mut self) -> bool;
 }
 
-- 
2.50.0


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

* [PATCH v9 2/7] rust: hrtimer: Add HrTimerInstant
       [not found] <20250821193259.964504-1-lyude@redhat.com>
  2025-08-21 19:32 ` [PATCH v9 1/7] rust: hrtimer: Document the return value for HrTimerHandle::cancel() Lyude Paul
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 3/7] rust: hrtimer: Add HrTimer::raw_forward() and forward() Lyude Paul
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Andreas Hindborg, Daniel Almeida, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

Since we want to add HrTimer methods that can accept Instants, we will want
to make sure that for each method we are using the correct Clocksource for
the given HrTimer. This would get a bit overly-verbose, so add a simple
HrTimerInstant type-alias to handle this for us.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/time/hrtimer.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 6bfc0223f4f57..be1bad4aacaad 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -72,6 +72,11 @@
 use core::marker::PhantomData;
 use pin_init::PinInit;
 
+/// A type-alias to refer to the [`Instant<C>`] for a given `T` from [`HrTimer<T>`].
+///
+/// Where `C` is the [`ClockSource`] of the [`HrTimer`].
+pub type HrTimerInstant<T> = Instant<<<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Clock>;
+
 /// A timer backed by a C `struct hrtimer`.
 ///
 /// # Invariants
-- 
2.50.0


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

* [PATCH v9 3/7] rust: hrtimer: Add HrTimer::raw_forward() and forward()
       [not found] <20250821193259.964504-1-lyude@redhat.com>
  2025-08-21 19:32 ` [PATCH v9 1/7] rust: hrtimer: Document the return value for HrTimerHandle::cancel() Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 2/7] rust: hrtimer: Add HrTimerInstant Lyude Paul
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 4/7] rust: hrtimer: Add HrTimerCallbackContext and ::forward() Lyude Paul
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Daniel Almeida, Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

Within the hrtimer API there are quite a number of functions that can only
be safely called from one of two contexts:

* When we have exclusive access to the hrtimer and the timer is not active.
* When we're within the hrtimer's callback context as it is being executed.

This commit adds bindings for hrtimer_forward() for the first such context,
along with HrTimer::raw_forward() for later use in implementing the
hrtimer_forward() in the latter context.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

---
V4:
* Fix the safety contract for raw_forward()
* Require Pin<&mut Self>, not &mut self
* Drop incorrect UniquePin example
* Rewrite documentation a bit (re: Andreas)
V6:
* Remove the reference to HrTimerCallbackContext::forward() until this
  function gets added.
V7
* Split up Timer::forward() a bit, apply Andreas's SAFETY comment
  recommendations

 rust/kernel/time/hrtimer.rs | 40 +++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index be1bad4aacaad..79fed14b2d98e 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -168,6 +168,46 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
         // handled on the C side.
         unsafe { bindings::hrtimer_cancel(c_timer_ptr) != 0 }
     }
+
+    /// Forward the timer expiry for a given timer pointer.
+    ///
+    /// # 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.
+    #[inline]
+    unsafe fn raw_forward(self_ptr: *mut Self, now: HrTimerInstant<T>, interval: Delta) -> u64
+    where
+        T: HasHrTimer<T>,
+    {
+        // SAFETY:
+        // * The C API requirements for this function are fulfilled by our safety contract.
+        // * `self_ptr` is guaranteed to point to a valid `Self` via our safety contract
+        unsafe {
+            bindings::hrtimer_forward(Self::raw_get(self_ptr), now.as_nanos(), interval.as_nanos())
+        }
+    }
+
+    /// Conditionally forward the timer.
+    ///
+    /// If the timer expires after `now`, this function does nothing and returns 0. If the timer
+    /// expired at or before `now`, this function forwards the timer by `interval` until the timer
+    /// expires after `now` and then returns the number of times the timer was forwarded by
+    /// `interval`.
+    ///
+    /// 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
+    where
+        T: HasHrTimer<T>,
+    {
+        // SAFETY: `raw_forward` does not move `Self`
+        let this = unsafe { self.get_unchecked_mut() };
+
+        // SAFETY: By existence of `Pin<&mut Self>`, the pointer passed to `raw_forward` points to a
+        // valid `Self` that we have exclusive access to.
+        unsafe { Self::raw_forward(this, now, interval) }
+    }
 }
 
 /// Implemented by pointer types that point to structs that contain a [`HrTimer`].
-- 
2.50.0


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

* [PATCH v9 4/7] rust: hrtimer: Add HrTimerCallbackContext and ::forward()
       [not found] <20250821193259.964504-1-lyude@redhat.com>
                   ` (2 preceding siblings ...)
  2025-08-21 19:32 ` [PATCH v9 3/7] rust: hrtimer: Add HrTimer::raw_forward() and forward() Lyude Paul
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 5/7] rust: hrtimer: Add forward_now() to HrTimer and HrTimerCallbackContext Lyude Paul
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Daniel Almeida, Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

With Linux's hrtimer API, there's a number of methods that can only be
called in two situations:

* When we have exclusive access to the hrtimer and it is not currently
  active
* When we're within the context of an hrtimer callback context

This commit handles the second situation and implements hrtimer_forward()
support in the context of a timer callback. We do this by introducing a
HrTimerCallbackContext type which is provided to users during the
RawHrTimerCallback::run() callback, and then add a forward() function to
the type.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

---
V2:
* Improve SAFETY comments for HrTimerCallbackContext uses (I forgot to
  mention that we're within RawHrTimerCallback::run()
* Split forward into forward() and raw_forward() since we're going to have
  two contexts that we can call forward() from now.
* Clarify contexts in which certain hrtimer methods can be called.
* Make sure that we use a mutable reference for forward() here - just in
  case :).
* Rename interval to duration
V3:
* Rename duration -back- to interval (now that I actually have read
  hrtimer_forward's source, interval does make more sense than duration
  considering the fact we return the number of overruns that occurred
  according to the given interval).
* Rewrite documentation a bit (re: Andreas)
V5:
* Fix unbounded T on HrTimerCallbackContext
V6:
* Move reference to HrTimerCallbackContext::forward() in HrTimer::forward()
  comments into this commit so rustdoc doesn't fail.
* Deduplicate documentation for HrTimerCallbackContext::forward()
* Add missing changelog note

 rust/kernel/time/hrtimer.rs         | 63 +++++++++++++++++++++++++++--
 rust/kernel/time/hrtimer/arc.rs     |  9 ++++-
 rust/kernel/time/hrtimer/pin.rs     |  9 ++++-
 rust/kernel/time/hrtimer/pin_mut.rs | 12 ++++--
 rust/kernel/time/hrtimer/tbox.rs    |  9 ++++-
 5 files changed, 93 insertions(+), 9 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 79fed14b2d98e..1e8839d277292 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -69,7 +69,7 @@
 
 use super::{ClockSource, Delta, Instant};
 use crate::{prelude::*, types::Opaque};
-use core::marker::PhantomData;
+use core::{marker::PhantomData, ptr::NonNull};
 use pin_init::PinInit;
 
 /// A type-alias to refer to the [`Instant<C>`] for a given `T` from [`HrTimer<T>`].
@@ -196,6 +196,10 @@ unsafe fn raw_forward(self_ptr: *mut Self, now: HrTimerInstant<T>, interval: Del
     /// expires after `now` and then returns the number of times the timer was forwarded by
     /// `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()`].
+    ///
     /// 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
     where
@@ -345,9 +349,13 @@ pub trait HrTimerCallback {
     type Pointer<'a>: RawHrTimerCallback;
 
     /// Called by the timer logic when the timer fires.
-    fn run(this: <Self::Pointer<'_> as RawHrTimerCallback>::CallbackTarget<'_>) -> HrTimerRestart
+    fn run(
+        this: <Self::Pointer<'_> as RawHrTimerCallback>::CallbackTarget<'_>,
+        ctx: HrTimerCallbackContext<'_, Self>,
+    ) -> HrTimerRestart
     where
-        Self: Sized;
+        Self: Sized,
+        Self: HasHrTimer<Self>;
 }
 
 /// A handle representing a potentially running timer.
@@ -632,6 +640,55 @@ 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) }
+    }
+}
+
 /// 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 ed490a7a89503..7be82bcb352ac 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -3,6 +3,7 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
+use super::HrTimerCallbackContext;
 use super::HrTimerHandle;
 use super::HrTimerMode;
 use super::HrTimerPointer;
@@ -99,6 +100,12 @@ impl<T> RawHrTimerCallback for Arc<T>
         //    allocation from other `Arc` clones.
         let receiver = unsafe { ArcBorrow::from_raw(data_ptr) };
 
-        T::run(receiver).into_c()
+        // 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) };
+
+        T::run(receiver, context).into_c()
     }
 }
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index aef16d9ee2f0c..4d39ef7816971 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -3,6 +3,7 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
+use super::HrTimerCallbackContext;
 use super::HrTimerHandle;
 use super::HrTimerMode;
 use super::RawHrTimerCallback;
@@ -103,6 +104,12 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
         // here.
         let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
 
-        T::run(receiver_pin).into_c()
+        // 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) };
+
+        T::run(receiver_pin, context).into_c()
     }
 }
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 767d0a4e8a2c1..9d9447d4d57e8 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -1,8 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{
-    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, HrTimerMode, RawHrTimerCallback,
-    UnsafeHrTimerPointer,
+    HasHrTimer, HrTimer, HrTimerCallback, HrTimerCallbackContext, HrTimerHandle, HrTimerMode,
+    RawHrTimerCallback, UnsafeHrTimerPointer,
 };
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
 
@@ -107,6 +107,12 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
         // here.
         let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
 
-        T::run(receiver_pin).into_c()
+        // 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) };
+
+        T::run(receiver_pin, context).into_c()
     }
 }
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index ec08303315f28..aa1ee31a71953 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -3,6 +3,7 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
+use super::HrTimerCallbackContext;
 use super::HrTimerHandle;
 use super::HrTimerMode;
 use super::HrTimerPointer;
@@ -119,6 +120,12 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>>
         //   `data_ptr` exist.
         let data_mut_ref = unsafe { Pin::new_unchecked(&mut *data_ptr) };
 
-        T::run(data_mut_ref).into_c()
+        // 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) };
+
+        T::run(data_mut_ref, context).into_c()
     }
 }
-- 
2.50.0


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

* [PATCH v9 5/7] rust: hrtimer: Add forward_now() to HrTimer and HrTimerCallbackContext
       [not found] <20250821193259.964504-1-lyude@redhat.com>
                   ` (3 preceding siblings ...)
  2025-08-21 19:32 ` [PATCH v9 4/7] rust: hrtimer: Add HrTimerCallbackContext and ::forward() Lyude Paul
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 6/7] rust: time: Add Instant::from_ktime() Lyude Paul
  2025-08-21 19:32 ` [PATCH v9 7/7] rust: hrtimer: Add HrTimer::expires() Lyude Paul
  6 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Daniel Almeida, Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

---
V2:
* Change from Ktime to Delta
* Make sure that forward_now() takes a mutable reference to the timer
  struct
* Reword this to point out that we're adding forward_now() to both callback
  context and mutable timer reference
* Rename interval to duration
V4:
* Fix rust documentation for HrTimerCallbackContext (forgot to update both
  forward_now() declarations)
* Use Pin<&mut Self> for context-less forward.
V6:
* Drop raw_cb_time(), use Instant::now() instead
* Split out expires() from this patch, at some point it seems I mistakenly
  combined it with this patch
V7:
* Remove leftover comment about raw_cb_time from patch description

 rust/kernel/time/hrtimer.rs | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 1e8839d277292..e0d78a8859903 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -212,6 +212,17 @@ pub fn forward(self: Pin<&mut Self>, now: HrTimerInstant<T>, interval: Delta) ->
         // valid `Self` that we have exclusive access to.
         unsafe { Self::raw_forward(this, now, interval) }
     }
+
+    /// Conditionally forward the timer.
+    ///
+    /// This is a variant of [`forward()`](Self::forward) that uses an interval after the current
+    /// time of the base clock for the [`HrTimer`].
+    pub fn forward_now(self: Pin<&mut Self>, interval: Delta) -> u64
+    where
+        T: HasHrTimer<T>,
+    {
+        self.forward(HrTimerInstant::<T>::now(), interval)
+    }
 }
 
 /// Implemented by pointer types that point to structs that contain a [`HrTimer`].
@@ -687,6 +698,14 @@ pub fn forward(&mut self, now: HrTimerInstant<T>, interval: Delta) -> u64 {
         // - 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.
-- 
2.50.0


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

* [PATCH v9 6/7] rust: time: Add Instant::from_ktime()
       [not found] <20250821193259.964504-1-lyude@redhat.com>
                   ` (4 preceding siblings ...)
  2025-08-21 19:32 ` [PATCH v9 5/7] rust: hrtimer: Add forward_now() to HrTimer and HrTimerCallbackContext Lyude Paul
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-21 23:10   ` FUJITA Tomonori
                     ` (2 more replies)
  2025-08-21 19:32 ` [PATCH v9 7/7] rust: hrtimer: Add HrTimer::expires() Lyude Paul
  6 siblings, 3 replies; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

For implementing Rust bindings which can return a point in time.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V4:
* Turn from_nanos() into an unsafe function in order to ensure that we
  uphold the invariants of Instant
V5:
* Add debug_assert!() to from_nanos
V8:
* Change name of function from Instant::from_nanos() to
  Instant::from_ktime()
V9:
* Fix outdated comments in from_ktime (nanos -> ktime)

 rust/kernel/time.rs | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 64c8dcf548d63..874a1023dcdf9 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -200,6 +200,29 @@ pub fn elapsed(&self) -> Delta {
     pub(crate) fn as_nanos(&self) -> i64 {
         self.inner
     }
+
+    /// Create an [`Instant`] from a `ktime_t` without checking if it is non-negative.
+    ///
+    /// # Panics
+    ///
+    /// On debug builds, this function will panic if `ktime` is not in the range from 0 to
+    /// `KTIME_MAX`.
+    ///
+    /// # Safety
+    ///
+    /// The caller promises that `ktime` is in the range from 0 to `KTIME_MAX`.
+    #[expect(unused)]
+    #[inline]
+    pub(crate) unsafe fn from_ktime(ktime: bindings::ktime_t) -> Self {
+        debug_assert!(ktime >= 0);
+
+        // INVARIANT: Our safety contract ensures that `ktime` is in the range from 0 to
+        // `KTIME_MAX`.
+        Self {
+            inner: ktime,
+            _c: PhantomData,
+        }
+    }
 }
 
 impl<C: ClockSource> core::ops::Sub for Instant<C> {
-- 
2.50.0


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

* [PATCH v9 7/7] rust: hrtimer: Add HrTimer::expires()
       [not found] <20250821193259.964504-1-lyude@redhat.com>
                   ` (5 preceding siblings ...)
  2025-08-21 19:32 ` [PATCH v9 6/7] rust: time: Add Instant::from_ktime() Lyude Paul
@ 2025-08-21 19:32 ` Lyude Paul
  2025-08-26  8:43   ` Andreas Hindborg
  6 siblings, 1 reply; 12+ messages in thread
From: Lyude Paul @ 2025-08-21 19:32 UTC (permalink / raw)
  Cc: Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

Add a simple callback for retrieving the current expiry time for an
HrTimer. In rvkms, we use the HrTimer expiry value in order to calculate
the approximate vblank timestamp during each emulated vblank interrupt.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V8:
* Fix bogus safety comment I noticed after Fujita's comments. In expires()
  we're not guaranteed to get a non-negative ktime_t because of ktime_t
  itself, we're guaranteed to get one because a negative expiration time
  for a timer doesn't make sense.

 rust/kernel/time.rs         |  1 -
 rust/kernel/time/hrtimer.rs | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 874a1023dcdf9..7320d8715bcc2 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -211,7 +211,6 @@ pub(crate) fn as_nanos(&self) -> i64 {
     /// # Safety
     ///
     /// The caller promises that `ktime` is in the range from 0 to `KTIME_MAX`.
-    #[expect(unused)]
     #[inline]
     pub(crate) unsafe fn from_ktime(ktime: bindings::ktime_t) -> Self {
         debug_assert!(ktime >= 0);
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index e0d78a8859903..856d2d929a008 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -223,6 +223,29 @@ pub fn forward_now(self: Pin<&mut Self>, interval: Delta) -> u64
     {
         self.forward(HrTimerInstant::<T>::now(), interval)
     }
+
+    /// 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>
+    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:
+        // - 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)),
+            )
+        }
+    }
 }
 
 /// Implemented by pointer types that point to structs that contain a [`HrTimer`].
-- 
2.50.0


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

* Re: [PATCH v9 6/7] rust: time: Add Instant::from_ktime()
  2025-08-21 19:32 ` [PATCH v9 6/7] rust: time: Add Instant::from_ktime() Lyude Paul
@ 2025-08-21 23:10   ` FUJITA Tomonori
  2025-08-22  7:46   ` Alice Ryhl
  2025-08-26  8:45   ` Andreas Hindborg
  2 siblings, 0 replies; 12+ messages in thread
From: FUJITA Tomonori @ 2025-08-21 23:10 UTC (permalink / raw)
  To: lyude
  Cc: a.hindborg, boqun.feng, fujita.tomonori, frederic, tglx,
	anna-maria, jstultz, sboyd, ojeda, alex.gaynor, gary, bjorn3_gh,
	lossin, aliceryhl, tmgross, dakr, rust-for-linux, linux-kernel

On Thu, 21 Aug 2025 15:32:46 -0400
Lyude Paul <lyude@redhat.com> wrote:

> For implementing Rust bindings which can return a point in time.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> 
> ---
> V4:
> * Turn from_nanos() into an unsafe function in order to ensure that we
>   uphold the invariants of Instant
> V5:
> * Add debug_assert!() to from_nanos
> V8:
> * Change name of function from Instant::from_nanos() to
>   Instant::from_ktime()
> V9:
> * Fix outdated comments in from_ktime (nanos -> ktime)
> 
>  rust/kernel/time.rs | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>


Thanks!

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

* Re: [PATCH v9 6/7] rust: time: Add Instant::from_ktime()
  2025-08-21 19:32 ` [PATCH v9 6/7] rust: time: Add Instant::from_ktime() Lyude Paul
  2025-08-21 23:10   ` FUJITA Tomonori
@ 2025-08-22  7:46   ` Alice Ryhl
  2025-08-26  8:20     ` Andreas Hindborg
  2025-08-26  8:45   ` Andreas Hindborg
  2 siblings, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2025-08-22  7:46 UTC (permalink / raw)
  To: Lyude Paul
  Cc: Andreas Hindborg, Boqun Feng, FUJITA Tomonori,
	Frederic Weisbecker, Thomas Gleixner, Anna-Maria Behnsen,
	John Stultz, Stephen Boyd, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Trevor Gross,
	Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

On Thu, Aug 21, 2025 at 9:34 PM Lyude Paul <lyude@redhat.com> wrote:
>
> For implementing Rust bindings which can return a point in time.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> V4:
> * Turn from_nanos() into an unsafe function in order to ensure that we
>   uphold the invariants of Instant
> V5:
> * Add debug_assert!() to from_nanos
> V8:
> * Change name of function from Instant::from_nanos() to
>   Instant::from_ktime()
> V9:
> * Fix outdated comments in from_ktime (nanos -> ktime)
>
>  rust/kernel/time.rs | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 64c8dcf548d63..874a1023dcdf9 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -200,6 +200,29 @@ pub fn elapsed(&self) -> Delta {
>      pub(crate) fn as_nanos(&self) -> i64 {
>          self.inner
>      }
> +
> +    /// Create an [`Instant`] from a `ktime_t` without checking if it is non-negative.
> +    ///
> +    /// # Panics
> +    ///
> +    /// On debug builds, this function will panic if `ktime` is not in the range from 0 to
> +    /// `KTIME_MAX`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller promises that `ktime` is in the range from 0 to `KTIME_MAX`.
> +    #[expect(unused)]

I would consider just making it pub to avoid the need for this expect.

> +    #[inline]
> +    pub(crate) unsafe fn from_ktime(ktime: bindings::ktime_t) -> Self {
> +        debug_assert!(ktime >= 0);

If you're going to have a debug assertion, perhaps it would make sense
to check both bounds?

Alice

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

* Re: [PATCH v9 6/7] rust: time: Add Instant::from_ktime()
  2025-08-22  7:46   ` Alice Ryhl
@ 2025-08-26  8:20     ` Andreas Hindborg
  0 siblings, 0 replies; 12+ messages in thread
From: Andreas Hindborg @ 2025-08-26  8:20 UTC (permalink / raw)
  To: Alice Ryhl, Lyude Paul
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Thomas Gleixner,
	Anna-Maria Behnsen, John Stultz, Stephen Boyd, Miguel Ojeda,
	Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
	Trevor Gross, Danilo Krummrich,
	open list:DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST], open list

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Thu, Aug 21, 2025 at 9:34 PM Lyude Paul <lyude@redhat.com> wrote:
>>

<cut>

>
>> +    #[inline]
>> +    pub(crate) unsafe fn from_ktime(ktime: bindings::ktime_t) -> Self {
>> +        debug_assert!(ktime >= 0);
>
> If you're going to have a debug assertion, perhaps it would make sense
> to check both bounds?

The upper bound is bindings::ktime_t::MAX, so it would be a useless check.


Best regards,
Andreas Hindborg






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

* Re: [PATCH v9 7/7] rust: hrtimer: Add HrTimer::expires()
  2025-08-21 19:32 ` [PATCH v9 7/7] rust: hrtimer: Add HrTimer::expires() Lyude Paul
@ 2025-08-26  8:43   ` Andreas Hindborg
  0 siblings, 0 replies; 12+ messages in thread
From: Andreas Hindborg @ 2025-08-26  8:43 UTC (permalink / raw)
  To: Lyude Paul
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Thomas Gleixner,
	Anna-Maria Behnsen, John Stultz, Stephen Boyd, Miguel Ojeda,
	Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel

"Lyude Paul" <lyude@redhat.com> writes:

> Add a simple callback for retrieving the current expiry time for an
> HrTimer. In rvkms, we use the HrTimer expiry value in order to calculate
> the approximate vblank timestamp during each emulated vblank interrupt.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg




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

* Re: [PATCH v9 6/7] rust: time: Add Instant::from_ktime()
  2025-08-21 19:32 ` [PATCH v9 6/7] rust: time: Add Instant::from_ktime() Lyude Paul
  2025-08-21 23:10   ` FUJITA Tomonori
  2025-08-22  7:46   ` Alice Ryhl
@ 2025-08-26  8:45   ` Andreas Hindborg
  2 siblings, 0 replies; 12+ messages in thread
From: Andreas Hindborg @ 2025-08-26  8:45 UTC (permalink / raw)
  To: Lyude Paul
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Thomas Gleixner,
	Anna-Maria Behnsen, John Stultz, Stephen Boyd, Miguel Ojeda,
	Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel

"Lyude Paul" <lyude@redhat.com> writes:

> For implementing Rust bindings which can return a point in time.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg




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

end of thread, other threads:[~2025-08-26  8:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250821193259.964504-1-lyude@redhat.com>
2025-08-21 19:32 ` [PATCH v9 1/7] rust: hrtimer: Document the return value for HrTimerHandle::cancel() Lyude Paul
2025-08-21 19:32 ` [PATCH v9 2/7] rust: hrtimer: Add HrTimerInstant Lyude Paul
2025-08-21 19:32 ` [PATCH v9 3/7] rust: hrtimer: Add HrTimer::raw_forward() and forward() Lyude Paul
2025-08-21 19:32 ` [PATCH v9 4/7] rust: hrtimer: Add HrTimerCallbackContext and ::forward() Lyude Paul
2025-08-21 19:32 ` [PATCH v9 5/7] rust: hrtimer: Add forward_now() to HrTimer and HrTimerCallbackContext Lyude Paul
2025-08-21 19:32 ` [PATCH v9 6/7] rust: time: Add Instant::from_ktime() Lyude Paul
2025-08-21 23:10   ` FUJITA Tomonori
2025-08-22  7:46   ` Alice Ryhl
2025-08-26  8:20     ` Andreas Hindborg
2025-08-26  8:45   ` Andreas Hindborg
2025-08-21 19:32 ` [PATCH v9 7/7] rust: hrtimer: Add HrTimer::expires() Lyude Paul
2025-08-26  8:43   ` Andreas Hindborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).