public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rust: hrtimer: document handle based design rationale
@ 2026-02-15 20:36 Andreas Hindborg
  2026-02-16  8:38 ` Alice Ryhl
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andreas Hindborg @ 2026-02-15 20:36 UTC (permalink / raw)
  To: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
	Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
	Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich
  Cc: rust-for-linux, linux-kernel, Andreas Hindborg

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>



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] rust: hrtimer: document handle based design rationale
  2026-02-15 20:36 [PATCH] rust: hrtimer: document handle based design rationale Andreas Hindborg
@ 2026-02-16  8:38 ` Alice Ryhl
  2026-02-17 20:56 ` Boqun Feng
  2026-02-28  3:08 ` FUJITA Tomonori
  2 siblings, 0 replies; 6+ messages in thread
From: Alice Ryhl @ 2026-02-16  8:38 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
	Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
	Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rust: hrtimer: document handle based design rationale
  2026-02-15 20:36 [PATCH] rust: hrtimer: document handle based design rationale Andreas Hindborg
  2026-02-16  8:38 ` Alice Ryhl
@ 2026-02-17 20:56 ` Boqun Feng
  2026-02-18 19:31   ` Andreas Hindborg
  2026-02-28  3:08 ` FUJITA Tomonori
  2 siblings, 1 reply; 6+ messages in thread
From: Boqun Feng @ 2026-02-17 20:56 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
	Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
	Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel

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>
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rust: hrtimer: document handle based design rationale
  2026-02-17 20:56 ` Boqun Feng
@ 2026-02-18 19:31   ` Andreas Hindborg
  2026-02-18 20:15     ` Boqun Feng
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Hindborg @ 2026-02-18 19:31 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
	Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
	Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel

"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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rust: hrtimer: document handle based design rationale
  2026-02-18 19:31   ` Andreas Hindborg
@ 2026-02-18 20:15     ` Boqun Feng
  0 siblings, 0 replies; 6+ messages in thread
From: Boqun Feng @ 2026-02-18 20:15 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
	Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
	Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel

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
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rust: hrtimer: document handle based design rationale
  2026-02-15 20:36 [PATCH] rust: hrtimer: document handle based design rationale Andreas Hindborg
  2026-02-16  8:38 ` Alice Ryhl
  2026-02-17 20:56 ` Boqun Feng
@ 2026-02-28  3:08 ` FUJITA Tomonori
  2 siblings, 0 replies; 6+ messages in thread
From: FUJITA Tomonori @ 2026-02-28  3:08 UTC (permalink / raw)
  To: a.hindborg
  Cc: boqun.feng, fujita.tomonori, frederic, lyude, tglx, anna-maria,
	jstultz, sboyd, ojeda, gary, bjorn3_gh, lossin, aliceryhl,
	tmgross, dakr, rust-for-linux, linux-kernel

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-28  3:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-15 20:36 [PATCH] rust: hrtimer: document handle based design rationale Andreas Hindborg
2026-02-16  8:38 ` Alice Ryhl
2026-02-17 20:56 ` Boqun Feng
2026-02-18 19:31   ` Andreas Hindborg
2026-02-18 20:15     ` Boqun Feng
2026-02-28  3:08 ` FUJITA Tomonori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox