[PATCH v2 0/3] Add Devres managed IRQ vectors allocation

[PATCH v2 0/3] Add Devres managed IRQ vectors allocation

[Shawn Lin]

There is a long-standing design issue in the PCI/MSI subsystem where the
implicit, automatic management of IRQ vectors by the devres framework
conflicts with explicit driver cleanup, creating ambiguity and potential
resource management bugs.

Historically, `pcim_enable_device()` not only manages standard PCI resources
(BARs) via devres but also implicitly triggers automatic IRQ vector management
by setting a flag that registers `pcim_msi_release()` as a cleanup action.

This creates an ambiguous ownership model. Many drivers follow a pattern of:
1. Calling `pci_alloc_irq_vectors()` to allocate interrupts.
2. Also calling `pci_free_irq_vectors()` in their error paths or remove routines.

When such a driver also uses `pcim_enable_device()`, the devres framework may
attempt to free the IRQ vectors a second time upon device release, leading to
a double-free. Analysis of the tree shows this hazardous pattern exists widely,
while 35 other drivers correctly rely solely on the implicit cleanup.

This series introduces new managed APIs: pcim_alloc_irq_vectors()and
pcim_alloc_irq_vectors_affinity(). Drivers that wish to have devres-managed IRQ
vectors should use these functions. They appropriately set the is_msi_managed
flag and ensure proper automatic cleanup.

In the short term, the series converts two drivers within the PCI subsystem to
use the new APIs. The long-term goal is to convert all other drivers to use these
managed functions, and finally to remove the problematic hybrid management pattern
from pcim_enable_device() entirely.


Changes in v2:
- Rebase
- Introduce patches only for PCI subsystem to convert the API

Shawn Lin (3):
  PCI/MSI: Add Devres managed IRQ vectors allocation
  PCI: switchtec: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  PCI: vmd: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()

 drivers/pci/controller/vmd.c   |  4 ++--
 drivers/pci/msi/api.c          | 26 ++++++++++++++++++++++++++
 drivers/pci/switch/switchtec.c |  6 +++---
 include/linux/pci.h            | 22 ++++++++++++++++++++++
 4 files changed, 53 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
pci device drivers which rely on the devres machinery to help cleanup the IRQ
vectors.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

Changes in v2: None

 drivers/pci/msi/api.c | 26 ++++++++++++++++++++++++++
 include/linux/pci.h   | 22 ++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
index c18559b..2362fca 100644
--- a/drivers/pci/msi/api.c
+++ b/drivers/pci/msi/api.c
@@ -297,6 +297,32 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
 
 /**
+ * pcim_alloc_irq_vectors() - devres managed pci_alloc_irq_vectors()
+ * Interrupt vectors are automatically freed by the devres machinery
+ */
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+			   unsigned int max_vecs, unsigned int flags)
+{
+	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					       flags, NULL);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors);
+
+/**
+ * pcim_alloc_irq_vectors_affinity() - devres managed pci_alloc_irq_vectors_affinity()
+ * Interrupt vectors are automatically freed by the devres machinery
+ */
+int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+				   unsigned int max_vecs, unsigned int flags,
+				   struct irq_affinity *affd)
+{
+	dev->is_msi_managed = true;
+	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					      flags, affd);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors_affinity);
+
+/**
  * pci_irq_vector() - Get Linux IRQ number of a device interrupt vector
  * @dev: the PCI device to operate on
  * @nr:  device-relative interrupt vector index (0-based); has different
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2c44545..3716c67 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1770,6 +1770,12 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 				   unsigned int max_vecs, unsigned int flags,
 				   struct irq_affinity *affd);
 
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+			  unsigned int max_vecs, unsigned int flags);
+int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+				   unsigned int max_vecs, unsigned int flags,
+				   struct irq_affinity *affd);
+
 bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
 struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
 				     const struct irq_affinity_desc *affdesc);
@@ -1812,6 +1818,22 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
 					      flags, NULL);
 }
 
+static inline int
+pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+			       unsigned int max_vecs, unsigned int flags,
+			       struct irq_affinity *aff_desc)
+{
+	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					      flags, aff_desc);
+}
+static inline int
+pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+		      unsigned int max_vecs, unsigned int flags)
+{
+	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					      flags, NULL);
+}
+
 static inline bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
 { return false; }
 static inline struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
-- 
2.7.4

Re: [PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Frank Li]

On Fri, Apr 17, 2026 at 10:26:05AM +0800, Shawn Lin wrote:
> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> pci device drivers which rely on the devres machinery to help cleanup the IRQ
> vectors.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
> Changes in v2: None
>
>  drivers/pci/msi/api.c | 26 ++++++++++++++++++++++++++
>  include/linux/pci.h   | 22 ++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
>
...
> +/**
> + * pcim_alloc_irq_vectors_affinity() - devres managed pci_alloc_irq_vectors_affinity()
> + * Interrupt vectors are automatically freed by the devres machinery
> + */
> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +				   unsigned int max_vecs, unsigned int flags,
> +				   struct irq_affinity *affd)
> +{
> +	dev->is_msi_managed = true;
> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> +					      flags, affd);

any side effect if pci_alloc_irq_vectors_affinity() return fail and
is_msi_managed keep true.

Frank
>

Re: [PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

Hi Frank

在 2026/04/17 星期五 14:10, Frank Li 写道:
> On Fri, Apr 17, 2026 at 10:26:05AM +0800, Shawn Lin wrote:
>> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
>> pci device drivers which rely on the devres machinery to help cleanup the IRQ
>> vectors.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v2: None
>>
>>   drivers/pci/msi/api.c | 26 ++++++++++++++++++++++++++
>>   include/linux/pci.h   | 22 ++++++++++++++++++++++
>>   2 files changed, 48 insertions(+)
>>
> ...
>> +/**
>> + * pcim_alloc_irq_vectors_affinity() - devres managed pci_alloc_irq_vectors_affinity()
>> + * Interrupt vectors are automatically freed by the devres machinery
>> + */
>> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>> +				   unsigned int max_vecs, unsigned int flags,
>> +				   struct irq_affinity *affd)
>> +{
>> +	dev->is_msi_managed = true;
>> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>> +					      flags, affd);
> 
> any side effect if pci_alloc_irq_vectors_affinity() return fail and
> is_msi_managed keep true.

Thanks for the review! This is a good point.

You're absolutely right that in the current implementation, if
pci_alloc_irq_vectors_affinity() fails, is_msi_managed remains true
while no vectors are actually allocated.

This replicates the behavior of pcim_enable_device() +
pci_alloc_irq_vectors_affinity() , which also sets
is_msi_managed unconditionally, but that doesn't make it ideal.

I think we should fix this for better self-contained behavior. The
pcim_* APIs should be robust on their own.

I'll move the is_msi_managed assignment to after successful allocation
in v3, if the general apporach looks feasible.


> 
> Frank
>>
> 
> 

Re: [PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Philipp Stanner]

Hello Shawn,

On Fri, 2026-04-17 at 10:26 +0800, Shawn Lin wrote:
> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> pci device drivers which rely on the devres machinery to help cleanup the IRQ
> vectors.

The commit message doesn't really detail *why* you add this. The rule
of thumb is to first describe the problem and then describe roughly
what the patch does about the problem. That also makes it easier for
reviewers to quickly match code to intent

> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

Could contain a Suggested-by: Philipp Stanner <phasta@kernel.org> ? :)

