[PATCH] rust: hrtimer: document handle based design rationale

Andreas Hindborg posted 1 patch 1 month, 2 weeks ago
rust/kernel/time/hrtimer.rs | 12 ++++++++++++
1 file changed, 12 insertions(+)
[PATCH] rust: hrtimer: document handle based design rationale
Posted by Andreas Hindborg 1 month, 2 weeks ago
Add implementation notes explaining why the hrtimer abstraction uses a
handle based approach.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/time/hrtimer.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 856d2d929a008..f92880b2cbdbd 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -67,6 +67,18 @@
 //! A `restart` operation on a timer in the **stopped** state is equivalent to a
 //! `start` operation.
 
+// Implementation details
+//
+// The reasoning for adopting a handle based approach:
+// - If we explicitly drop the target of a timer callback in the timer callback, we
+//   may get a dangling reference.
+// - If the callback owns the last reference to the target, target may be dropped
+//   in non-sleepable context when the callback is finished.
+// - When dropping an object that is the target of an armed timer, we may drop
+//   fields accessed by the timer callback before we cancel the timer (drop order).
+//
+// By using a handle, we can make the handle own the callback target and avoid these problems.
+
 use super::{ClockSource, Delta, Instant};
 use crate::{prelude::*, types::Opaque};
 use core::{marker::PhantomData, ptr::NonNull};

---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260215-hrtimer-docs-52ec9c020285

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>
Re: [PATCH] rust: hrtimer: document handle based design rationale
Posted by FUJITA Tomonori 1 month ago
On Sun, 15 Feb 2026 21:36:04 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> Add implementation notes explaining why the hrtimer abstraction uses a
> handle based approach.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/kernel/time/hrtimer.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 856d2d929a008..f92880b2cbdbd 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -67,6 +67,18 @@
>  //! A `restart` operation on a timer in the **stopped** state is equivalent to a
>  //! `start` operation.
>  
> +// Implementation details
> +//
> +// The reasoning for adopting a handle based approach:
> +// - If we explicitly drop the target of a timer callback in the timer callback, we
> +//   may get a dangling reference.
> +// - If the callback owns the last reference to the target, target may be dropped
> +//   in non-sleepable context when the callback is finished.
> +// - When dropping an object that is the target of an armed timer, we may drop
> +//   fields accessed by the timer callback before we cancel the timer (drop order).
> +//
> +// By using a handle, we can make the handle own the callback target and avoid these problems.
> +

Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Re: [PATCH] rust: hrtimer: document handle based design rationale
Posted by Boqun Feng 1 month, 2 weeks ago
On Sun, Feb 15, 2026 at 09:36:04PM +0100, Andreas Hindborg wrote:
> Add implementation notes explaining why the hrtimer abstraction uses a
> handle based approach.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Thanks for adding this!

Reviewed-by: Boqun Feng <boqun@kernel.org>

However I feel these are not just implementation details. They are the
design decision we made because of the limitation you mentioned. Maybe
make them "//!" doc comment and just put them as a separate section that
describes the necessity of handles? Thoughts?

Regards,
Boqun

> ---
>  rust/kernel/time/hrtimer.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 856d2d929a008..f92880b2cbdbd 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -67,6 +67,18 @@
>  //! A `restart` operation on a timer in the **stopped** state is equivalent to a
>  //! `start` operation.
>  
> +// Implementation details



> +//
> +// The reasoning for adopting a handle based approach:
> +// - If we explicitly drop the target of a timer callback in the timer callback, we
> +//   may get a dangling reference.
> +// - If the callback owns the last reference to the target, target may be dropped
> +//   in non-sleepable context when the callback is finished.
> +// - When dropping an object that is the target of an armed timer, we may drop
> +//   fields accessed by the timer callback before we cancel the timer (drop order).
> +//
> +// By using a handle, we can make the handle own the callback target and avoid these problems.
> +
>  use super::{ClockSource, Delta, Instant};
>  use crate::{prelude::*, types::Opaque};
>  use core::{marker::PhantomData, ptr::NonNull};
> 
> ---
> base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
> change-id: 20260215-hrtimer-docs-52ec9c020285
> 
> Best regards,
> -- 
> Andreas Hindborg <a.hindborg@kernel.org>
> 
>
Re: [PATCH] rust: hrtimer: document handle based design rationale
Posted by Andreas Hindborg 1 month, 1 week ago
"Boqun Feng" <boqun@kernel.org> writes:

> On Sun, Feb 15, 2026 at 09:36:04PM +0100, Andreas Hindborg wrote:
>> Add implementation notes explaining why the hrtimer abstraction uses a
>> handle based approach.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Thanks for adding this!
>
> Reviewed-by: Boqun Feng <boqun@kernel.org>
>
> However I feel these are not just implementation details. They are the
> design decision we made because of the limitation you mentioned. Maybe
> make them "//!" doc comment and just put them as a separate section that
> describes the necessity of handles? Thoughts?

I don't think these details are important for the reader of the API? If
you just want to use a timer, this is not something you want to read
through.


Best regards,
Andreas Hindborg
Re: [PATCH] rust: hrtimer: document handle based design rationale
Posted by Boqun Feng 1 month, 1 week ago
On Wed, Feb 18, 2026 at 08:31:06PM +0100, Andreas Hindborg wrote:
> "Boqun Feng" <boqun@kernel.org> writes:
> 
> > On Sun, Feb 15, 2026 at 09:36:04PM +0100, Andreas Hindborg wrote:
> >> Add implementation notes explaining why the hrtimer abstraction uses a
> >> handle based approach.
> >>
> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >
> > Thanks for adding this!
> >
> > Reviewed-by: Boqun Feng <boqun@kernel.org>
> >
> > However I feel these are not just implementation details. They are the
> > design decision we made because of the limitation you mentioned. Maybe
> > make them "//!" doc comment and just put them as a separate section that
> > describes the necessity of handles? Thoughts?
> 
> I don't think these details are important for the reader of the API? If
> you just want to use a timer, this is not something you want to read
> through.
> 

Fair enough ;-)

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
> 
>
Re: [PATCH] rust: hrtimer: document handle based design rationale
Posted by Alice Ryhl 1 month, 2 weeks ago
On Sun, Feb 15, 2026 at 09:36:04PM +0100, Andreas Hindborg wrote:
> Add implementation notes explaining why the hrtimer abstraction uses a
> handle based approach.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>