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

[Alice Ryhl <aliceryhl@google.com>]

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@nvidia.com>
> Tested-by: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.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