> 
> ---
> 
> Changes in v2: None
> 
>  drivers/pci/msi/api.c | 26 ++++++++++++++++++++++++++
>  include/linux/pci.h   | 22 ++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
> index c18559b..2362fca 100644
> --- a/drivers/pci/msi/api.c
> +++ b/drivers/pci/msi/api.c
> @@ -297,6 +297,32 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
>  
>  /**
> + * pcim_alloc_irq_vectors() - devres managed pci_alloc_irq_vectors()
> + * Interrupt vectors are automatically freed by the devres machinery

nit: "devres machinery" is not a very common expression. I would just
say "through devres".

For correct docstrings, though, you need to describe all the parameters
below, like 'dev', 'min_vecs' etc.

Also the return value needs to be described.

https://docs.kernel.org/doc-guide/kernel-doc.html#function-documentation

Do you do "make htmldocs"? I think that should provide warnings for
that?


> + */
> +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> +			   unsigned int max_vecs, unsigned int flags)
> +{
> +	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> +					       flags, NULL);
> +}
> +EXPORT_SYMBOL(pcim_alloc_irq_vectors);
> +
> +/**
> + * pcim_alloc_irq_vectors_affinity() - devres managed pci_alloc_irq_vectors_affinity()
> + * Interrupt vectors are automatically freed by the devres machinery
> + */

Same here with docstrings

> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +				   unsigned int max_vecs, unsigned int flags,
> +				   struct irq_affinity *affd)
> +{
> +	dev->is_msi_managed = true;

Do you have to set this to 'false' again? If so, who does that? A small
comment here could describe that.


Note that for your cleanup, in the long run, once all users are ported,
these helper bools should not be necessary anymore because devres
already stores all the relevant state.

Likewise, I would like to remove the "is_managed" and "is_pinned"
flags, but that can only be done once all API users are ported.

If this is the case with 'is_msi_managed', too, there should be a
cleanup TODO comment in my opinion. "This can be removed once…"

(But I'm not super deep in PCI devres since many months; but I think I
remember that not properly dealing with that flag state even caused us
a bug or two in the past)


Regards,
Philipp

> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> +					      flags, affd);
> +}
> +EXPORT_SYMBOL(pcim_alloc_irq_vectors_affinity);
> +
> +/**
>   * pci_irq_vector() - Get Linux IRQ number of a device interrupt vector
>   * @dev: the PCI device to operate on
>   * @nr:  device-relative interrupt vector index (0-based); has different
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c44545..3716c67 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1770,6 +1770,12 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  				   unsigned int max_vecs, unsigned int flags,
>  				   struct irq_affinity *affd);
>  
> +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> +			  unsigned int max_vecs, unsigned int flags);
> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +				   unsigned int max_vecs, unsigned int flags,
> +				   struct irq_affinity *affd);
> +
>  bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
>  struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
>  				     const struct irq_affinity_desc *affdesc);
> @@ -1812,6 +1818,22 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>  					      flags, NULL);
>  }
>  
> +static inline int
> +pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +			       unsigned int max_vecs, unsigned int flags,
> +			       struct irq_affinity *aff_desc)
> +{
> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> +					      flags, aff_desc);
> +}
> +static inline int
> +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> +		      unsigned int max_vecs, unsigned int flags)
> +{
> +	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> +					      flags, NULL);
> +}
> +
>  static inline bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
>  { return false; }
>  static inline struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,

Re: [PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

Hi Philipp

在 2026/04/17 星期五 16:44, Philipp Stanner 写道:
> Hello Shawn,
> 
> On Fri, 2026-04-17 at 10:26 +0800, Shawn Lin wrote:
>> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
>> pci device drivers which rely on the devres machinery to help cleanup the IRQ
>> vectors.
> 
> The commit message doesn't really detail *why* you add this. The rule
> of thumb is to first describe the problem and then describe roughly
> what the patch does about the problem. That also makes it easier for
> reviewers to quickly match code to intent
> 

Sure, will do.

>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> Could contain a Suggested-by: Philipp Stanner <phasta@kernel.org> ? :)
> 

Sure!

>>
>> ---
>>
>> Changes in v2: None
>>
>>   drivers/pci/msi/api.c | 26 ++++++++++++++++++++++++++
>>   include/linux/pci.h   | 22 ++++++++++++++++++++++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
>> index c18559b..2362fca 100644
>> --- a/drivers/pci/msi/api.c
>> +++ b/drivers/pci/msi/api.c
>> @@ -297,6 +297,32 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>>   EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
>>   
>>   /**
>> + * pcim_alloc_irq_vectors() - devres managed pci_alloc_irq_vectors()
>> + * Interrupt vectors are automatically freed by the devres machinery
> 
> nit: "devres machinery" is not a very common expression. I would just
> say "through devres".
> 
> For correct docstrings, though, you need to describe all the parameters
> below, like 'dev', 'min_vecs' etc.
> 
> Also the return value needs to be described.
> 
> https://docs.kernel.org/doc-guide/kernel-doc.html#function-documentation
> 
> Do you do "make htmldocs"? I think that should provide warnings for
> that?
> 


Ahh, I didn't. I will describe all the parameters.

> 
>> + */
>> +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>> +			   unsigned int max_vecs, unsigned int flags)
>> +{
>> +	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>> +					       flags, NULL);
>> +}
>> +EXPORT_SYMBOL(pcim_alloc_irq_vectors);
>> +
>> +/**
>> + * pcim_alloc_irq_vectors_affinity() - devres managed pci_alloc_irq_vectors_affinity()
>> + * Interrupt vectors are automatically freed by the devres machinery
>> + */
> 
> Same here with docstrings
> 
>> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>> +				   unsigned int max_vecs, unsigned int flags,
>> +				   struct irq_affinity *affd)
>> +{
>> +	dev->is_msi_managed = true;
> 
> Do you have to set this to 'false' again? If so, who does that? A small
> comment here could describe that.
> 
> 
> Note that for your cleanup, in the long run, once all users are ported,
> these helper bools should not be necessary anymore because devres
> already stores all the relevant state.
> 
> Likewise, I would like to remove the "is_managed" and "is_pinned"
> flags, but that can only be done once all API users are ported.
> 
> If this is the case with 'is_msi_managed', too, there should be a
> cleanup TODO comment in my opinion. "This can be removed once…"
> 

My initial plan was to make it into 3 steps:
(1) Set is_msi_managed true via pcim_alloc_irq_vectors*();
(2) All pcim_enable_device() + pci_alloc_irq_vectors*() users are ported
(3) remove is_msi_managed assignment‌ from pcim_enable_device()

As you could see, on step 1 & 2, pcim_enable_device() +
pci_alloc_irq_vectors*() users uncondictionly set is_msi_managed true.
So they don't have to set it false again. At least it keeps the previous
behaviour. For potential new pci_enable_device() +
pcim_alloc_irq_vectors*() users before all the convertion is done, It's
indeed a problem here.

Does something below make sense to you?

int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int
                                     min_vecs, unsigned int max_vecs,
                                     unsigned int flags,
                                     struct irq_affinity *affd)
{
	int nvecs;
	bool already_set_msi_managed = dev->is_msi_managed;

	if (!already_set_msi_managed)
		dev->is_msi_managed= true;

	nvecs = pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
					      flags, affd);
	if (nvecs < 0 && !already_set_msi_managed)
		dev->is_msi_managed = false;

	return nvecs;
}


Thanks.

