public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux•intel.com>
To: Shawn Lin <shawn.lin@rock-chips•com>
Cc: Andy Shevchenko <andy.shevchenko@gmail•com>,
	Bjorn Helgaas <bhelgaas@google•com>,
	"Vaibhaav Ram T . L" <vaibhaavram.tl@microchip•com>,
	Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip•com>,
	Even Xu <even.xu@intel•com>, Xinpeng Sun <xinpeng.sun@intel•com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux•intel.com>,
	Jiri Kosina <jikos@kernel•org>,
	Alexandre Belloni <alexandre.belloni@bootlin•com>,
	Zhou Wang <wangzhou1@hisilicon•com>,
	Longfang Liu <liulongfang@huawei•com>,
	Vinod Koul <vkoul@kernel•org>, Lee Jones <lee@kernel•org>,
	Jijie Shao <shaojijie@huawei•com>,
	Jian Shen <shenjian15@huawei•com>,
	Sunil Goutham <sgoutham@marvell•com>,
	Andrew Lunn <andrew+netdev@lunn•ch>,
	Heiner Kallweit <hkallweit1@gmail•com>,
	"David S . Miller" <davem@davemloft•net>,
	Jeff Hugo <jeff.hugo@oss•qualcomm.com>,
	Oded Gabbay <ogabbay@kernel•org>,
	Maciej Falkowski <maciej.falkowski@linux•intel.com>,
	Karol Wachowski <karol.wachowski@linux•intel.com>,
	Min Ma <mamin506@gmail•com>, Lizhi Hou <lizhi.hou@amd•com>,
	Andreas Noever <andreas.noever@gmail•com>,
	Mika Westerberg <westeri@kernel•org>,
	Tomasz Jeznach <tjeznach@rivosinc•com>,
	Will Deacon <will@kernel•org>,
	Xinliang Liu <xinliang.liu@linaro•org>,
	Tian Tao <tiantao6@hisilicon•com>,
	Davidlohr Bueso <dave@stgolabs•net>,
	Jonathan Cameron <jonathan.cameron@huawei•com>,
	Srujana Challa <schalla@marvell•com>,
	Bharat Bhushan <bbhushan2@marvell•com>,
	Antoine Tenart <atenart@kernel•org>,
	Herbert Xu <herbert@gondor•apana.org.au>,
	Raag Jadav <raag.jadav@intel•com>,
	Hans de Goede <hansg@kernel•org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
	Jiri Slaby <jirislaby@kernel•org>,
	Andy Shevchenko <andy@kernel•org>,
	Manivannan Sadhasivam <mani@kernel•org>,
	Mika Westerberg <mika.westerberg@linux•intel.com>,
	Andi Shyti <andi.shyti@kernel•org>,
	Robert Richter <rric@kernel•org>, Mark Brown <broonie@kernel•org>,
	Nirmal Patel <nirmal.patel@linux•intel.com>,
	Kurt Schwemmer <kurt.schwemmer@microsemi•com>,
	Logan Gunthorpe <logang@deltatee•com>,
	Linus Walleij <linusw@kernel•org>,
	Bartosz Golaszewski <brgl@kernel•org>,
	Sakari Ailus <sakari.ailus@linux•intel.com>,
	Bingbu Cao <bingbu.cao@intel•com>,
	Ulf Hansson <ulf.hansson@linaro•org>,
	Arnd Bergmann <arnd@arndb•de>,
	Benjamin Tissoires <bentiss@kernel•org>,
	linux-input@vger•kernel.org, linux-i3c@lists•infradead.org,
	dmaengine@vger•kernel.org, Philipp Stanner <phasta@kernel•org>,
	netdev@vger•kernel.org, nic_swsd@realtek•com,
	linux-arm-msm@vger•kernel.org, dri-devel@lists•freedesktop.org,
	linux-usb@vger•kernel.org, iommu@lists•linux.dev,
	linux-riscv@lists•infradead.org, David Airlie <airlied@gmail•com>,
	Simona Vetter <simona@ffwll•ch>,
	linux-cxl@vger•kernel.org, linux-crypto@vger•kernel.org,
	platform-driver-x86@vger•kernel.org,
	linux-serial@vger•kernel.org, mhi@lists•linux.dev,
	Jan Dabros <jsd@semihalf•com>,
	linux-i2c@vger•kernel.org, Daniel Mack <daniel@zonque•org>,
	Haojian Zhuang <haojian.zhuang@gmail•com>,
	linux-spi@vger•kernel.org,
	Jonathan Derrick <jonathan.derrick@linux•dev>,
	linux-pci@vger•kernel.org, linux-gpio@vger•kernel.org,
	Mauro Carvalho Chehab <mchehab@kernel•org>,
	linux-media@vger•kernel.org, linux-mmc@vger•kernel.org
Subject: Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free
Date: Mon, 23 Feb 2026 19:38:33 +0200	[thread overview]
Message-ID: <aZyQmc7nOt87jitX@smile.fi.intel.com> (raw)
In-Reply-To: <cb878741-7b61-b72c-5a72-6ed6d5091b1f@rock-chips.com>

