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

[Andreas Hindborg <a.hindborg@kernel.org>]

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