[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87o6yneqkz.ffs@tglx>
Date: Thu, 27 Feb 2025 09:31:40 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Andreas Hindborg <a.hindborg@...nel.org>, Miguel Ojeda
<ojeda@...nel.org>, Anna-Maria Behnsen <anna-maria@...utronix.de>,
Frederic Weisbecker <frederic@...nel.org>, Danilo Krummrich
<dakr@...nel.org>
Cc: Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>, Lyude Paul <lyude@...hat.com>, Guangbo
Cui <2407018371@...com>, Dirk Behme <dirk.behme@...il.com>, Daniel Almeida
<daniel.almeida@...labora.com>, Tamir Duberstein <tamird@...il.com>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, Andreas
Hindborg <a.hindborg@...nel.org>
Subject: Re: [PATCH v9 01/13] rust: hrtimer: introduce hrtimer support
On Mon, Feb 24 2025 at 13:03, Andreas Hindborg wrote:
> This patch adds support for intrusive use of the hrtimer system. For
> now,
git grep 'This patch' Documentation/process/
> only one timer can be embedded in a Rust struct.
>
> +//! ## State Diagram
> +//!
> +//! ```text
> +//! Return NoRestart
> +//! +---------------------------------------------------------------------+
> +//! | |
> +//! | |
> +//! | |
> +//! | Return Restart |
> +//! | +------------------------+ |
> +//! | | | |
> +//! | | | |
> +//! v v | |
> +//! +-----------------+ Start +------------------+ +--------+-----+--+
> +//! | +---------------->| | | |
> +//! Init | | | | Expire | |
> +//! --------->| Stopped | | Started +---------->| Running |
> +//! | | Cancel | | | |
> +//! | |<----------------+ | | |
> +//! +-----------------+ +---------------+--+ +-----------------+
> +//! ^ |
> +//! | |
> +//! +---------+
> +//! Restart
> +//! ```
> +//!
> +//!
> +//! A timer is initialized in the **stopped** state. A stopped timer can be
> +//! **started** by the `start` operation, with an **expiry** time. After the
> +//! `start` operation, the timer is in the **started** state. When the timer
> +//! **expires**, the timer enters the **running** state and the handler is
> +//! executed. After the handler has finished executing, the timer may enter the
> +//! **started* or **stopped** state, depending on the return value of the
> +//! handler. A running timer can be **canceled** by the `cancel` operation. A
> +//! timer that is cancelled enters the **stopped** state.
> +//!
> +//! A `cancel` or `restart` operation on a timer in the **running** state takes
> +//! effect after the handler has finished executing and the timer has transitioned
> +//! out of the **running** state.
> +//!
> +//! A `restart` operation on a timer in the **stopped** state is equivalent to a
> +//! `start` operation.
Nice explanation!
> + /// Cancel an initialized and potentially running timer.
> + ///
> + /// If the timer handler is running, this will block until the handler is
> + /// finished.
> + ///
> + /// Users of the `HrTimer` API would not usually call this method directly.
> + /// Instead they would use the safe [`HrTimerHandle::cancel`] on the handle
> + /// returned when the timer was started.
> + ///
> + /// This function does not create any references.
> + ///
> + /// # Safety
> + ///
> + /// `self_ptr` must point to a valid `Self`.
> + #[allow(dead_code)]
> + pub(crate) unsafe fn raw_cancel(self_ptr: *const Self) -> bool {
> + // SAFETY: timer_ptr points to an allocation of at least `HrTimer` size.
> + let c_timer_ptr = unsafe { HrTimer::raw_get(self_ptr) };
> +
> + // If the handler is running, this will wait for the handler to finish
> + // before returning.
> + // SAFETY: `c_timer_ptr` is initialized and valid. Synchronization is
> + // handled on C side.
You might want to be more explicit about the provided synchronization.
The hrtimer core only guarantees that operations on the hrtimer object
are strictly serialized. But it does not provide any guarantee about
external concurrency. The following case cannot be handled by the core:
T0 T1
cancel() start()
lock()
.... lock() <- contended
dequeue()
unlock()
enqueue()
unlock()
So there is no guarantee for T0 that the timer is actually canceled in
this case. The hrtimer core can do nothing about this, that's a problem
of the call sites.
We've implemented timer_shutdown() for the timer wheel timers, which
prevents that the timer can be started after shutdown() succeeds. It
might be a good thing to implement this for hrtimers as well.
Thanks,
tglx
Powered by blists - more mailing lists