| 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
| ||
|
Message-ID: <87bjunr7j0.fsf@kernel.org>
Date: Thu, 27 Feb 2025 11:44:51 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Thomas Gleixner" <tglx@...utronix.de>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Anna-Maria Behnsen"
<anna-maria@...utronix.de>, "Frederic Weisbecker" <frederic@...nel.org>,
"Danilo Krummrich" <dakr@...nel.org>, "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>
Subject: Re: [PATCH v9 01/13] rust: hrtimer: introduce hrtimer support
"Thomas Gleixner" <tglx@...utronix.de> writes:
> 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/
I was made aware and have change the mood to imperative for next spin.
>
>> 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!
Thanks.
>
>> + /// 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.
Right, this was also my understanding. I can add a note about this race.
> 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.
Sounds like that would be useful.
Best regards,
Andreas Hindborg
Powered by blists - more mailing lists