Re: [RFC RESEND v10 04/14] rust: Introduce interrupt module

["Benno Lossin" <lossin@kernel.org>]

On Wed May 28, 2025 at 12:21 AM CEST, Lyude Paul wrote:
> This introduces a module for dealing with interrupt-disabled contexts,
> including the ability to enable and disable interrupts along with the
> ability to annotate functions as expecting that IRQs are already
> disabled on the local CPU.
>
> [Boqun: This is based on Lyude's work on interrupt disable abstraction,
> I port to the new local_interrupt_disable() mechanism to make it work
> as a guard type. I cannot even take the credit of this design, since
> Lyude also brought up the same idea in zulip. Anyway, this is only for
> POC purpose, and of course all bugs are mine]
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Co-Developed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Two nits below, with those fixed:

Reviewed-by: Benno Lossin <lossin@kernel.org>

> diff --git a/rust/kernel/interrupt.rs b/rust/kernel/interrupt.rs
> new file mode 100644
> index 0000000000000..e66aa85f79940
> --- /dev/null
> +++ b/rust/kernel/interrupt.rs
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Interrupt controls
> +//!
> +//! This module allows Rust code to annotate areas of code where local processor interrupts should
> +//! be disabled, along with actually disabling local processor interrupts.
> +//!
> +//! # ⚠️ Warning! ⚠️
> +//!
> +//! The usage of this module can be more complicated than meets the eye, especially surrounding
> +//! [preemptible kernels]. It's recommended to take care when using the functions and types defined
> +//! here and familiarize yourself with the various documentation we have before using them, along
> +//! with the various documents we link to here.
> +//!
> +//! # Reading material
> +//!
> +//! - [Software interrupts and realtime (LWN)](https://lwn.net/Articles/520076)
> +//!
> +//! [preemptible kernels]: https://www.kernel.org/doc/html/latest/locking/preempt-locking.html
> +
> +use bindings;

This shouldn't be necessary, right?

> +impl LocalInterruptDisabled {
> +    const ASSUME_DISABLED: &'static LocalInterruptDisabled = &LocalInterruptDisabled(NotThreadSafe);

I'd move this into the function body.

---
Cheers,
Benno

> +
> +    /// Assume that local processor interrupts are disabled on preemptible kernels.
> +    ///
> +    /// This can be used for annotating code that is known to be run in contexts where local
> +    /// processor interrupts are disabled on preemptible kernels. It makes no changes to the local
> +    /// interrupt state on its own.
> +    ///
> +    /// # Safety
> +    ///
> +    /// For the whole life `'a`, local interrupts must be disabled on preemptible kernels. This
> +    /// could be a context like for example, an interrupt handler.
> +    pub unsafe fn assume_disabled<'a>() -> &'a LocalInterruptDisabled {
> +        Self::ASSUME_DISABLED
> +    }
> +}