On Tue, Feb 24, 2026 at 12:09:37AM +0800, Shawn Lin wrote:
> 在 2026/02/23 星期一 23:50, Andy Shevchenko 写道:
> > On Mon, Feb 23, 2026 at 5:32 PM Shawn Lin <shawn.lin@rock-chips•com> wrote:
> > > 
> > > This patch series addresses 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.
> > > 
> > > ==== The Problem: Implicit vs. Explicit Management ====
> > > 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.
> > 
> > Is this confirmed? What I read from the cover letter, this series was
> > only compile-tested, so how can you prove the problem exists in the
> > first place?
> 
> Yes, it's confirmed. My debug of a double free issue of a out-of-tree
> PCIe wifi driver which uses
> pcim_enable_device + pci_alloc_irq_vectors + pci_free_irq_vectors expose
> it. And we did have a TODO to cleanup this hybrid usage, targeted in
> this cycle[1] suggested by Philipp:

Okay, fair enough. I think this bit was missing in the cover letter.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=msi

> > > ==== The Solution: Making Management Explicit ====
> > > This series enforces a clear, predictable model:
> > > 1.  New Managed API (Patch 1/37): Introduces pcim_alloc_irq_vectors() and
> > >      pcim_alloc_irq_vectors_affinity(). Drivers that desire devres-managed IRQ
> > >      vectors should use these functions, which set the is_msi_managed flag and
> > >      ensure automatic cleanup.
> > > 2.  Patches 2 through 36 convert each driver that uses pcim_enable_device() alongside
> > >      pci_alloc_irq_vectors() and relies on devres for IRQ vector cleanup to instead
> > >      make an explicit call to pcim_alloc_irq_vectors().
> > > 3.  Core Change (Patch 37/37): With the former cleanup, now modifies pcim_setup_msi_release()
> > >      to check only the is_msi_managed flag. This decouples automatic IRQ cleanup from
> > >      pcim_enable_device(). IRQ vectors allocated via pci_alloc_irq_vectors*()
> > >      are now solely the driver's responsibility to free with pci_free_irq_vectors().
> > > 
> > > With these changes, we clear ownership model: Explicit resource management eliminates
> > > ambiguity and follows the "principle of least surprise." New drivers choose one model and
> > > be consistent.
> > > - Use `pci_alloc_irq_vectors()` + `pci_free_irq_vectors()` for explicit control.
> > > - Use `pcim_alloc_irq_vectors()` for devres-managed, automatic cleanup.
> > 
> > Have you checked previous attempts? Why is your series better than those?
> 
> There seems not previous attempts.

Maybe we are looking to the different projects...

https://lore.kernel.org/all/?q=pcim_alloc_irq_vectors

> > > ==== Testing And Review ====
> > > 1. This series is only compiled test with allmodconfig.
> > > 2. Given the substantial size of this patch series, I have structured the mailing
> > >     to facilitate efficient review. The cover letter, the first patch and the last one will be sent
> > >     to all relevant mailing lists and key maintainers to ensure broad visibility and
> > >     initial feedback on the overall approach. The remaining subsystem-specific patches
> > >     will be sent only to the respective subsystem maintainers and their associated
> > >     mailing lists, reducing noise.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2026-02-23 17:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 15:29 [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free Shawn Lin
2026-02-23 15:29 ` [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation Shawn Lin
2026-02-24  0:04   ` Jakub Kicinski
2026-02-24  2:08     ` Shawn Lin
2026-02-24  7:47       ` Philipp Stanner
2026-02-24  8:21         ` Shawn Lin
2026-02-24  8:32           ` Philipp Stanner
2026-02-24 16:20   ` Jonathan Cameron
2026-02-23 15:29 ` [PATCH 37/37] PCI/MSI: Only check is_msi_managed in pcim_setup_msi_release() Shawn Lin
2026-02-23 15:50 ` [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free Andy Shevchenko
2026-02-23 16:09   ` Shawn Lin
2026-02-23 17:38     ` Andy Shevchenko [this message]
2026-02-24  2:29       ` Shawn Lin
2026-02-23 15:56 ` [PATCH 26/37] net: stmmac: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors() Shawn Lin
2026-02-23 15:56 ` [PATCH 27/37] r8169: " Shawn Lin
2026-02-23 15:57 ` [PATCH 28/37] net: thunder_bgx: " Shawn Lin
2026-02-23 15:57 ` [PATCH 29/37] net: hibmcge: " Shawn Lin
2026-02-24  4:14 ` [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free Simon Richter
2026-02-24  7:39   ` Philipp Stanner
2026-02-24  9:12     ` Andy Shevchenko
2026-02-24 10:30       ` Philipp Stanner
2026-02-24 10:39         ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZyQmc7nOt87jitX@smile.fi.intel.com \
    --to=andriy.shevchenko@linux$(echo .)intel.com \
    --cc=airlied@gmail$(echo .)com \
    --cc=alexandre.belloni@bootlin$(echo .)com \
    --cc=andi.shyti@kernel$(echo .)org \
    --cc=andreas.noever@gmail$(echo .)com \
    --cc=andrew+netdev@lunn$(echo .)ch \
    --cc=andy.shevchenko@gmail$(echo .)com \
    --cc=andy@kernel$(echo .)org \
    --cc=arnd@arndb$(echo .)de \
    --cc=atenart@kernel$(echo .)org \
    --cc=bbhushan2@marvell$(echo .)com \
    --cc=bentiss@kernel$(echo .)org \
    --cc=bhelgaas@google$(echo .)com \
    --cc=bingbu.cao@intel$(echo .)com \
    --cc=brgl@kernel$(echo .)org \
    --cc=broonie@kernel$(echo .)org \
    --cc=daniel@zonque$(echo .)org \
    --cc=dave@stgolabs$(echo .)net \
    --cc=davem@davemloft$(echo .)net \
    --cc=dmaengine@vger$(echo .)kernel.org \
    --cc=dri-devel@lists$(echo .)freedesktop.org \
    --cc=even.xu@intel$(echo .)com \
    --cc=gregkh@linuxfoundation$(echo .)org \
    --cc=hansg@kernel$(echo .)org \
    --cc=haojian.zhuang@gmail$(echo .)com \
    --cc=herbert@gondor$(echo .)apana.org.au \
    --cc=hkallweit1@gmail$(echo .)com \
    --cc=iommu@lists$(echo .)linux.dev \
    --cc=jeff.hugo@oss$(echo .)qualcomm.com \
    --cc=jikos@kernel$(echo .)org \
    --cc=jirislaby@kernel$(echo .)org \
    --cc=jonathan.cameron@huawei$(echo .)com \
    --cc=jonathan.derrick@linux$(echo .)dev \
    --cc=jsd@semihalf$(echo .)com \
    --cc=karol.wachowski@linux$(echo .)intel.com \
    --cc=kumaravel.thiagarajan@microchip$(echo .)com \
    --cc=kurt.schwemmer@microsemi$(echo .)com \
    --cc=lee@kernel$(echo .)org \
    --cc=linusw@kernel$(echo .)org \
    --cc=linux-arm-msm@vger$(echo .)kernel.org \
    --cc=linux-crypto@vger$(echo .)kernel.org \
    --cc=linux-cxl@vger$(echo .)kernel.org \
    --cc=linux-gpio@vger$(echo .)kernel.org \
    --cc=linux-i2c@vger$(echo .)kernel.org \
    --cc=linux-i3c@lists$(echo .)infradead.org \
    --cc=linux-input@vger$(echo .)kernel.org \
    --cc=linux-media@vger$(echo .)kernel.org \
    --cc=linux-mmc@vger$(echo .)kernel.org \
    --cc=linux-pci@vger$(echo .)kernel.org \
    --cc=linux-riscv@lists$(echo .)infradead.org \
    --cc=linux-serial@vger$(echo .)kernel.org \
    --cc=linux-spi@vger$(echo .)kernel.org \
    --cc=linux-usb@vger$(echo .)kernel.org \
    --cc=liulongfang@huawei$(echo .)com \
    --cc=lizhi.hou@amd$(echo .)com \
    --cc=logang@deltatee$(echo .)com \
    --cc=maciej.falkowski@linux$(echo .)intel.com \
    --cc=mamin506@gmail$(echo .)com \
    --cc=mani@kernel$(echo .)org \
    --cc=mchehab@kernel$(echo .)org \
    --cc=mhi@lists$(echo .)linux.dev \
    --cc=mika.westerberg@linux$(echo .)intel.com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=nic_swsd@realtek$(echo .)com \
    --cc=nirmal.patel@linux$(echo .)intel.com \
    --cc=ogabbay@kernel$(echo .)org \
    --cc=phasta@kernel$(echo .)org \
    --cc=platform-driver-x86@vger$(echo .)kernel.org \
    --cc=raag.jadav@intel$(echo .)com \
    --cc=rric@kernel$(echo .)org \
    --cc=sakari.ailus@linux$(echo .)intel.com \
    --cc=schalla@marvell$(echo .)com \
    --cc=sgoutham@marvell$(echo .)com \
    --cc=shaojijie@huawei$(echo .)com \
    --cc=shawn.lin@rock-chips$(echo .)com \
    --cc=shenjian15@huawei$(echo .)com \
    --cc=simona@ffwll$(echo .)ch \
    --cc=srinivas.pandruvada@linux$(echo .)intel.com \
    --cc=tiantao6@hisilicon$(echo .)com \
    --cc=tjeznach@rivosinc$(echo .)com \
    --cc=ulf.hansson@linaro$(echo .)org \
    --cc=vaibhaavram.tl@microchip$(echo .)com \
    --cc=vkoul@kernel$(echo .)org \
    --cc=wangzhou1@hisilicon$(echo .)com \
    --cc=westeri@kernel$(echo .)org \
    --cc=will@kernel$(echo .)org \
    --cc=xinliang.liu@linaro$(echo .)org \
    --cc=xinpeng.sun@intel$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox