[PATCH v4 1/2] rust: hrtimer: Restrict expires() to safe contexts

[PATCH v4 1/2] 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 expires_unchecked() 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>
---
v4
- Add a patch to make HrTimer repr(transparent)
v3: https://lore.kernel.org/all/20260303061000.73970-1-tomo@aliasing.net/
- Change the signature of raw_expires()
- Rename raw_expires() to expires_unchecked()
v2: https://lore.kernel.org/rust-for-linux/20260224132507.315637-1-tomo@aliasing.net/
- 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 | 46 ++++++++++++++++++++++++++-----------
 2 files changed, 39 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 2d7f1131a813..1db84cd4cbe8 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -560,27 +560,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 either have exclusive access to `self`, or be within the context of the
+    /// timer callback.
+    #[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>
+    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() }
     }
 }
 
@@ -1065,6 +1074,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 the timer.
+    ///
+    /// 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 { self.0.as_ref().expires_unchecked() }
+    }
 }
 
 /// Use to implement the [`HasHrTimer<T>`] trait.

base-commit: dc01dfb37b34beeefcfe1c3055364d41a4070c7e
-- 
2.43.0

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

[FUJITA Tomonori]

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>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 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 1db84cd4cbe8..04f47af3541b 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -418,8 +418,12 @@
 /// # Invariants
 ///
 /// * `self.timer` is initialized by `bindings::hrtimer_setup`.
+// `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 7be82bcb352a..09f748f2f28c 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -80,7 +80,7 @@ impl<T> RawHrTimerCallback for Arc<T>
     type CallbackTarget<'a> = ArcBorrow<'a, T>;
 
     unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> 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 4d39ef781697..e86dfc63eb97 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -83,7 +83,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a T>
     type CallbackTarget<'b> = Self;
 
     unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> 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 9d9447d4d57e..65172c9e55e9 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -86,7 +86,7 @@ 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 {
-        // `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 aa1ee31a7195..1dd68fcf2bd6 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -103,7 +103,7 @@ 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 {
-        // `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.43.0

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

[Andreas Hindborg]

"FUJITA Tomonori" <tomo@flapping.org> writes:

> 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>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  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 1db84cd4cbe8..04f47af3541b 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -418,8 +418,12 @@
>  /// # Invariants
>  ///
>  /// * `self.timer` is initialized by `bindings::hrtimer_setup`.
> +// `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`.

Missing bullet. With that fixed:

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

Best regards,
Andreas Hindborg

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

[FUJITA Tomonori]

On Mon, 10 Aug 2026 13:07:20 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "FUJITA Tomonori" <tomo@flapping.org> writes:
> 
>> 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>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>>  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 1db84cd4cbe8..04f47af3541b 100644
>> --- a/rust/kernel/time/hrtimer.rs
>> +++ b/rust/kernel/time/hrtimer.rs
>> @@ -418,8 +418,12 @@
>>  /// # Invariants
>>  ///
>>  /// * `self.timer` is initialized by `bindings::hrtimer_setup`.
>> +// `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`.
> 
> Missing bullet. With that fixed:
> 
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

This is a plain `//` comment, not documentation. Did you mean that you
want it documented as one of the `# Invariants` bullets instead?

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

[Andreas Hindborg]

FUJITA Tomonori <tomo@flapping.org> writes:

> On Mon, 10 Aug 2026 13:07:20 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "FUJITA Tomonori" <tomo@flapping.org> writes:
>> 
>>> 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>
>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>> ---
>>>  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 1db84cd4cbe8..04f47af3541b 100644
>>> --- a/rust/kernel/time/hrtimer.rs
>>> +++ b/rust/kernel/time/hrtimer.rs
>>> @@ -418,8 +418,12 @@
>>>  /// # Invariants
>>>  ///
>>>  /// * `self.timer` is initialized by `bindings::hrtimer_setup`.
>>> +// `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`.
>> 
>> Missing bullet. With that fixed:
>> 
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> This is a plain `//` comment, not documentation. Did you mean that you
> want it documented as one of the `# Invariants` bullets instead?

Ah, thanks for clarifying, I did not see that. No I guess it is fine.
Maybe add a newline?


Best regards,
Andreas Hindborg

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

[FUJITA Tomonori]

On Mon, 10 Aug 2026 15:28:06 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> FUJITA Tomonori <tomo@flapping.org> writes:
> 
>> On Mon, 10 Aug 2026 13:07:20 +0200
>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>>> "FUJITA Tomonori" <tomo@flapping.org> writes:
>>> 
>>>> 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>
>>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>>> ---
>>>>  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 1db84cd4cbe8..04f47af3541b 100644
>>>> --- a/rust/kernel/time/hrtimer.rs
>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>> @@ -418,8 +418,12 @@
>>>>  /// # Invariants
>>>>  ///
>>>>  /// * `self.timer` is initialized by `bindings::hrtimer_setup`.
>>>> +// `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`.
>>> 
>>> Missing bullet. With that fixed:
>>> 
>>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>> This is a plain `//` comment, not documentation. Did you mean that you
>> want it documented as one of the `# Invariants` bullets instead?
> 
> Ah, thanks for clarifying, I did not see that. No I guess it is fine.
> Maybe add a newline?

Documentation/rust/coding-guidelines.rst gives an example where a
comment follows the documentation with no blank line in between.

The existing code follows that too, so I think we should stay
consistent with the documented convention here.

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

[Andreas Hindborg]

FUJITA Tomonori <tomo@flapping.org> writes:

> On Mon, 10 Aug 2026 15:28:06 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> FUJITA Tomonori <tomo@flapping.org> writes:
>> 
>>> On Mon, 10 Aug 2026 13:07:20 +0200
>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>>> "FUJITA Tomonori" <tomo@flapping.org> writes:
>>>> 
>>>>> 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>
>>>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>>>> ---
>>>>>  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 1db84cd4cbe8..04f47af3541b 100644
>>>>> --- a/rust/kernel/time/hrtimer.rs
>>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>>> @@ -418,8 +418,12 @@
>>>>>  /// # Invariants
>>>>>  ///
>>>>>  /// * `self.timer` is initialized by `bindings::hrtimer_setup`.
>>>>> +// `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`.
>>>> 
>>>> Missing bullet. With that fixed:
>>>> 
>>>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>
>>> This is a plain `//` comment, not documentation. Did you mean that you
>>> want it documented as one of the `# Invariants` bullets instead?
>> 
>> Ah, thanks for clarifying, I did not see that. No I guess it is fine.
>> Maybe add a newline?
>
> Documentation/rust/coding-guidelines.rst gives an example where a
> comment follows the documentation with no blank line in between.
>
> The existing code follows that too, so I think we should stay
> consistent with the documented convention here.

Ok then.

Best regards,
Andreas Hindborg

Re: [PATCH v4 1/2] rust: hrtimer: Restrict expires() to safe contexts

[Andreas Hindborg]

"FUJITA Tomonori" <tomo@flapping.org> writes:

> 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 expires_unchecked() 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>

This looks good to me now.

@Miguel, can you take this through rust or rust-fixes? Please add the
missing bullet in the invariant on patch 2.

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


Best regards,
Andreas Hindborg