> (But I'm not super deep in PCI devres since many months; but I think I
> remember that not properly dealing with that flag state even caused us
> a bug or two in the past)
> 
> 
> Regards,
> Philipp
> 
>> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>> +					      flags, affd);
>> +}
>> +EXPORT_SYMBOL(pcim_alloc_irq_vectors_affinity);
>> +
>> +/**
>>    * pci_irq_vector() - Get Linux IRQ number of a device interrupt vector
>>    * @dev: the PCI device to operate on
>>    * @nr:  device-relative interrupt vector index (0-based); has different
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 2c44545..3716c67 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1770,6 +1770,12 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>>   				   unsigned int max_vecs, unsigned int flags,
>>   				   struct irq_affinity *affd);
>>   
>> +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>> +			  unsigned int max_vecs, unsigned int flags);
>> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>> +				   unsigned int max_vecs, unsigned int flags,
>> +				   struct irq_affinity *affd);
>> +
>>   bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
>>   struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
>>   				     const struct irq_affinity_desc *affdesc);
>> @@ -1812,6 +1818,22 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>>   					      flags, NULL);
>>   }
>>   
>> +static inline int
>> +pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>> +			       unsigned int max_vecs, unsigned int flags,
>> +			       struct irq_affinity *aff_desc)
>> +{
>> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>> +					      flags, aff_desc);
>> +}
>> +static inline int
>> +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>> +		      unsigned int max_vecs, unsigned int flags)
>> +{
>> +	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>> +					      flags, NULL);
>> +}
>> +
>>   static inline bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
>>   { return false; }
>>   static inline struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
> 
> 

Re: [PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Philipp Stanner]

On Fri, 2026-04-17 at 17:33 +0800, Shawn Lin wrote:
> Hi Philipp
> 
> 在 2026/04/17 星期五 16:44, Philipp Stanner 写道:
> > Hello Shawn,
> > 
> > On Fri, 2026-04-17 at 10:26 +0800, Shawn Lin wrote:
> > > pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> > > pci device drivers which rely on the devres machinery to help cleanup the IRQ
> > > vectors.
> > 
> > The commit message doesn't really detail *why* you add this. The rule
> > of thumb is to first describe the problem and then describe roughly
> > what the patch does about the problem. That also makes it easier for
> > reviewers to quickly match code to intent
> > 
> 
> Sure, will do.
> > `

[…]

> > > +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> > > +				   unsigned int max_vecs, unsigned int flags,
> > > +				   struct irq_affinity *affd)
> > > +{
> > > +	dev->is_msi_managed = true;
> > 
> > Do you have to set this to 'false' again? If so, who does that? A small
> > comment here could describe that.
> > 
> > 
> > Note that for your cleanup, in the long run, once all users are ported,
> > these helper bools should not be necessary anymore because devres
> > already stores all the relevant state.
> > 
> > Likewise, I would like to remove the "is_managed" and "is_pinned"
> > flags, but that can only be done once all API users are ported.
> > 
> > If this is the case with 'is_msi_managed', too, there should be a
> > cleanup TODO comment in my opinion. "This can be removed once…"
> > 
> 
> My initial plan was to make it into 3 steps:
> (1) Set is_msi_managed true via pcim_alloc_irq_vectors*();
> (2) All pcim_enable_device() + pci_alloc_irq_vectors*() users are ported
> (3) remove is_msi_managed assignment‌ from pcim_enable_device()
> 
> As you could see, on step 1 & 2, pcim_enable_device() +
> pci_alloc_irq_vectors*() users uncondictionly set is_msi_managed true.

pcim_enable_device() does not set that flag (if you meant that; I'm not
sure).

The flag is already set by pcim_setup_msi_release(). Why do you need a
second place to set it?

> So they don't have to set it false again. At least it keeps the previous
> behaviour. For potential new pci_enable_device() +
> pcim_alloc_irq_vectors*() users before all the convertion is done, It's
> indeed a problem here.

The thing is that I believe that this should be orthogonal.

1. You'd keep the old functions exactly as they are now.

2. Then, you add new pcim_irq_vector() et. al. functions. They won't
need any flag, because the managed state is stored by the devres
backend.

3. You port a few users of the pcim_enable_device() + alloc_irq()
function combination

4. Once you have ported all of them, you can completely remove the
is_managed flag and related devres functionality from the old
interfaces.


I don't see why new code should interact with that flag mechanism. That
seems incorrect to me. Can you explain to me why you think it's
necessary?


> 
> Does something below make sense to you?
> 
> int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int
>                                      min_vecs, unsigned int max_vecs,
>                                      unsigned int flags,
>                                      struct irq_affinity *affd)
> {
> 	int nvecs;
> 	bool already_set_msi_managed = dev->is_msi_managed;
> 
> 	if (!already_set_msi_managed)
> 		dev->is_msi_managed= true;
> 
> 	nvecs = pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> 					      flags, affd);
> 	if (nvecs < 0 && !already_set_msi_managed)
> 		dev->is_msi_managed = false;

Hmm, not sure. See above.


P.

> 
> 	return nvecs;
> }
> 
> 
> Thanks.
> 
> > (But I'm not super deep in PCI devres since many months; but I think I
> > remember that not properly dealing with that flag state even caused us
> > a bug or two in the past)
> > 
> > 
> > Regards,
> > Philipp
> > 
> > > +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> > > +					      flags, affd);
> > > +}
> > > +EXPORT_SYMBOL(pcim_alloc_irq_vectors_affinity);
> > > +
> > > +/**
> > >    * pci_irq_vector() - Get Linux IRQ number of a device interrupt vector
> > >    * @dev: the PCI device to operate on
> > >    * @nr:  device-relative interrupt vector index (0-based); has different
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index 2c44545..3716c67 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -1770,6 +1770,12 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> > >   				   unsigned int max_vecs, unsigned int flags,
> > >   				   struct irq_affinity *affd);
> > >   
> > > +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> > > +			  unsigned int max_vecs, unsigned int flags);
> > > +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> > > +				   unsigned int max_vecs, unsigned int flags,
> > > +				   struct irq_affinity *affd);
> > > +
> > >   bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
> > >   struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
> > >   				     const struct irq_affinity_desc *affdesc);
> > > @@ -1812,6 +1818,22 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> > >   					      flags, NULL);
> > >   }
> > >   
> > > +static inline int
> > > +pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> > > +			       unsigned int max_vecs, unsigned int flags,
> > > +			       struct irq_affinity *aff_desc)
> > > +{
> > > +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> > > +					      flags, aff_desc);
> > > +}
> > > +static inline int
> > > +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> > > +		      unsigned int max_vecs, unsigned int flags)
> > > +{
> > > +	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
> > > +					      flags, NULL);
> > > +}
> > > +
> > >   static inline bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
> > >   { return false; }
> > >   static inline struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
> > 
> > 

Re: [PATCH v2 1/3] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

在 2026/04/20 星期一 17:28, Philipp Stanner 写道:
> On Fri, 2026-04-17 at 17:33 +0800, Shawn Lin wrote:
>> Hi Philipp
>>
>> 在 2026/04/17 星期五 16:44, Philipp Stanner 写道:
>>> Hello Shawn,
>>>
>>> On Fri, 2026-04-17 at 10:26 +0800, Shawn Lin wrote:
>>>> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
>>>> pci device drivers which rely on the devres machinery to help cleanup the IRQ
>>>> vectors.
>>>
>>> The commit message doesn't really detail *why* you add this. The rule
>>> of thumb is to first describe the problem and then describe roughly
>>> what the patch does about the problem. That also makes it easier for
>>> reviewers to quickly match code to intent
>>>
>>
>> Sure, will do.
>>> `
> 
> […]
> 
>>>> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>>>> +				   unsigned int max_vecs, unsigned int flags,
>>>> +				   struct irq_affinity *affd)
>>>> +{
>>>> +	dev->is_msi_managed = true;
>>>
>>> Do you have to set this to 'false' again? If so, who does that? A small
>>> comment here could describe that.
>>>
>>>
>>> Note that for your cleanup, in the long run, once all users are ported,
>>> these helper bools should not be necessary anymore because devres
>>> already stores all the relevant state.
>>>
>>> Likewise, I would like to remove the "is_managed" and "is_pinned"
>>> flags, but that can only be done once all API users are ported.
>>>
>>> If this is the case with 'is_msi_managed', too, there should be a
>>> cleanup TODO comment in my opinion. "This can be removed once…"
>>>
>>
>> My initial plan was to make it into 3 steps:
>> (1) Set is_msi_managed true via pcim_alloc_irq_vectors*();
>> (2) All pcim_enable_device() + pci_alloc_irq_vectors*() users are ported
>> (3) remove is_msi_managed assignment‌ from pcim_enable_device()
>>
>> As you could see, on step 1 & 2, pcim_enable_device() +
>> pci_alloc_irq_vectors*() users uncondictionly set is_msi_managed true.
> 
> pcim_enable_device() does not set that flag (if you meant that; I'm not
> sure).
> 

My bad, I meant pcim_enable_device() sets is_managed, so
pcim_setup_msi_release() would skip the check of `if 
(!pci_is_managed(dev) || dev->is_msi_managed)`, thus is_msi_managed is 
set at the end.

> The flag is already set by pcim_setup_msi_release(). Why do you need a
> second place to set it?
> 
>> So they don't have to set it false again. At least it keeps the previous
>> behaviour. For potential new pci_enable_device() +
>> pcim_alloc_irq_vectors*() users before all the convertion is done, It's
>> indeed a problem here.
> 
> The thing is that I believe that this should be orthogonal.
> 
> 1. You'd keep the old functions exactly as they are now.
> 
> 2. Then, you add new pcim_irq_vector() et. al. functions. They won't
> need any flag, because the managed state is stored by the devres
> backend.
> 
> 3. You port a few users of the pcim_enable_device() + alloc_irq()
> function combination
> 
> 4. Once you have ported all of them, you can completely remove the
> is_managed flag and related devres functionality from the old
> interfaces.
> 
> 
> I don't see why new code should interact with that flag mechanism. That
> seems incorrect to me. Can you explain to me why you think it's
> necessary?
> 
> 

I was thinking if we need to unset the flag but thinking more about it,
the answer seems no, you're right, the devres backend manages it
properly.

Thanks, Philipp!

>>
>> Does something below make sense to you?
>>
>> int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int
>>                                       min_vecs, unsigned int max_vecs,
>>                                       unsigned int flags,
>>                                       struct irq_affinity *affd)
>> {
>> 	int nvecs;
>> 	bool already_set_msi_managed = dev->is_msi_managed;
>>
>> 	if (!already_set_msi_managed)
>> 		dev->is_msi_managed= true;
>>
>> 	nvecs = pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>> 					      flags, affd);
>> 	if (nvecs < 0 && !already_set_msi_managed)
>> 		dev->is_msi_managed = false;
> 
> Hmm, not sure. See above.
> 
> 
> P.
> 
>>
>> 	return nvecs;
>> }
>>
>>
>> Thanks.
>>
>>> (But I'm not super deep in PCI devres since many months; but I think I
>>> remember that not properly dealing with that flag state even caused us
>>> a bug or two in the past)
>>>
>>>
>>> Regards,
>>> Philipp
>>>
>>>> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>>>> +					      flags, affd);
>>>> +}
>>>> +EXPORT_SYMBOL(pcim_alloc_irq_vectors_affinity);
>>>> +
>>>> +/**
>>>>     * pci_irq_vector() - Get Linux IRQ number of a device interrupt vector
>>>>     * @dev: the PCI device to operate on
>>>>     * @nr:  device-relative interrupt vector index (0-based); has different
>>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>>> index 2c44545..3716c67 100644
>>>> --- a/include/linux/pci.h
>>>> +++ b/include/linux/pci.h
>>>> @@ -1770,6 +1770,12 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>>>>    				   unsigned int max_vecs, unsigned int flags,
>>>>    				   struct irq_affinity *affd);
>>>>    
>>>> +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>>>> +			  unsigned int max_vecs, unsigned int flags);
>>>> +int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>>>> +				   unsigned int max_vecs, unsigned int flags,
>>>> +				   struct irq_affinity *affd);
>>>> +
>>>>    bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
>>>>    struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
>>>>    				     const struct irq_affinity_desc *affdesc);
>>>> @@ -1812,6 +1818,22 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>>>>    					      flags, NULL);
>>>>    }
>>>>    
>>>> +static inline int
>>>> +pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>>>> +			       unsigned int max_vecs, unsigned int flags,
>>>> +			       struct irq_affinity *aff_desc)
>>>> +{
>>>> +	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>>>> +					      flags, aff_desc);
>>>> +}
>>>> +static inline int
>>>> +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>>>> +		      unsigned int max_vecs, unsigned int flags)
>>>> +{
>>>> +	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
>>>> +					      flags, NULL);
>>>> +}
>>>> +
>>>>    static inline bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
>>>>    { return false; }
>>>>    static inline struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
>>>
>>>
> 
> 

[PATCH v2 2/3] PCI: switchtec: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()

[Shawn Lin]

Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors() to
explicitly request devres-managed interrupt vectors. This makes the
driver's intention clear and avoids the ambiguous implicit management
previously provided by pcim_enable_device().

The change prepares the driver for the eventual removal of the hybrid
IRQ management pattern from pcim_enable_device(), ensuring consistent
resource management through devres.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

 drivers/pci/switch/switchtec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 93ebec9..ed42661 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1493,9 +1493,9 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
 	if (nirqs < 4)
 		nirqs = 4;
 
-	nvecs = pci_alloc_irq_vectors(stdev->pdev, 1, nirqs,
-				      PCI_IRQ_MSIX | PCI_IRQ_MSI |
-				      PCI_IRQ_VIRTUAL);
+	nvecs = pcim_alloc_irq_vectors(stdev->pdev, 1, nirqs,
+				       PCI_IRQ_MSIX | PCI_IRQ_MSI |
+				       PCI_IRQ_VIRTUAL);
 	if (nvecs < 0)
 		return nvecs;
 
-- 
2.7.4

Re: [PATCH v2 2/3] PCI: switchtec: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()

[Logan Gunthorpe]

On 2026-04-16 8:26 p.m., Shawn Lin wrote:
> Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors() to
> explicitly request devres-managed interrupt vectors. This makes the
> driver's intention clear and avoids the ambiguous implicit management
> previously provided by pcim_enable_device().
> 
> The change prepares the driver for the eventual removal of the hybrid
> IRQ management pattern from pcim_enable_device(), ensuring consistent
> resource management through devres.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

This direction makes sense to me. I remember thinking it was odd that
pci_alloc_ir_vectors() was applicable to managed cases without the pcim
prefix. Thanks!

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

[PATCH v2 3/3] PCI: vmd: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()

[Shawn Lin]

Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors() to
explicitly request devres-managed interrupt vectors. This makes the
driver's intention clear and avoids the ambiguous implicit management
previously provided by pcim_enable_device().

The change prepares the driver for the eventual removal of the hybrid
IRQ management pattern from pcim_enable_device(), ensuring consistent
resource management through devres.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

 drivers/pci/controller/vmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250..d8de63a 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -684,8 +684,8 @@ static int vmd_alloc_irqs(struct vmd_dev *vmd)
 	if (vmd->msix_count < 0)
 		return -ENODEV;
 
-	vmd->msix_count = pci_alloc_irq_vectors(dev, vmd->first_vec + 1,
-						vmd->msix_count, PCI_IRQ_MSIX);
+	vmd->msix_count = pcim_alloc_irq_vectors(dev, vmd->first_vec + 1,
+						 vmd->msix_count, PCI_IRQ_MSIX);
 	if (vmd->msix_count < 0)
 		return vmd->msix_count;
 
-- 
2.7.4