Re: [PATCH 2/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type
"Gary Guo" <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,dev.linux.lists.driver-core,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <[email protected]> |
On Mon Aug 10, 2026 at 11:47 PM 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. This reference flows through IrqRequest (via the IrqRequestAnchor > generic) into irq::Registration, creating a borrow chain that prevents > the vector allocation from being dropped while any handler is still > registered. > > alloc_irq_vectors() returns IrqVectorRegistration<'_> directly, giving > drivers explicit control over the allocation lifetime. This is also > needed by e.g. net and block drivers that re-allocate vectors, e.g. > during queue reconfiguration or device recovery. > > Signed-off-by: Danilo Krummrich <[email protected]> > --- > rust/kernel/pci.rs | 3 +- > rust/kernel/pci/irq.rs | 149 ++++++++++++++++++++++------------------- > 2 files changed, 83 insertions(+), 69 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..38a7d72dcda7 100644 > --- a/rust/kernel/pci/irq.rs > +++ b/rust/kernel/pci/irq.rs > @@ -7,17 +7,15 @@ > bindings, > device, > device::Bound, > - devres, > error::to_result, > irq::{ > self, > - IrqRequest, // > + IrqRequest, > + IrqRequestAnchor, // > }, > - 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 +76,7 @@ const fn as_raw(self) -> u32 { > #[derive(Clone, Copy)] > pub struct IrqVector<'a> { > dev: &'a Device<Bound>, > + reg: &'a IrqVectorRegistration<'a>, > index: u32, > } > > @@ -86,10 +85,11 @@ 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. > @@ -98,75 +98,72 @@ fn index(&self) -> u32 { > } > } > > -impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> { > +impl IrqRequestAnchor for &IrqVectorRegistration<'_> {} > + > +impl<'a> IrqRequest<'a, &'a IrqVectorRegistration<'a>> { > + /// Returns the [`IrqVectorRegistration`] this request was derived from. > + pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> { > + self.anchor() > + } > +} > + > +impl<'a> TryInto<IrqRequest<'a, &'a IrqVectorRegistration<'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`. > + fn try_into(self) -> Result<IrqRequest<'a, &'a IrqVectorRegistration<'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`. > - Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) }) > + // SAFETY: `irq` is guaranteed to be a valid IRQ number for `self.dev`. > + Ok(unsafe { IrqRequest::new_anchored(self.dev.as_ref(), irq as u32, self.reg) }) Why is this anchoring thing needed instead apart from the lifetime that already exists on `IrqRequest`? Best, Gary > } > } > > -/// 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>, > }