[PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[FUJITA Tomonori]

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

HrTimer::expires() previously read node.expires via a volatile load, which
can race with C-side updates. Rework the API so it is only callable with
exclusive access or from the callback context.

Introduce raw_expires() with an explicit safety contract, switch
HrTimer::expires() to Pin<&mut Self>, add
HrTimerCallbackContext::expires(), and route the read through
hrtimer_get_expires() via a Rust helper.

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>
---
v2:
- Add Fixes and Closes tags
- Fix and improve comments
v1: https://lore.kernel.org/rust-for-linux/20260110115838.3109895-1-fujita.tomonori@gmail.com/
---
 rust/helpers/time.c         |  6 +++++
 rust/kernel/time/hrtimer.rs | 49 +++++++++++++++++++++++++++----------
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/rust/helpers/time.c b/rust/helpers/time.c
index 32f495970493..ef8999621399 100644
--- a/rust/helpers/time.c
+++ b/rust/helpers/time.c
@@ -2,6 +2,7 @@
 
 #include <linux/delay.h>
 #include <linux/ktime.h>
+#include <linux/hrtimer.h>
 #include <linux/timekeeping.h>
 
 __rust_helper void rust_helper_fsleep(unsigned long usecs)
@@ -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 856d2d929a00..78e2343fd016 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -224,27 +224,39 @@ 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
+    ///
+    /// - `self_ptr` must point to a valid `Self`.
+    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
+    ///   within the context of the timer callback.
+    #[inline]
+    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
+    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.
+        // - Timers cannot have negative `ktime_t` values as their expiration time.
+        unsafe { Instant::from_ktime(bindings::hrtimer_get_expires(Self::raw_get(self_ptr))) }
+    }
+
     /// 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>
+    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: `raw_expires` 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>`, the pointer passed to `raw_expires` points to a
+        // valid `Self` that we have exclusive access to.
+        unsafe { Self::raw_expires(this) }
     }
 }
 
@@ -729,6 +741,17 @@ pub fn forward(&mut self, now: HrTimerInstant<T>, interval: Delta) -> u64 {
     pub fn forward_now(&mut self, duration: Delta) -> u64 {
         self.forward(HrTimerInstant::<T>::now(), duration)
     }
+
+    /// Return the time expiry for this [`HrTimer`].
+    ///
+    /// This function is identical to [`HrTimer::expires()`] except that it may only be used from
+    /// within the context of a [`HrTimer`] callback.
+    pub fn expires(&self) -> HrTimerInstant<T> {
+        // 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_expires(self.0.as_ptr()) }
+    }
 }
 
 /// Use to implement the [`HasHrTimer<T>`] trait.

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
-- 
2.43.0

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Gary Guo]

On Tue Feb 24, 2026 at 1:25 PM GMT, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> HrTimer::expires() previously read node.expires via a volatile load, which
> can race with C-side updates. Rework the API so it is only callable with
> exclusive access or from the callback context.
>
> Introduce raw_expires() with an explicit safety contract, switch
> HrTimer::expires() to Pin<&mut Self>, add
> HrTimerCallbackContext::expires(), and route the read through
> hrtimer_get_expires() via a Rust helper.
>
> 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>
> ---
> v2:
> - Add Fixes and Closes tags
> - Fix and improve comments
> v1: https://lore.kernel.org/rust-for-linux/20260110115838.3109895-1-fujita.tomonori@gmail.com/
> ---
>  rust/helpers/time.c         |  6 +++++
>  rust/kernel/time/hrtimer.rs | 49 +++++++++++++++++++++++++++----------
>  2 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
> index 32f495970493..ef8999621399 100644
> --- a/rust/helpers/time.c
> +++ b/rust/helpers/time.c
> @@ -2,6 +2,7 @@
>  
>  #include <linux/delay.h>
>  #include <linux/ktime.h>
> +#include <linux/hrtimer.h>
>  #include <linux/timekeeping.h>
>  
>  __rust_helper void rust_helper_fsleep(unsigned long usecs)
> @@ -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 856d2d929a00..78e2343fd016 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -224,27 +224,39 @@ 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
> +    ///
> +    /// - `self_ptr` must point to a valid `Self`.
> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
> +    ///   within the context of the timer callback.
> +    #[inline]
> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>

I guess the signature of this can also be

`unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
first safety requirement.

Best,
Gary

> +    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.
> +        // - Timers cannot have negative `ktime_t` values as their expiration time.
> +        unsafe { Instant::from_ktime(bindings::hrtimer_get_expires(Self::raw_get(self_ptr))) }
> +    }
> +
>      /// 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>
> +    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: `raw_expires` 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>`, the pointer passed to `raw_expires` points to a
> +        // valid `Self` that we have exclusive access to.
> +        unsafe { Self::raw_expires(this) }
>      }
>  }
>  
> @@ -729,6 +741,17 @@ pub fn forward(&mut self, now: HrTimerInstant<T>, interval: Delta) -> u64 {
>      pub fn forward_now(&mut self, duration: Delta) -> u64 {
>          self.forward(HrTimerInstant::<T>::now(), duration)
>      }
> +
> +    /// Return the time expiry for this [`HrTimer`].
> +    ///
> +    /// This function is identical to [`HrTimer::expires()`] except that it may only be used from
> +    /// within the context of a [`HrTimer`] callback.
> +    pub fn expires(&self) -> HrTimerInstant<T> {
> +        // 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_expires(self.0.as_ptr()) }
> +    }
>  }
>  
>  /// Use to implement the [`HasHrTimer<T>`] trait.
>
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[FUJITA Tomonori]

On Thu, 26 Feb 2026 11:42:39 +0000
"Gary Guo" <gary@garyguo.net> wrote:

>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>> index 856d2d929a00..78e2343fd016 100644
>> --- a/rust/kernel/time/hrtimer.rs
>> +++ b/rust/kernel/time/hrtimer.rs
>> @@ -224,27 +224,39 @@ 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
>> +    ///
>> +    /// - `self_ptr` must point to a valid `Self`.
>> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>> +    ///   within the context of the timer callback.
>> +    #[inline]
>> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
> 
> I guess the signature of this can also be
> 
> `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
> first safety requirement.

Thanks, I've updated the signature to use &self.

Andreas, this breaks the consistency with raw_forward. Is this
acceptable?

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Andreas Hindborg]

FUJITA Tomonori <tomo@aliasing.net> writes:

> On Thu, 26 Feb 2026 11:42:39 +0000
> "Gary Guo" <gary@garyguo.net> wrote:
> 
> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> >> index 856d2d929a00..78e2343fd016 100644
> >> --- a/rust/kernel/time/hrtimer.rs
> >> +++ b/rust/kernel/time/hrtimer.rs
> >> @@ -224,27 +224,39 @@ 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
> >> +    ///
> >> +    /// - `self_ptr` must point to a valid `Self`.
> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
> >> +    ///   within the context of the timer callback.
> >> +    #[inline]
> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
> > 
> > I guess the signature of this can also be
> > 
> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
> > first safety requirement.
> 
> Thanks, I've updated the signature to use &self.
> 
> Andreas, this breaks the consistency with raw_forward. Is this
> acceptable?

If we do this, we get to argue that the NonNull we have in the callback context
is convertible to a reference. Either way is fine with me.

If you change this to a reference, then I think we should rename the function to
`expires_unchecked`. The `raw` usually refers to operations on pointers rather
than references.

Best regards,
Andreas

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Gary Guo]

On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
> FUJITA Tomonori <tomo@aliasing.net> writes:
>
>> On Thu, 26 Feb 2026 11:42:39 +0000
>> "Gary Guo" <gary@garyguo.net> wrote:
>> 
>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>> >> index 856d2d929a00..78e2343fd016 100644
>> >> --- a/rust/kernel/time/hrtimer.rs
>> >> +++ b/rust/kernel/time/hrtimer.rs
>> >> @@ -224,27 +224,39 @@ 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
>> >> +    ///
>> >> +    /// - `self_ptr` must point to a valid `Self`.
>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>> >> +    ///   within the context of the timer callback.
>> >> +    #[inline]
>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>> > 
>> > I guess the signature of this can also be
>> > 
>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>> > first safety requirement.
>> 
>> Thanks, I've updated the signature to use &self.
>> 
>> Andreas, this breaks the consistency with raw_forward. Is this
>> acceptable?
>
> If we do this, we get to argue that the NonNull we have in the callback context
> is convertible to a reference. Either way is fine with me.
>
> If you change this to a reference, then I think we should rename the function to
> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
> than references.

Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?

Best,
Gary

>
> Best regards,
> Andreas

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Andreas Hindborg]

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

> On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
>> FUJITA Tomonori <tomo@aliasing.net> writes:
>>
>>> On Thu, 26 Feb 2026 11:42:39 +0000
>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>
>>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>> >> index 856d2d929a00..78e2343fd016 100644
>>> >> --- a/rust/kernel/time/hrtimer.rs
>>> >> +++ b/rust/kernel/time/hrtimer.rs
>>> >> @@ -224,27 +224,39 @@ 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
>>> >> +    ///
>>> >> +    /// - `self_ptr` must point to a valid `Self`.
>>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>>> >> +    ///   within the context of the timer callback.
>>> >> +    #[inline]
>>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>>> >
>>> > I guess the signature of this can also be
>>> >
>>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>>> > first safety requirement.
>>>
>>> Thanks, I've updated the signature to use &self.
>>>
>>> Andreas, this breaks the consistency with raw_forward. Is this
>>> acceptable?
>>
>> If we do this, we get to argue that the NonNull we have in the callback context
>> is convertible to a reference. Either way is fine with me.
>>
>> If you change this to a reference, then I think we should rename the function to
>> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
>> than references.
>
> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?

I assume you mean `HrTimerCallbackContext`.

Could it be a mut reference? We are supposed to have exclusive access
when we have the context.


Best regards,
Andreas Hindborg

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Gary Guo]

On Thu Feb 26, 2026 at 6:33 PM GMT, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
>>> FUJITA Tomonori <tomo@aliasing.net> writes:
>>>
>>>> On Thu, 26 Feb 2026 11:42:39 +0000
>>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>>
>>>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>> >> index 856d2d929a00..78e2343fd016 100644
>>>> >> --- a/rust/kernel/time/hrtimer.rs
>>>> >> +++ b/rust/kernel/time/hrtimer.rs
>>>> >> @@ -224,27 +224,39 @@ 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
>>>> >> +    ///
>>>> >> +    /// - `self_ptr` must point to a valid `Self`.
>>>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>>>> >> +    ///   within the context of the timer callback.
>>>> >> +    #[inline]
>>>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>>>> >
>>>> > I guess the signature of this can also be
>>>> >
>>>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>>>> > first safety requirement.
>>>>
>>>> Thanks, I've updated the signature to use &self.
>>>>
>>>> Andreas, this breaks the consistency with raw_forward. Is this
>>>> acceptable?
>>>
>>> If we do this, we get to argue that the NonNull we have in the callback context
>>> is convertible to a reference. Either way is fine with me.
>>>
>>> If you change this to a reference, then I think we should rename the function to
>>> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
>>> than references.
>>
>> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?
>
> I assume you mean `HrTimerCallbackContext`.
>
> Could it be a mut reference? We are supposed to have exclusive access
> when we have the context.

If `Pin<&mut Hrtimer>` would work, then we can perhaps remove the
`HrTimerCallbackContext` type and just pass a `Pin<&mut HrTimer>` to the callback?

Best,
Gary

>
>
> Best regards,
> Andreas Hindborg

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[FUJITA Tomonori]

On Fri, 27 Feb 2026 12:32:05 +0000
"Gary Guo" <gary@garyguo.net> wrote:

> On Thu Feb 26, 2026 at 6:33 PM GMT, Andreas Hindborg wrote:
>> "Gary Guo" <gary@garyguo.net> writes:
>>
>>> On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
>>>> FUJITA Tomonori <tomo@aliasing.net> writes:
>>>>
>>>>> On Thu, 26 Feb 2026 11:42:39 +0000
>>>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>>>
>>>>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>> >> index 856d2d929a00..78e2343fd016 100644
>>>>> >> --- a/rust/kernel/time/hrtimer.rs
>>>>> >> +++ b/rust/kernel/time/hrtimer.rs
>>>>> >> @@ -224,27 +224,39 @@ 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
>>>>> >> +    ///
>>>>> >> +    /// - `self_ptr` must point to a valid `Self`.
>>>>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>>>>> >> +    ///   within the context of the timer callback.
>>>>> >> +    #[inline]
>>>>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>>>>> >
>>>>> > I guess the signature of this can also be
>>>>> >
>>>>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>>>>> > first safety requirement.
>>>>>
>>>>> Thanks, I've updated the signature to use &self.
>>>>>
>>>>> Andreas, this breaks the consistency with raw_forward. Is this
>>>>> acceptable?
>>>>
>>>> If we do this, we get to argue that the NonNull we have in the callback context
>>>> is convertible to a reference. Either way is fine with me.
>>>>
>>>> If you change this to a reference, then I think we should rename the function to
>>>> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
>>>> than references.
>>>
>>> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?
>>
>> I assume you mean `HrTimerCallbackContext`.
>>
>> Could it be a mut reference? We are supposed to have exclusive access
>> when we have the context.
> 
> If `Pin<&mut Hrtimer>` would work, then we can perhaps remove the
> `HrTimerCallbackContext` type and just pass a `Pin<&mut HrTimer>` to the callback?

During the callback, ArcTimerHandle still holds Arc<T>, and the
callback receives ArcBorrow<T>. Creating Pin<&mut HrTimer<T>> would
cause aliasing between &HrTimer<T> and &mut HrTimer<T>?

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Gary Guo]

On Sat Feb 28, 2026 at 1:26 AM GMT, FUJITA Tomonori wrote:
> On Fri, 27 Feb 2026 12:32:05 +0000
> "Gary Guo" <gary@garyguo.net> wrote:
>
>> On Thu Feb 26, 2026 at 6:33 PM GMT, Andreas Hindborg wrote:
>>> "Gary Guo" <gary@garyguo.net> writes:
>>>
>>>> On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
>>>>> FUJITA Tomonori <tomo@aliasing.net> writes:
>>>>>
>>>>>> On Thu, 26 Feb 2026 11:42:39 +0000
>>>>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>>>>
>>>>>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>> >> index 856d2d929a00..78e2343fd016 100644
>>>>>> >> --- a/rust/kernel/time/hrtimer.rs
>>>>>> >> +++ b/rust/kernel/time/hrtimer.rs
>>>>>> >> @@ -224,27 +224,39 @@ 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
>>>>>> >> +    ///
>>>>>> >> +    /// - `self_ptr` must point to a valid `Self`.
>>>>>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>>>>>> >> +    ///   within the context of the timer callback.
>>>>>> >> +    #[inline]
>>>>>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>>>>>> >
>>>>>> > I guess the signature of this can also be
>>>>>> >
>>>>>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>>>>>> > first safety requirement.
>>>>>>
>>>>>> Thanks, I've updated the signature to use &self.
>>>>>>
>>>>>> Andreas, this breaks the consistency with raw_forward. Is this
>>>>>> acceptable?
>>>>>
>>>>> If we do this, we get to argue that the NonNull we have in the callback context
>>>>> is convertible to a reference. Either way is fine with me.
>>>>>
>>>>> If you change this to a reference, then I think we should rename the function to
>>>>> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
>>>>> than references.
>>>>
>>>> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?
>>>
>>> I assume you mean `HrTimerCallbackContext`.
>>>
>>> Could it be a mut reference? We are supposed to have exclusive access
>>> when we have the context.
>> 
>> If `Pin<&mut Hrtimer>` would work, then we can perhaps remove the
>> `HrTimerCallbackContext` type and just pass a `Pin<&mut HrTimer>` to the callback?
>
> During the callback, ArcTimerHandle still holds Arc<T>, and the
> callback receives ArcBorrow<T>. Creating Pin<&mut HrTimer<T>> would
> cause aliasing between &HrTimer<T> and &mut HrTimer<T>?

This is fine from an aliasing POV as `Opaque` cancels the noalias requirement
of `&mut` already.

As long as we can guarantee that no method on `Pin<&mut HrTimer<T>>` can race
with methods on `&HrTimer<T>` it should be okay.

Best,
Gary

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[FUJITA Tomonori]

On Sat, 28 Feb 2026 13:02:10 +0000
"Gary Guo" <gary@garyguo.net> wrote:

> On Sat Feb 28, 2026 at 1:26 AM GMT, FUJITA Tomonori wrote:
>> On Fri, 27 Feb 2026 12:32:05 +0000
>> "Gary Guo" <gary@garyguo.net> wrote:
>>
>>> On Thu Feb 26, 2026 at 6:33 PM GMT, Andreas Hindborg wrote:
>>>> "Gary Guo" <gary@garyguo.net> writes:
>>>>
>>>>> On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
>>>>>> FUJITA Tomonori <tomo@aliasing.net> writes:
>>>>>>
>>>>>>> On Thu, 26 Feb 2026 11:42:39 +0000
>>>>>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>>>>>
>>>>>>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>>> >> index 856d2d929a00..78e2343fd016 100644
>>>>>>> >> --- a/rust/kernel/time/hrtimer.rs
>>>>>>> >> +++ b/rust/kernel/time/hrtimer.rs
>>>>>>> >> @@ -224,27 +224,39 @@ 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
>>>>>>> >> +    ///
>>>>>>> >> +    /// - `self_ptr` must point to a valid `Self`.
>>>>>>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>>>>>>> >> +    ///   within the context of the timer callback.
>>>>>>> >> +    #[inline]
>>>>>>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>>>>>>> >
>>>>>>> > I guess the signature of this can also be
>>>>>>> >
>>>>>>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>>>>>>> > first safety requirement.
>>>>>>>
>>>>>>> Thanks, I've updated the signature to use &self.
>>>>>>>
>>>>>>> Andreas, this breaks the consistency with raw_forward. Is this
>>>>>>> acceptable?
>>>>>>
>>>>>> If we do this, we get to argue that the NonNull we have in the callback context
>>>>>> is convertible to a reference. Either way is fine with me.
>>>>>>
>>>>>> If you change this to a reference, then I think we should rename the function to
>>>>>> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
>>>>>> than references.
>>>>>
>>>>> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?
>>>>
>>>> I assume you mean `HrTimerCallbackContext`.
>>>>
>>>> Could it be a mut reference? We are supposed to have exclusive access
>>>> when we have the context.
>>> 
>>> If `Pin<&mut Hrtimer>` would work, then we can perhaps remove the
>>> `HrTimerCallbackContext` type and just pass a `Pin<&mut HrTimer>` to the callback?
>>
>> During the callback, ArcTimerHandle still holds Arc<T>, and the
>> callback receives ArcBorrow<T>. Creating Pin<&mut HrTimer<T>> would
>> cause aliasing between &HrTimer<T> and &mut HrTimer<T>?
> 
> This is fine from an aliasing POV as `Opaque` cancels the noalias requirement
> of `&mut` already.
> 
> As long as we can guarantee that no method on `Pin<&mut HrTimer<T>>` can race
> with methods on `&HrTimer<T>` it should be okay.

If that's the case, we may be able to remove HrTimerCallbackContext
entirely.

Andreas, what do you think?

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Andreas Hindborg]

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

> On Sat Feb 28, 2026 at 1:26 AM GMT, FUJITA Tomonori wrote:
>> On Fri, 27 Feb 2026 12:32:05 +0000
>> "Gary Guo" <gary@garyguo.net> wrote:
>>
>>> On Thu Feb 26, 2026 at 6:33 PM GMT, Andreas Hindborg wrote:
>>>> "Gary Guo" <gary@garyguo.net> writes:
>>>>
>>>>> On Thu Feb 26, 2026 at 1:19 PM GMT, Andreas Hindborg wrote:
>>>>>> FUJITA Tomonori <tomo@aliasing.net> writes:
>>>>>>
>>>>>>> On Thu, 26 Feb 2026 11:42:39 +0000
>>>>>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>>>>>
>>>>>>> >> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>>> >> index 856d2d929a00..78e2343fd016 100644
>>>>>>> >> --- a/rust/kernel/time/hrtimer.rs
>>>>>>> >> +++ b/rust/kernel/time/hrtimer.rs
>>>>>>> >> @@ -224,27 +224,39 @@ 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
>>>>>>> >> +    ///
>>>>>>> >> +    /// - `self_ptr` must point to a valid `Self`.
>>>>>>> >> +    /// - The caller must either have exclusive access to the data pointed to by `self_ptr`, or be
>>>>>>> >> +    ///   within the context of the timer callback.
>>>>>>> >> +    #[inline]
>>>>>>> >> +    unsafe fn raw_expires(self_ptr: *const Self) -> HrTimerInstant<T>
>>>>>>> >
>>>>>>> > I guess the signature of this can also be
>>>>>>> >
>>>>>>> > `unsafe fn raw_expires(&self) -> HrTimerInstant<T>`? Then you can remove the
>>>>>>> > first safety requirement.
>>>>>>>
>>>>>>> Thanks, I've updated the signature to use &self.
>>>>>>>
>>>>>>> Andreas, this breaks the consistency with raw_forward. Is this
>>>>>>> acceptable?
>>>>>>
>>>>>> If we do this, we get to argue that the NonNull we have in the callback context
>>>>>> is convertible to a reference. Either way is fine with me.
>>>>>>
>>>>>> If you change this to a reference, then I think we should rename the function to
>>>>>> `expires_unchecked`. The `raw` usually refers to operations on pointers rather
>>>>>> than references.
>>>>>
>>>>> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?
>>>>
>>>> I assume you mean `HrTimerCallbackContext`.
>>>>
>>>> Could it be a mut reference? We are supposed to have exclusive access
>>>> when we have the context.
>>> 
>>> If `Pin<&mut Hrtimer>` would work, then we can perhaps remove the
>>> `HrTimerCallbackContext` type and just pass a `Pin<&mut HrTimer>` to the callback?
>>
>> During the callback, ArcTimerHandle still holds Arc<T>, and the
>> callback receives ArcBorrow<T>. Creating Pin<&mut HrTimer<T>> would
>> cause aliasing between &HrTimer<T> and &mut HrTimer<T>?
>
> This is fine from an aliasing POV as `Opaque` cancels the noalias requirement
> of `&mut` already.
>
> As long as we can guarantee that no method on `Pin<&mut HrTimer<T>>` can race
> with methods on `&HrTimer<T>` it should be okay.

