Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

["Gary Guo" <gary@garyguo.net>]

On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
> Convert IrqVectorRegistration from a devres-managed internal type to a
> lifetime-annotated type that owns the PCI interrupt vector allocation.
> Dropping it frees the vectors.
>
> IrqVector gains a reference to the IrqVectorRegistration it was derived
> from. Since vector() borrows the registration, the compiler prevents the
> allocation from being dropped while any IrqVector (and hence any
> irq::Registration built from it) is still live.
>
> alloc_irq_vectors() returns IrqVectorRegistration<'_> directly, giving
> drivers explicit control over the allocation lifetime, which is needed
> by net and block drivers that re-allocate vectors at runtime, e.g.
> during queue reconfiguration or device recovery.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/pci.rs     |   3 +-
>  rust/kernel/pci/irq.rs | 127 ++++++++++++++++++++++-------------------
>  2 files changed, 69 insertions(+), 61 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index c6417af2bb17..2757a0cc0f11 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -51,7 +51,8 @@
>  pub use self::irq::{
>      IrqType,
>      IrqTypes,
> -    IrqVector, //
> +    IrqVector,
> +    IrqVectorRegistration, //
>  };
>  
>  /// An adapter for the registration of PCI drivers.
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index fea484dcf9cf..8e0651587829 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -7,17 +7,14 @@
>      bindings,
>      device,
>      device::Bound,
> -    devres,
>      error::to_result,
>      irq::{
>          self,
>          IrqRequest, //
>      },
> -    prelude::*,
> -    str::CStr,
> -    sync::aref::ARef, //
> +    prelude::*, //
>  };
> -use core::ops::RangeInclusive;
> +use core::num::NonZero;
>  
>  /// IRQ type flags for PCI interrupt allocation.
>  #[derive(Debug, Clone, Copy)]
> @@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
>  #[derive(Clone, Copy)]
>  pub struct IrqVector<'a> {
>      dev: &'a Device<Bound>,
> +    reg: &'a IrqVectorRegistration<'a>,

The registration has a refence to the device so we don't need to keep both reg
and dev?

