lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