Is this really OK?

struct Foo {
  timer: HrTimer<Foo>,
}

Surely it cannot be OK to hold both `&Foo` and `Pin<&mut HrTimer<Foo>>`,
even though the internals of `HrTimer<_>` is an `Opaque` field?

I understand that it would be OK if we had a `Pin<&mut
Opaque<bindings::hrtimer>` referencing inside `Foo`, but the other one
really breaks my mental model.

Even if it works out with the current types, if we add a field to
`HrTimer`, stuff breaks.


Best regards,
Andreas Hindborg

Re: [PATCH v2] rust: hrtimer: Restrict expires() to safe contexts

[Gary Guo]

On Tue Mar 10, 2026 at 8:24 AM GMT, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Sat Feb 28, 2026 at 1:26 AM GMT, FUJITA Tomonori wrote:
>>> On Fri, 27 Feb 2026 12:32:05 +0000
>>> "Gary Guo" <gary@garyguo.net> wrote:
>>>
>>>> On Thu Feb 26, 2026 at 6:33 PM GMT, Andreas Hindborg wrote:
>>>>> "Gary Guo" <gary@garyguo.net> writes:
>>>>>
>>>>>> Does it make sense to simply have `HrTimerCallback<'a, T: ...>(&'a HrTimer<T>)`?
>>>>>
>>>>> I assume you mean `HrTimerCallbackContext`.
>>>>>
>>>>> Could it be a mut reference? We are supposed to have exclusive access
>>>>> when we have the context.
>>>> 
>>>> If `Pin<&mut Hrtimer>` would work, then we can perhaps remove the
>>>> `HrTimerCallbackContext` type and just pass a `Pin<&mut HrTimer>` to the callback?
>>>
>>> During the callback, ArcTimerHandle still holds Arc<T>, and the
>>> callback receives ArcBorrow<T>. Creating Pin<&mut HrTimer<T>> would
>>> cause aliasing between &HrTimer<T> and &mut HrTimer<T>?
>>
>> This is fine from an aliasing POV as `Opaque` cancels the noalias requirement
>> of `&mut` already.
>>
>> As long as we can guarantee that no method on `Pin<&mut HrTimer<T>>` can race
>> with methods on `&HrTimer<T>` it should be okay.
>
> Is this really OK?
>
> struct Foo {
>   timer: HrTimer<Foo>,
> }
>
> Surely it cannot be OK to hold both `&Foo` and `Pin<&mut HrTimer<Foo>>`,
> even though the internals of `HrTimer<_>` is an `Opaque` field?

It's not UB to hold both, otherwise you cannot write intrusive data structures.

It might be useful to think the mutable reference that the hrtime callback
receives is th same as getting a reference to mutex content, while others can
still reference the mutex.

We can probably have two types, where one can have shared refs (for starting the
timer) and the other is exclusive only?

Best,
Gary