>      index: u32,
>  }
>  
> @@ -86,87 +84,81 @@ impl<'a> IrqVector<'a> {
>      ///
>      /// # Safety
>      ///
> -    /// - `index` must be a valid IRQ vector index for `dev`.
> -    /// - `dev` must point to a [`Device`] that has successfully allocated IRQ vectors.
> -    unsafe fn new(dev: &'a Device<Bound>, index: u32) -> Self {
> -        Self { dev, index }
> +    /// - `index` must be a valid IRQ vector index for `reg`.
> +    /// - `dev` must be the device `reg` was allocated from.
> +    #[inline]
> +    unsafe fn new(dev: &'a Device<Bound>, reg: &'a IrqVectorRegistration<'a>, index: u32) -> Self {
> +        Self { dev, reg, index }
>      }
>  
>      /// Returns the raw vector index.
>      fn index(&self) -> u32 {
>          self.index
>      }
> +
> +    /// Returns the [`IrqVectorRegistration`] this vector was derived from.

#[inline]

> +    pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
> +        self.reg
> +    }
>  }
>  
>  impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> {
>      type Error = Error;
>  
>      fn try_into(self) -> Result<IrqRequest<'a>> {
> -        // SAFETY: `self.as_raw` returns a valid pointer to a `struct pci_dev`.
> +        // SAFETY: `self.dev.as_raw()` returns a valid pointer to a `struct pci_dev`.
>          let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), self.index()) };
>          if irq < 0 {
>              return Err(crate::error::Error::from_errno(irq));
>          }
> -        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
> +        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `self.dev`.
>          Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) })
>      }
>  }
>  
> -/// Represents an IRQ vector allocation for a PCI device.
> +/// An allocation of PCI interrupt vectors for a device.
>  ///
> -/// This type ensures that IRQ vectors are properly allocated and freed by
> -/// tying the allocation to the lifetime of this registration object.
> +/// This type owns the vector allocation; dropping it frees the vectors. IRQ handlers borrow from
> +/// this registration and must be dropped before it is.
>  ///
>  /// # Invariants
>  ///
> -/// The [`Device`] has successfully allocated IRQ vectors.
> -struct IrqVectorRegistration {
> -    dev: ARef<Device>,
> +/// `dev` has an allocation of `count` interrupt vectors.
> +pub struct IrqVectorRegistration<'a> {
> +    dev: &'a Device<Bound>,
> +    count: NonZero<usize>,

I wonder if it should be called "len" as I view this as a collection of IRQ
vectors.

>  }
>  
> -impl IrqVectorRegistration {
> -    /// Allocate and register IRQ vectors for the given PCI device.
> +impl<'a> IrqVectorRegistration<'a> {
> +    /// Returns the number of allocated vectors.
>      ///
> -    /// Allocates IRQ vectors and registers them with devres for automatic cleanup.
> -    /// Returns a range of valid IRQ vectors.
> -    fn register<'a>(
> -        dev: &'a Device<Bound>,
> -        min_vecs: u32,
> -        max_vecs: u32,
> -        irq_types: IrqTypes,
> -    ) -> Result<RangeInclusive<IrqVector<'a>>> {
> -        // SAFETY:
> -        // - `dev.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
> -        //   by the type invariant of `Device`.
> -        // - `pci_alloc_irq_vectors` internally validates all other parameters
> -        //   and returns error codes.
> -        let ret = unsafe {
> -            bindings::pci_alloc_irq_vectors(dev.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
> -        };
> -
> -        to_result(ret)?;
> -        let count = ret as u32;
> -
> -        // SAFETY:
> -        // - `pci_alloc_irq_vectors` returns the number of allocated vectors on success.
> -        // - Vectors are 0-based, so valid indices are [0, count-1].
> -        // - `pci_alloc_irq_vectors` guarantees `count >= min_vecs > 0`, so both `0` and
> -        //   `count - 1` are valid IRQ vector indices for `dev`.
> -        let range = unsafe { IrqVector::new(dev, 0)..=IrqVector::new(dev, count - 1) };
> +    /// This is at least the `min_vecs` that [`Device::alloc_irq_vectors`] was asked for.
> +    #[inline]
> +    pub fn vector_count(&self) -> usize {
> +        self.count.get()
> +    }
>  
> -        // INVARIANT: The IRQ vector allocation for `dev` above was successful.
> -        let irq_vecs = Self { dev: dev.into() };
> -        devres::register(dev.as_ref(), irq_vecs, GFP_KERNEL)?;
> +    /// Returns the [`IrqVector`] at `index`.
> +    ///
> +    /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation
> +    /// remains live while any handler is registered on it.

nit: I think this is redundant information as it's just stating what the
signature already conveys.

> +    #[inline]
> +    pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
> +        if index >= self.count.get() {
> +            return Err(EINVAL);
> +        }

Given that the error is for out-of-bound access only, perhaps return `Option`
like `get()` function of various containers?

>  
> -        Ok(range)
> +        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
> +        // the device it was allocated from.
> +        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>      }
>  }
>  
> -impl Drop for IrqVectorRegistration {
> +impl Drop for IrqVectorRegistration<'_> {
> +    #[inline]
>      fn drop(&mut self) {
> -        // SAFETY:
> -        // - By the type invariant, `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
> -        // - `self.dev` has successfully allocated IRQ vectors.
> +        // SAFETY: By the type invariant, `self.dev.as_raw()` is a valid pointer to a
> +        // `struct pci_dev` that has successfully allocated IRQ vectors.
>          unsafe { bindings::pci_free_irq_vectors(self.dev.as_raw()) };
>      }
>  }
> @@ -214,15 +206,16 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
>          })
>      }
>  
> -    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
> +    /// Allocate IRQ vectors for this PCI device.
>      ///
>      /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
>      /// The allocation will use MSI-X, MSI, or INTx interrupts based on the `irq_types`
>      /// parameter and hardware capabilities. When multiple types are specified, the kernel
>      /// will try them in order of preference: MSI-X first, then MSI, then INTx interrupts.
>      ///
> -    /// The allocated vectors are automatically freed when the device is unbound, using the
> -    /// devres (device resource management) system.
> +    /// The allocated vectors are freed when the returned [`IrqVectorRegistration`] is dropped.
> +    /// IRQ handlers registered via [`Self::request_irq`] or [`Self::request_threaded_irq`]
> +    /// borrow from the registration, so the compiler ensures they are freed first.
>      ///
>      /// # Arguments
>      ///
> @@ -232,8 +225,8 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
>      ///
>      /// # Returns
>      ///
> -    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
> -    /// allocation fails or cannot meet the minimum requirement.
> +    /// Returns the IRQ vector registration, or an error if `min_vecs` vectors cannot be
> +    /// allocated.
>      ///
>      /// # Examples
>      ///
> @@ -256,7 +249,21 @@ pub fn alloc_irq_vectors(
>          min_vecs: u32,
>          max_vecs: u32,
>          irq_types: IrqTypes,
> -    ) -> Result<RangeInclusive<IrqVector<'_>>> {
> -        IrqVectorRegistration::register(self, min_vecs, max_vecs, irq_types)
> +    ) -> Result<IrqVectorRegistration<'_>> {
> +        // SAFETY:
> +        // - `self.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
> +        //   by the type invariant of `Device`.
> +        // - `pci_alloc_irq_vectors` internally validates all other parameters
> +        //   and returns error codes.
> +        let ret = unsafe {
> +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
> +        };
> +
> +        to_result(ret)?;
> +
> +        let count = NonZero::new(ret as usize).ok_or(EINVAL)?;

I don't think `ret` can ever be zero. `expect` or `new_unchecked()` perhaps?

Best,
Gary

> +
> +        // INVARIANT: `pci_alloc_irq_vectors()` allocated `count` vectors for `self`.
> +        Ok(IrqVectorRegistration { dev: self, count })
>      }
>  }