This patch adds support for intrusive use of the hrtimer system. For now,
only one timer can be embedded in a Rust struct.
The hrtimer Rust API is based on the intrusive style pattern introduced by
the Rust workqueue API.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/hrtimer.rs | 231 +++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
2 files changed, 232 insertions(+)
create mode 100644 rust/kernel/hrtimer.rs
diff --git a/rust/kernel/hrtimer.rs b/rust/kernel/hrtimer.rs
new file mode 100644
index 000000000000..5c92afd8eb2c
--- /dev/null
+++ b/rust/kernel/hrtimer.rs
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Intrusive high resolution timers.
+//!
+//! Allows scheduling timer callbacks without doing allocations at the time of
+//! scheduling. For now, only one timer per type is allowed.
+
+use crate::{init::PinInit, prelude::*, time::Ktime, types::Opaque};
+use core::marker::PhantomData;
+
+/// A timer backed by a C `struct hrtimer`.
+///
+/// # Invariants
+///
+/// * `self.timer` is initialized by `bindings::hrtimer_init`.
+#[repr(transparent)]
+#[pin_data]
+pub struct Timer<U> {
+ #[pin]
+ timer: Opaque<bindings::hrtimer>,
+ _t: PhantomData<U>,
+}
+
+// SAFETY: A `Timer` can be moved to other threads and used/dropped from there.
+unsafe impl<U> Send for Timer<U> {}
+
+// SAFETY: Timer operations are locked on C side, so it is safe to operate on a
+// timer from multiple threads
+unsafe impl<U> Sync for Timer<U> {}
+
+impl<T> Timer<T> {
+ /// Return an initializer for a new timer instance.
+ pub fn new() -> impl PinInit<Self>
+ where
+ T: TimerCallback,
+ {
+ pin_init!( Self {
+ // INVARIANTS: We initialize `timer` with `hrtimer_init` below.
+ timer <- Opaque::ffi_init(move |place: *mut bindings::hrtimer| {
+ // SAFETY: By design of `pin_init!`, `place` is a pointer live
+ // allocation. hrtimer_init will initialize `place` and does not
+ // require `place` to be initialized prior to the call.
+ unsafe {
+ bindings::hrtimer_init(
+ place,
+ bindings::CLOCK_MONOTONIC as i32,
+ bindings::hrtimer_mode_HRTIMER_MODE_REL,
+ );
+ }
+
+ // SAFETY: `place` is pointing to a live allocation, so the deref
+ // is safe.
+ let function: *mut Option<_> =
+ unsafe { core::ptr::addr_of_mut!((*place).function) };
+
+ // SAFETY: `function` points to a valid allocation and we have
+ // exclusive access.
+ unsafe { core::ptr::write(function, Some(T::CallbackTarget::run)) };
+ }),
+ _t: PhantomData,
+ })
+ }
+
+ /// Get a pointer to the contained `bindings::hrtimer`.
+ ///
+ /// # Safety
+ ///
+ /// `ptr` must point to a live allocation of at least the size of `Self`.
+ unsafe fn raw_get(ptr: *const Self) -> *mut bindings::hrtimer {
+ // SAFETY: The field projection to `timer` does not go out of bounds,
+ // because the caller of this function promises that `ptr` points to an
+ // allocation of at least the size of `Self`.
+ unsafe { Opaque::raw_get(core::ptr::addr_of!((*ptr).timer)) }
+ }
+}
+
+/// Implemented by pointer types that point to structs that embed a [`Timer`].
+///
+/// Typical implementers would be [`Box<T>`], [`Arc<T>`], [`ARef<T>`] where `T`
+/// has a field of type `Timer`.
+///
+/// Target must be [`Sync`] because timer callbacks happen in another thread of
+/// execution (hard or soft interrupt context).
+///
+/// Scheduling a timer returns a [`TimerHandle`] that can be used to manipulate
+/// the timer. Note that it is OK to call the schedule function repeatedly, and
+/// that more than one [`TimerHandle`] associated with a `TimerPointer` may
+/// exist. A timer can be manipulated through any of the handles, and a handle
+/// may represent a cancelled timer.
+///
+/// [`Box<T>`]: Box
+/// [`Arc<T>`]: crate::sync::Arc
+/// [`ARef<T>`]: crate::types::ARef
+pub trait TimerPointer: Sync + Sized {
+ /// A handle representing a scheduled timer.
+ ///
+ /// If the timer is armed or if the timer callback is running when the
+ /// handle is dropped, the drop method of `TimerHandle` should not return
+ /// until the timer is unarmed and the callback has completed.
+ ///
+ /// Note: It must be safe to leak the handle.
+ type TimerHandle: TimerHandle;
+
+ /// Schedule the timer after `expires` time units. If the timer was already
+ /// scheduled, it is rescheduled at the new expiry time.
+ fn schedule(self, expires: Ktime) -> Self::TimerHandle;
+}
+
+/// Implemented by [`TimerPointer`] implementers to give the C timer callback a
+/// function to call.
+// This is split from `TimerPointer` to make it easier to specify trait bounds.
+pub trait RawTimerCallback {
+ /// Callback to be called from C when timer fires.
+ ///
+ /// # Safety
+ ///
+ /// Only to be called by C code in `hrtimer` subsystem. `ptr` must point to
+ /// the `bindings::hrtimer` structure that was used to schedule the timer.
+ unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart;
+}
+
+/// Implemented by structs that can the target of a timer callback.
+pub trait TimerCallback {
+ /// The type that was used for scheduling the timer.
+ type CallbackTarget<'a>: RawTimerCallback;
+
+ /// Called by the timer logic when the timer fires.
+ fn run(this: Self::CallbackTarget<'_>)
+ where
+ Self: Sized;
+}
+
+/// A handle representing a potentially armed timer.
+///
+/// More than one handle representing the same timer might exist.
+///
+/// # Safety
+///
+/// When dropped, the timer represented by this handle must be cancelled, if it
+/// is armed. If the timer handler is running when the handle is dropped, the
+/// drop method must wait for the handler to finish before returning.
+pub unsafe trait TimerHandle {}
+
+/// Implemented by structs that contain timer nodes.
+///
+/// Clients of the timer API would usually safely implement this trait by using
+/// the [`impl_has_timer`] macro.
+///
+/// # Safety
+///
+/// Implementers of this trait must ensure that the implementer has a [`Timer`]
+/// field at the offset specified by `OFFSET` and that all trait methods are
+/// implemented according to their documentation.
+///
+/// [`impl_has_timer`]: crate::impl_has_timer
+pub unsafe trait HasTimer<U> {
+ /// Offset of the [`Timer`] field within `Self`
+ const OFFSET: usize;
+
+ /// Return a pointer to the [`Timer`] within `Self`.
+ ///
+ /// # Safety
+ ///
+ /// `ptr` must point to a valid struct of type `Self`.
+ unsafe fn raw_get_timer(ptr: *const Self) -> *const Timer<U> {
+ // SAFETY: By the safety requirement of this trait, the trait
+ // implementor will have a `Timer` field at the specified offset.
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast::<Timer<U>>() }
+ }
+
+ /// Return a pointer to the struct that is embedding the [`Timer`] pointed
+ /// to by `ptr`.
+ ///
+ /// # Safety
+ ///
+ /// `ptr` must point to a [`Timer<U>`] field in a struct of type `Self`.
+ unsafe fn timer_container_of(ptr: *mut Timer<U>) -> *mut Self
+ where
+ Self: Sized,
+ {
+ // SAFETY: By the safety requirement of this function and the `HasTimer`
+ // trait, the following expression will yield a pointer to the `Self`
+ // containing the timer addressed by `ptr`.
+ unsafe { ptr.cast::<u8>().sub(Self::OFFSET).cast::<Self>() }
+ }
+
+ /// Get pointer to embedded `bindings::hrtimer` struct.
+ ///
+ /// # Safety
+ ///
+ /// `self_ptr` must point to a valid `Self`.
+ unsafe fn c_timer_ptr(self_ptr: *const Self) -> *const bindings::hrtimer {
+ // SAFETY: `self_ptr` is a valid pointer to a `Self`.
+ let timer_ptr = unsafe { Self::raw_get_timer(self_ptr) };
+
+ // SAFETY: timer_ptr points to an allocation of at least `Timer` size.
+ unsafe { Timer::raw_get(timer_ptr) }
+ }
+}
+
+/// Use to implement the [`HasTimer<T>`] trait.
+///
+/// See [`module`] documentation for an example.
+///
+/// [`module`]: crate::hrtimer
+#[macro_export]
+macro_rules! impl_has_timer {
+ (
+ impl$({$($generics:tt)*})?
+ HasTimer<$timer_type:ty>
+ for $self:ty
+ { self.$field:ident }
+ $($rest:tt)*
+ ) => {
+ // SAFETY: This implementation of `raw_get_timer` only compiles if the
+ // field has the right type.
+ unsafe impl$(<$($generics)*>)? $crate::hrtimer::HasTimer<$timer_type> for $self {
+ const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
+
+ #[inline]
+ unsafe fn raw_get_timer(ptr: *const Self) ->
+ *const $crate::hrtimer::Timer<$timer_type>
+ {
+ // SAFETY: The caller promises that the pointer is not dangling.
+ unsafe {
+ ::core::ptr::addr_of!((*ptr).$field)
+ }
+ }
+ }
+ }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 274bdc1b0a82..55f846c5a849 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -34,6 +34,7 @@
pub mod error;
#[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
pub mod firmware;
+pub mod hrtimer;
pub mod init;
pub mod ioctl;
#[cfg(CONFIG_KUNIT)]
--
2.46.0On 18.09.24 00:27, Andreas Hindborg wrote:
> This patch adds support for intrusive use of the hrtimer system. For now,
> only one timer can be embedded in a Rust struct.
>
> The hrtimer Rust API is based on the intrusive style pattern introduced by
> the Rust workqueue API.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/hrtimer.rs | 231 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 2 files changed, 232 insertions(+)
> create mode 100644 rust/kernel/hrtimer.rs
>
> diff --git a/rust/kernel/hrtimer.rs b/rust/kernel/hrtimer.rs
> new file mode 100644
> index 000000000000..5c92afd8eb2c
> --- /dev/null
> +++ b/rust/kernel/hrtimer.rs
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Intrusive high resolution timers.
> +//!
> +//! Allows scheduling timer callbacks without doing allocations at the time of
> +//! scheduling. For now, only one timer per type is allowed.
> +
> +use crate::{init::PinInit, prelude::*, time::Ktime, types::Opaque};
> +use core::marker::PhantomData;
> +
> +/// A timer backed by a C `struct hrtimer`.
> +///
> +/// # Invariants
> +///
> +/// * `self.timer` is initialized by `bindings::hrtimer_init`.
> +#[repr(transparent)]
> +#[pin_data]
> +pub struct Timer<U> {
> + #[pin]
> + timer: Opaque<bindings::hrtimer>,
> + _t: PhantomData<U>,
> +}
> +
> +// SAFETY: A `Timer` can be moved to other threads and used/dropped from there.
> +unsafe impl<U> Send for Timer<U> {}
> +
> +// SAFETY: Timer operations are locked on C side, so it is safe to operate on a
> +// timer from multiple threads
> +unsafe impl<U> Sync for Timer<U> {}
> +
> +impl<T> Timer<T> {
> + /// Return an initializer for a new timer instance.
> + pub fn new() -> impl PinInit<Self>
> + where
> + T: TimerCallback,
> + {
> + pin_init!( Self {
I would remove the space after the `(`.
Would be great if we had rustfmt support for custom macros.
> + // INVARIANTS: We initialize `timer` with `hrtimer_init` below.
> + timer <- Opaque::ffi_init(move |place: *mut bindings::hrtimer| {
> + // SAFETY: By design of `pin_init!`, `place` is a pointer live
> + // allocation. hrtimer_init will initialize `place` and does not
> + // require `place` to be initialized prior to the call.
> + unsafe {
> + bindings::hrtimer_init(
> + place,
> + bindings::CLOCK_MONOTONIC as i32,
> + bindings::hrtimer_mode_HRTIMER_MODE_REL,
> + );
> + }
> +
> + // SAFETY: `place` is pointing to a live allocation, so the deref
> + // is safe.
> + let function: *mut Option<_> =
Do you really need this type hint?
> + unsafe { core::ptr::addr_of_mut!((*place).function) };
> +
> + // SAFETY: `function` points to a valid allocation and we have
> + // exclusive access.
> + unsafe { core::ptr::write(function, Some(T::CallbackTarget::run)) };
> + }),
> + _t: PhantomData,
> + })
> + }
> +
> + /// Get a pointer to the contained `bindings::hrtimer`.
> + ///
> + /// # Safety
> + ///
> + /// `ptr` must point to a live allocation of at least the size of `Self`.
> + unsafe fn raw_get(ptr: *const Self) -> *mut bindings::hrtimer {
> + // SAFETY: The field projection to `timer` does not go out of bounds,
> + // because the caller of this function promises that `ptr` points to an
> + // allocation of at least the size of `Self`.
> + unsafe { Opaque::raw_get(core::ptr::addr_of!((*ptr).timer)) }
> + }
> +}
> +
> +/// Implemented by pointer types that point to structs that embed a [`Timer`].
> +///
> +/// Typical implementers would be [`Box<T>`], [`Arc<T>`], [`ARef<T>`] where `T`
> +/// has a field of type `Timer`.
> +///
> +/// Target must be [`Sync`] because timer callbacks happen in another thread of
> +/// execution (hard or soft interrupt context).
> +///
> +/// Scheduling a timer returns a [`TimerHandle`] that can be used to manipulate
> +/// the timer. Note that it is OK to call the schedule function repeatedly, and
> +/// that more than one [`TimerHandle`] associated with a `TimerPointer` may
> +/// exist. A timer can be manipulated through any of the handles, and a handle
> +/// may represent a cancelled timer.
> +///
> +/// [`Box<T>`]: Box
> +/// [`Arc<T>`]: crate::sync::Arc
> +/// [`ARef<T>`]: crate::types::ARef
> +pub trait TimerPointer: Sync + Sized {
> + /// A handle representing a scheduled timer.
> + ///
> + /// If the timer is armed or if the timer callback is running when the
> + /// handle is dropped, the drop method of `TimerHandle` should not return
> + /// until the timer is unarmed and the callback has completed.
> + ///
> + /// Note: It must be safe to leak the handle.
> + type TimerHandle: TimerHandle;
Why does this need to be an associated type? Couldn't we have a
`TimerHandle<T>` struct? The schedule function below could then return
`TimerHandle<Self>`.
> +
> + /// Schedule the timer after `expires` time units. If the timer was already
> + /// scheduled, it is rescheduled at the new expiry time.
> + fn schedule(self, expires: Ktime) -> Self::TimerHandle;
> +}
> +/// Use to implement the [`HasTimer<T>`] trait.
> +///
> +/// See [`module`] documentation for an example.
> +///
> +/// [`module`]: crate::hrtimer
> +#[macro_export]
> +macro_rules! impl_has_timer {
> + (
> + impl$({$($generics:tt)*})?
> + HasTimer<$timer_type:ty>
> + for $self:ty
> + { self.$field:ident }
> + $($rest:tt)*
> + ) => {
> + // SAFETY: This implementation of `raw_get_timer` only compiles if the
> + // field has the right type.
> + unsafe impl$(<$($generics)*>)? $crate::hrtimer::HasTimer<$timer_type> for $self {
There is a double space in front of `for`.
> + const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
> +
> + #[inline]
> + unsafe fn raw_get_timer(ptr: *const Self) ->
> + *const $crate::hrtimer::Timer<$timer_type>
> + {
> + // SAFETY: The caller promises that the pointer is not dangling.
> + unsafe {
> + ::core::ptr::addr_of!((*ptr).$field)
> + }
> + }
> + }
> + }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 274bdc1b0a82..55f846c5a849 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -34,6 +34,7 @@
> pub mod error;
> #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
> pub mod firmware;
> +pub mod hrtimer;
> pub mod init;
> pub mod ioctl;
> #[cfg(CONFIG_KUNIT)]
> --
> 2.46.0
>
>
Hi Benno,
Thanks for the feedback. I will incorporate all the whitespace
suggestions you have =F0=9F=91=8D
"Benno Lossin" <benno.lossin@proton.me> writes:
> On 18.09.24 00:27, Andreas Hindborg wrote:
[...]
>> +
>> +impl<T> Timer<T> {
>> + /// Return an initializer for a new timer instance.
>> + pub fn new() -> impl PinInit<Self>
>> + where
>> + T: TimerCallback,
>> + {
>> + pin_init!( Self {
>
> I would remove the space after the `(`.
> Would be great if we had rustfmt support for custom macros.
Yes, that would be great!
>
>> + // INVARIANTS: We initialize `timer` with `hrtimer_init` be=
low.
>> + timer <- Opaque::ffi_init(move |place: *mut bindings::hrtim=
er| {
>> + // SAFETY: By design of `pin_init!`, `place` is a point=
er live
>> + // allocation. hrtimer_init will initialize `place` and=
does not
>> + // require `place` to be initialized prior to the call.
>> + unsafe {
>> + bindings::hrtimer_init(
>> + place,
>> + bindings::CLOCK_MONOTONIC as i32,
>> + bindings::hrtimer_mode_HRTIMER_MODE_REL,
>> + );
>> + }
>> +
>> + // SAFETY: `place` is pointing to a live allocation, so=
the deref
>> + // is safe.
>> + let function: *mut Option<_> =3D
>
> Do you really need this type hint?
Apparently not!
[...]
>> +pub trait TimerPointer: Sync + Sized {
>> + /// A handle representing a scheduled timer.
>> + ///
>> + /// If the timer is armed or if the timer callback is running when =
the
>> + /// handle is dropped, the drop method of `TimerHandle` should not =
return
>> + /// until the timer is unarmed and the callback has completed.
>> + ///
>> + /// Note: It must be safe to leak the handle.
>> + type TimerHandle: TimerHandle;
>
> Why does this need to be an associated type? Couldn't we have a
> `TimerHandle<T>` struct? The schedule function below could then return
> `TimerHandle<Self>`.
At one point, I had some cycles in trait resolution. Moving generics to
associated types solved that issue. Maybe this can be changed to a
generic. But are generics preferred over associated types for some
reason?
Best regards,
Andreas
On 19.09.24 07:43, Andreas Hindborg wrote:
> Hi Benno,
>
> Thanks for the feedback. I will incorporate all the whitespace
> suggestions you have =F0=9F=91=8D
There seems to be something wrong with the encoding in this email, is
that an issue on my side or yours?
> "Benno Lossin" <benno.lossin@proton.me> writes:
>
>> On 18.09.24 00:27, Andreas Hindborg wrote:
> [...]
>>> +
>>> +impl<T> Timer<T> {
>>> + /// Return an initializer for a new timer instance.
>>> + pub fn new() -> impl PinInit<Self>
>>> + where
>>> + T: TimerCallback,
>>> + {
>>> + pin_init!( Self {
>>
>> I would remove the space after the `(`.
>> Would be great if we had rustfmt support for custom macros.
>
> Yes, that would be great!
>
>>
>>> + // INVARIANTS: We initialize `timer` with `hrtimer_init` be=
> low.
>>> + timer <- Opaque::ffi_init(move |place: *mut bindings::hrtim=
> er| {
>>> + // SAFETY: By design of `pin_init!`, `place` is a point=
> er live
>>> + // allocation. hrtimer_init will initialize `place` and=
> does not
>>> + // require `place` to be initialized prior to the call.
>>> + unsafe {
>>> + bindings::hrtimer_init(
>>> + place,
>>> + bindings::CLOCK_MONOTONIC as i32,
>>> + bindings::hrtimer_mode_HRTIMER_MODE_REL,
>>> + );
>>> + }
>>> +
>>> + // SAFETY: `place` is pointing to a live allocation, so=
> the deref
>>> + // is safe.
>>> + let function: *mut Option<_> =3D
>>
>> Do you really need this type hint?
>
> Apparently not!
>
> [...]
>>> +pub trait TimerPointer: Sync + Sized {
>>> + /// A handle representing a scheduled timer.
>>> + ///
>>> + /// If the timer is armed or if the timer callback is running when =
> the
>>> + /// handle is dropped, the drop method of `TimerHandle` should not =
> return
>>> + /// until the timer is unarmed and the callback has completed.
>>> + ///
>>> + /// Note: It must be safe to leak the handle.
>>> + type TimerHandle: TimerHandle;
>>
>> Why does this need to be an associated type? Couldn't we have a
>> `TimerHandle<T>` struct? The schedule function below could then return
>> `TimerHandle<Self>`.
>
> At one point, I had some cycles in trait resolution. Moving generics to
> associated types solved that issue. Maybe this can be changed to a
> generic. But are generics preferred over associated types for some
> reason?
The associated type is more complicated IMO, because then every
implementer of the trait needs to create one. If we can avoid that, I
would prefer a generic type.
---
Cheers,
Benno
"Benno Lossin" <benno.lossin@proton.me> writes: > On 19.09.24 07:43, Andreas Hindborg wrote: >> Hi Benno, >> >> Thanks for the feedback. I will incorporate all the whitespace >> suggestions you have =F0=9F=91=8D > > There seems to be something wrong with the encoding in this email, is > that an issue on my side or yours? It seems to be on my end. It also showed up on lore with errors. I will check my tools. [...] >>>> + /// until the timer is unarmed and the callback has completed. >>>> + /// >>>> + /// Note: It must be safe to leak the handle. >>>> + type TimerHandle: TimerHandle; >>> >>> Why does this need to be an associated type? Couldn't we have a >>> `TimerHandle<T>` struct? The schedule function below could then return >>> `TimerHandle<Self>`. >> >> At one point, I had some cycles in trait resolution. Moving generics to >> associated types solved that issue. Maybe this can be changed to a >> generic. But are generics preferred over associated types for some >> reason? > > The associated type is more complicated IMO, because then every > implementer of the trait needs to create one. If we can avoid that, I > would prefer a generic type. When you say create, you mean specify? Users would not invent their own type to put here, they would use the types defined by the `hrtimer` module in later patches. BR Andreas
On 23.09.24 18:35, Andreas Hindborg wrote: > "Benno Lossin" <benno.lossin@proton.me> writes: >> On 19.09.24 07:43, Andreas Hindborg wrote: >>>>> + /// until the timer is unarmed and the callback has completed. >>>>> + /// >>>>> + /// Note: It must be safe to leak the handle. >>>>> + type TimerHandle: TimerHandle; >>>> >>>> Why does this need to be an associated type? Couldn't we have a >>>> `TimerHandle<T>` struct? The schedule function below could then return >>>> `TimerHandle<Self>`. >>> >>> At one point, I had some cycles in trait resolution. Moving generics to >>> associated types solved that issue. Maybe this can be changed to a >>> generic. But are generics preferred over associated types for some >>> reason? >> >> The associated type is more complicated IMO, because then every >> implementer of the trait needs to create one. If we can avoid that, I >> would prefer a generic type. > > When you say create, you mean specify? Yes. > Users would not invent their own type to put here, they would use the > types defined by the `hrtimer` module in later patches. I mean the implementers of this trait, not the users of the trait. You define an `ArcTimerHandle`, `PinTimerHandle` and a `PinMutTimerHandle` in this series. I think we can avoid that using a single generic struct. --- Cheers, Benno
Benno Lossin <benno.lossin@proton.me> writes:
> On 23.09.24 18:35, Andreas Hindborg wrote:
>> "Benno Lossin" <benno.lossin@proton.me> writes:
>>> On 19.09.24 07:43, Andreas Hindborg wrote:
>>>>>> + /// until the timer is unarmed and the callback has completed.
>>>>>> + ///
>>>>>> + /// Note: It must be safe to leak the handle.
>>>>>> + type TimerHandle: TimerHandle;
>>>>>
>>>>> Why does this need to be an associated type? Couldn't we have a
>>>>> `TimerHandle<T>` struct? The schedule function below could then return
>>>>> `TimerHandle<Self>`.
>>>>
>>>> At one point, I had some cycles in trait resolution. Moving generics to
>>>> associated types solved that issue. Maybe this can be changed to a
>>>> generic. But are generics preferred over associated types for some
>>>> reason?
>>>
>>> The associated type is more complicated IMO, because then every
>>> implementer of the trait needs to create one. If we can avoid that, I
>>> would prefer a generic type.
>>
>> When you say create, you mean specify?
>
> Yes.
>
>> Users would not invent their own type to put here, they would use the
>> types defined by the `hrtimer` module in later patches.
>
> I mean the implementers of this trait, not the users of the trait. You
> define an `ArcTimerHandle`, `PinTimerHandle` and a `PinMutTimerHandle`
> in this series. I think we can avoid that using a single generic struct.
It is not immediately clear for me how to do this. For `Box` we have:
pub struct BoxTimerHandle<U>
where
U: HasTimer<U>,
{
pub(crate) inner: *mut U,
}
but for `Pin<&U>` we have
pub struct PinTimerHandle<'a, U>
where
U: HasTimer<U>,
{
pub(crate) inner: Pin<&'a U>,
}
How can these be combined to a single generic struct?
Best regards,
Andreas
© 2016 - 2024 Red Hat, Inc.