Re: [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector
From: Gary Guo
Date: Thu Aug 13 2026 - 13:03:02 EST
On Thu Aug 13, 2026 at 5:52 PM BST, Danilo Krummrich wrote:
> Move the pci_irq_vector() call from the TryInto<IrqRequest> impl into
> IrqVectorRegistration::index(), so the IRQ number is resolved eagerly.
>
> IrqVector now embeds the resolved IrqRequest and a reference to the
> IrqVectorRegistration. The conversion to IrqRequest is infallible, which
> removes the need for pin_init_scope() in request_irq() /
> request_threaded_irq().
>
> Tested-by: John Hubbard <jhubbard@xxxxxxxxxx>
> Inspired-by: John Hubbard <jhubbard@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/20260808031120.363869-3-jhubbard@xxxxxxxxxx/
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> ---
> rust/kernel/pci/irq.rs | 67 +++++++++++++++---------------------------
> 1 file changed, 23 insertions(+), 44 deletions(-)
>
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index daba86505cd2..81b74c4c17d9 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -68,32 +68,25 @@ const fn as_raw(self) -> u32 {
> }
> }
>
> -/// Represents an allocated IRQ vector for a specific PCI device.
> +/// A resolved IRQ vector from a PCI interrupt vector allocation.
> ///
> -/// This type ties an IRQ vector to the device it was allocated for,
> -/// ensuring the vector is only used with the correct device.
> -#[derive(Clone, Copy)]
> +/// Created by [`IrqVectorRegistration::index`] and consumed by [`Device::request_irq`] or
> +/// [`Device::request_threaded_irq`]. Borrows the [`IrqVectorRegistration`] it was derived from,
> +/// so the allocation stays live until the handler is freed.
> pub struct IrqVector<'a> {
> - dev: &'a Device<Bound>,
> + request: IrqRequest<'a>,
> reg: &'a IrqVectorRegistration<'a>,
> - index: u32,
> }
>
> impl<'a> IrqVector<'a> {
> - /// Creates a new [`IrqVector`] for the given device and index.
> + /// Creates a new [`IrqVector`] with an already resolved [`IrqRequest`].
> ///
> /// # Safety
> ///
> - /// - `index` must be a valid IRQ vector index for `reg`.
> - /// - `dev` must be the device `reg` was allocated from.
> + /// `request` must have been resolved from `reg`.
> #[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
> + unsafe fn new(request: IrqRequest<'a>, reg: &'a IrqVectorRegistration<'a>) -> Self {
> + Self { request, reg }
> }
>
> /// Returns the [`IrqVectorRegistration`] this vector was derived from.
> @@ -103,17 +96,10 @@ pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
> }
> }
>
> -impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> {
> - type Error = Error;
> -
> - fn try_into(self) -> Result<IrqRequest<'a>> {
> - // 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.dev`.
> - Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) })
> +impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
I feel that this is actually one of the prime candidate of `DerefMove` when (or
if) Rust adds that.
If we have !Leak` in the langauge, then we can drop the unsafe on
`irq::Registration::new`, then we can move that to become a method on
`IrqRequest`; if we also have `DerefMove`, then you'd be able to do
irq_vector.request_thread_irq(...)
and this will look super clean.
That said, we have neither `DerefMove` nor `!Leak`, so a unsafe constructor + a
`.into()` does sound like the best option so far. But one can dream :)
Best,
Gary
> + #[inline]
> + fn from(vector: IrqVector<'a>) -> Self {
> + vector.request
> }
> }