Hi Danilo,
On 10/5/2025 8:56 AM, Danilo Krummrich wrote:
>
>> + /// Allocate IRQ vectors for this PCI device with automatic cleanup.
>> + ///
>> + /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for
>> the device.
>> + /// The allocation will use MSI-X, MSI, or legacy 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
>> legacy interrupts.
>> + ///
>> + /// The allocated vectors are automatically freed when the device is
>> unbound, using the
>> + /// devres (device resource management) system.
>> + ///
>> + /// # Arguments
>> + ///
>> + /// * `min_vecs` - Minimum number of vectors required
>> + /// * `max_vecs` - Maximum number of vectors to allocate
>> + /// * `irq_types` - Types of interrupts that can be used
>> + ///
>> + /// # Returns
>> + ///
>> + /// Returns a range of IRQ vectors that were successfully allocated, or
>> an error if the
>> + /// allocation fails or cannot meet the minimum requirement.
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```ignore
>> + /// // Allocate using any available interrupt type in the order
>> mentioned above.
>> + /// let vectors = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
>> + ///
>> + /// // Allocate MSI or MSI-X only (no legacy interrupts)
>> + /// let msi_only = IrqTypes::default()
>> + /// .with(IrqType::Msi)
>> + /// .with(IrqType::MsiX);
>> + /// let vectors = dev.alloc_irq_vectors(4, 16, msi_only)?;
>> + /// ```
>> + pub fn alloc_irq_vectors(
>> + &self,
>> + min_vecs: u32,
>> + max_vecs: u32,
>> + irq_types: IrqTypes,
>> + ) -> Result<RangeInclusive<IrqVector<'_>>> {
>> + let (irq_vecs, range) = IrqVectorRegistration::new(self, min_vecs,
>> max_vecs, irq_types)?;
>> +
>> + devres::register(self.as_ref(), irq_vecs, GFP_KERNEL)?;
>> If we move the call to devres::register() into IrqVectorRegistration::new()
> (which I'd call IrqVectorRegistration::register() then) we can enforce the
> guarantee that an IrqVectorRegistration must not out-live the device / driver
> binding internally.
Great idea, so paraphrasing for myself, your point is with the above code,
someone could theoretically do:
1. Call new() directly on IrqVectorRegistration (bypassing
alloc_irq_vectors()).
2. Forget to call devres::register().
3. Store the IrqVectorRegistration somewhere.
4. Device gets unbound.
5. Later when IrqVectorRegistration::drop() runs, it tries to free vectors on
a device that's gone.
Is that right?
So a better approach as you mentioned, is to do the devres registration during
the construction of the IrqVectorRegistration, so there's no way to do one
without the other. Did I get that right? Anyway great point and I have made this
change, thanks!
- Joel
