Allow pinned mutable references to structs that contain a `HrTimer` node to
be scheduled with the `hrtimer` subsystem.
Acked-by: Frederic Weisbecker <frederic@kernel.org>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/time/hrtimer.rs | 2 +
rust/kernel/time/hrtimer/pin_mut.rs | 110 ++++++++++++++++++++++++++++++++++++
2 files changed, 112 insertions(+)
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index fee8e44447ee..ab0950a964e8 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -433,3 +433,5 @@ unsafe fn timer_container_of(
pub use arc::ArcHrTimerHandle;
mod pin;
pub use pin::PinHrTimerHandle;
+mod pin_mut;
+pub use pin_mut::PinMutHrTimerHandle;
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
new file mode 100644
index 000000000000..007f47d26df6
--- /dev/null
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use super::{
+ HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
+};
+use crate::time::Ktime;
+use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
+
+/// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
+/// be running.
+pub struct PinMutHrTimerHandle<'a, T>
+where
+ T: HasHrTimer<T>,
+{
+ pub(crate) inner: NonNull<T>,
+ _p: PhantomData<&'a T>,
+}
+
+// SAFETY: We cancel the timer when the handle is dropped. The implementation of
+// the `cancel` method will block if the timer handler is running.
+unsafe impl<'a, T> HrTimerHandle for PinMutHrTimerHandle<'a, T>
+where
+ T: HasHrTimer<T>,
+{
+ fn cancel(&mut self) -> bool {
+ let self_ptr = self.inner.as_ptr();
+
+ // SAFETY: As we got `self_ptr` from a reference above, it must point to
+ // a valid `T`.
+ let timer_ptr = unsafe { <T as HasHrTimer<T>>::raw_get_timer(self_ptr) };
+
+ // SAFETY: As `timer_ptr` is derived from a reference, it must point to
+ // a valid and initialized `HrTimer`.
+ unsafe { HrTimer::<T>::raw_cancel(timer_ptr) }
+ }
+}
+
+impl<'a, T> Drop for PinMutHrTimerHandle<'a, T>
+where
+ T: HasHrTimer<T>,
+{
+ fn drop(&mut self) {
+ self.cancel();
+ }
+}
+
+// SAFETY: We capture the lifetime of `Self` when we create a
+// `PinMutHrTimerHandle`, so `Self` will outlive the handle.
+unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
+where
+ T: Send + Sync,
+ T: HasHrTimer<T>,
+ T: HrTimerCallback<Pointer<'a> = Self>,
+{
+ type TimerHandle = PinMutHrTimerHandle<'a, T>;
+
+ unsafe fn start(mut self, expires: Ktime) -> Self::TimerHandle {
+ // SAFETY:
+ // - We promise not to move out of `self`. We only pass `self`
+ // back to the caller as a `Pin<&mut self>`.
+ // - The return value of `get_unchecked_mut` is guaranteed not to be null.
+ let self_ptr = unsafe { NonNull::new_unchecked(self.as_mut().get_unchecked_mut()) };
+
+ // SAFETY:
+ // - As we derive `self_ptr` from a reference above, it must point to a
+ // valid `T`.
+ // - We keep `self` alive by wrapping it in a handle below.
+ unsafe { T::start(self_ptr.as_ptr(), expires) };
+
+ PinMutHrTimerHandle {
+ inner: self_ptr,
+ _p: PhantomData,
+ }
+ }
+}
+
+impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
+where
+ T: HasHrTimer<T>,
+ T: HrTimerCallback<Pointer<'a> = Self>,
+{
+ type CallbackTarget<'b> = Self;
+
+ unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
+ // `HrTimer` is `repr(C)`
+ let timer_ptr = ptr as *mut HrTimer<T>;
+
+ // SAFETY: By the safety requirement of this function, `timer_ptr`
+ // points to a `HrTimer<T>` contained in an `T`.
+ let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
+
+ // SAFETY:
+ // - By the safety requirement of this function, `timer_ptr`
+ // points to a `HrTimer<T>` contained in an `T`.
+ // - As per he safety requirements of the trait `HrTimerHandle`, the
+ // `PinMutHrTimerHandle` associated with this timer is guaranteed to
+ // be alive until this method returns. As the handle borrows from
+ // `T`, `T` is also guaranteed to be alive for the duration of this
+ // function.
+ // - Because `PinMutTimerHandle` borrows mutably from `T`, the
+ // reference we create here is guaranteed to be unique.
+ let receiver_ref = unsafe { &mut *receiver_ptr };
+
+ // SAFETY: `receiver_ref` only exists as pinned, so it is safe to pin it
+ // here.
+ let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
+
+ T::run(receiver_pin).into_c()
+ }
+}
--
2.47.0On Fri Mar 7, 2025 at 10:38 PM CET, Andreas Hindborg wrote:
> +impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
> +where
> + T: HasHrTimer<T>,
> + T: HrTimerCallback<Pointer<'a> = Self>,
> +{
> + type CallbackTarget<'b> = Self;
> +
> + unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
> + // `HrTimer` is `repr(C)`
> + let timer_ptr = ptr as *mut HrTimer<T>;
> +
> + // SAFETY: By the safety requirement of this function, `timer_ptr`
> + // points to a `HrTimer<T>` contained in an `T`.
> + let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
> +
> + // SAFETY:
> + // - By the safety requirement of this function, `timer_ptr`
> + // points to a `HrTimer<T>` contained in an `T`.
> + // - As per he safety requirements of the trait `HrTimerHandle`, the
> + // `PinMutHrTimerHandle` associated with this timer is guaranteed to
> + // be alive until this method returns. As the handle borrows from
> + // `T`, `T` is also guaranteed to be alive for the duration of this
> + // function.
Ah one more thing, I don't think that the second part is needed (i.e.
that `T` is alive). How about:
// - As per the safety requirements of the trait `HrTimerHandle`, the `PinMutHrTimerHandle`
// associated with this timer is guaranteed to be alive until this method returns. That
// handle borrows the `T` behind `receiver_ptr` mutably thus guaranteeing the validity of
// the reference created below.
Can you also adjust the other instances of this in the other patches?
Thanks!
---
Cheers,
Benno
> + // - Because `PinMutTimerHandle` borrows mutably from `T`, the
> + // reference we create here is guaranteed to be unique.
> + let receiver_ref = unsafe { &mut *receiver_ptr };
> +
> + // SAFETY: `receiver_ref` only exists as pinned, so it is safe to pin it
> + // here.
> + let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
> +
> + T::run(receiver_pin).into_c()
> + }
> +}
>
> --
> 2.47.0
"Benno Lossin" <benno.lossin@proton.me> writes:
> On Fri Mar 7, 2025 at 10:38 PM CET, Andreas Hindborg wrote:
>> +impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
>> +where
>> + T: HasHrTimer<T>,
>> + T: HrTimerCallback<Pointer<'a> = Self>,
>> +{
>> + type CallbackTarget<'b> = Self;
>> +
>> + unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
>> + // `HrTimer` is `repr(C)`
>> + let timer_ptr = ptr as *mut HrTimer<T>;
>> +
>> + // SAFETY: By the safety requirement of this function, `timer_ptr`
>> + // points to a `HrTimer<T>` contained in an `T`.
>> + let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
>> +
>> + // SAFETY:
>> + // - By the safety requirement of this function, `timer_ptr`
>> + // points to a `HrTimer<T>` contained in an `T`.
>> + // - As per he safety requirements of the trait `HrTimerHandle`, the
>> + // `PinMutHrTimerHandle` associated with this timer is guaranteed to
>> + // be alive until this method returns. As the handle borrows from
>> + // `T`, `T` is also guaranteed to be alive for the duration of this
>> + // function.
>
> Ah one more thing, I don't think that the second part is needed (i.e.
> that `T` is alive). How about:
>
> // - As per the safety requirements of the trait `HrTimerHandle`, the `PinMutHrTimerHandle`
> // associated with this timer is guaranteed to be alive until this method returns. That
> // handle borrows the `T` behind `receiver_ptr` mutably thus guaranteeing the validity of
> // the reference created below.
OK.
>
> Can you also adjust the other instances of this in the other patches?
> Thanks!
Yes!
Best regards,
Andreas Hindborg
On Fri Mar 7, 2025 at 10:38 PM CET, Andreas Hindborg wrote:
> Allow pinned mutable references to structs that contain a `HrTimer` node to
> be scheduled with the `hrtimer` subsystem.
>
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
One problem below, with that fixed:
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/time/hrtimer.rs | 2 +
> rust/kernel/time/hrtimer/pin_mut.rs | 110 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 112 insertions(+)
>
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index fee8e44447ee..ab0950a964e8 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -433,3 +433,5 @@ unsafe fn timer_container_of(
> pub use arc::ArcHrTimerHandle;
> mod pin;
> pub use pin::PinHrTimerHandle;
> +mod pin_mut;
> +pub use pin_mut::PinMutHrTimerHandle;
> diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
> new file mode 100644
> index 000000000000..007f47d26df6
> --- /dev/null
> +++ b/rust/kernel/time/hrtimer/pin_mut.rs
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use super::{
> + HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
> +};
> +use crate::time::Ktime;
> +use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
> +
> +/// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
> +/// be running.
> +pub struct PinMutHrTimerHandle<'a, T>
> +where
> + T: HasHrTimer<T>,
> +{
> + pub(crate) inner: NonNull<T>,
> + _p: PhantomData<&'a T>,
This should be `PhantomData<&'a mut T>`.
---
Cheers,
Benno
"Benno Lossin" <benno.lossin@proton.me> writes:
> On Fri Mar 7, 2025 at 10:38 PM CET, Andreas Hindborg wrote:
>> Allow pinned mutable references to structs that contain a `HrTimer` node to
>> be scheduled with the `hrtimer` subsystem.
>>
>> Acked-by: Frederic Weisbecker <frederic@kernel.org>
>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> One problem below, with that fixed:
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Thanks, will fix.
>
>> ---
>> rust/kernel/time/hrtimer.rs | 2 +
>> rust/kernel/time/hrtimer/pin_mut.rs | 110 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 112 insertions(+)
>>
>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>> index fee8e44447ee..ab0950a964e8 100644
>> --- a/rust/kernel/time/hrtimer.rs
>> +++ b/rust/kernel/time/hrtimer.rs
>> @@ -433,3 +433,5 @@ unsafe fn timer_container_of(
>> pub use arc::ArcHrTimerHandle;
>> mod pin;
>> pub use pin::PinHrTimerHandle;
>> +mod pin_mut;
>> +pub use pin_mut::PinMutHrTimerHandle;
>> diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
>> new file mode 100644
>> index 000000000000..007f47d26df6
>> --- /dev/null
>> +++ b/rust/kernel/time/hrtimer/pin_mut.rs
>> @@ -0,0 +1,110 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use super::{
>> + HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
>> +};
>> +use crate::time::Ktime;
>> +use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
>> +
>> +/// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
>> +/// be running.
>> +pub struct PinMutHrTimerHandle<'a, T>
>> +where
>> + T: HasHrTimer<T>,
>> +{
>> + pub(crate) inner: NonNull<T>,
>> + _p: PhantomData<&'a T>,
>
> This should be `PhantomData<&'a mut T>`.
Nice catch!
Best regards,
Andreas Hindborg
© 2016 - 2026 Red Hat, Inc.