linux-kernel - Re: [PATCH v8 4/6] rust: irq: add support for threaded IRQs and handlers

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: <aJnNxMmWarz9MWKH@google.com>
Date: Mon, 11 Aug 2025 11:02:28 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...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>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	"Krzysztof Wilczyński" <kwilczynski@...nel.org>, Benno Lossin <lossin@...nel.org>, linux-kernel@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, linux-pci@...r.kernel.org, 
	Joel Fernandes <joelagnelf@...dia.com>, Dirk Behme <dirk.behme@...bosch.com>
Subject: Re: [PATCH v8 4/6] rust: irq: add support for threaded IRQs and handlers

On Sun, Aug 10, 2025 at 09:32:17PM -0300, Daniel Almeida wrote:
> This patch adds support for threaded IRQs and handlers through
> irq::ThreadedRegistration and the irq::ThreadedHandler trait.
> 
> Threaded interrupts are more permissive in the sense that further
> processing is possible in a kthread. This means that said execution takes
> place outside of interrupt context, which is rather restrictive in many
> ways.
> 
> Registering a threaded irq is dependent upon having an IrqRequest that
> was previously allocated by a given device. This will be introduced in
> subsequent patches.
> 
> Tested-by: Joel Fernandes <joelagnelf@...dia.com>
> Tested-by: Dirk Behme <dirk.behme@...bosch.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>

Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>

> +/// # Examples
> +///
> +/// The following is an example of using [`ThreadedRegistration`]. It uses a
> +/// [`Mutex`](kernel::sync::Mutex) to provide interior mutability.
> +///
> +/// ```
> +/// # use kernel::c_str;
> +/// # use kernel::device::Bound;
> +/// # use kernel::irq::{
> +/// #   self, Flags, IrqRequest, IrqReturn, ThreadedHandler, ThreadedIrqReturn,
> +/// #   ThreadedRegistration,
> +/// # };
> +/// # use kernel::prelude::*;
> +/// # use kernel::sync::{Arc, Mutex};

I would probably remove the # and keep imports visible in the example.

> +/// // Declare a struct that will be passed in when the interrupt fires. The u32
> +/// // merely serves as an example of some internal data.
> +/// //
> +/// // [`irq::ThreadedHandler::handle`] takes `&self`. This example
> +/// // illustrates how interior mutability can be used when sharing the data
> +/// // between process context and IRQ context.
> +/// struct Data {
> +///     value: Mutex<u32>,
> +/// }

This example struct should use #[pin_data].

> +///
> +/// type Handler = Data;

I think the type alias is confusing in this example. I'd either rename
the struct or use "Data" everywhere.

> +/// impl ThreadedHandler for Handler {
> +///     // This will run (in a separate kthread) if and only if
> +///     // [`ThreadedHandler::handle`] returns [`WakeThread`], which it does by
> +///     // default.
> +///     fn handle_threaded(&self) -> IrqReturn {
> +///         let mut data = self.value.lock();
> +///         *data += 1;
> +///         IrqReturn::Handled
> +///     }
> +/// }
> +///
> +/// // Registers a threaded IRQ handler for the given [`IrqRequest`].
> +/// //
> +/// // This is executing in process context and assumes that `request` was
> +/// // previously acquired from a device.
> +/// fn register_threaded_irq(
> +///     handler: impl PinInit<Handler, Error>,
> +///     request: IrqRequest<'_>,
> +/// ) -> Result<Arc<ThreadedRegistration<Handler>>> {
> +///     let registration =
> +///         ThreadedRegistration::new(request, Flags::SHARED, c_str!("my_device"), handler);
> +///
> +///     let registration = Arc::pin_init(registration, GFP_KERNEL)?;
> +///
> +///     {
> +///         // The data can be accessed from process context too.
> +///         let mut data = registration.handler().value.lock();
> +///         *data += 1;
> +///     }
> +///
> +///     Ok(registration)
> +/// }
> +/// # Ok::<(), Error>(())
> +/// ```

Alice