* [PATCH v2] PCI/IOV: Add reentrant locking in sriov_add_vfs/sriov_del_vfs for complete serialization
2026-02-19 21:26 [PATCH v2 0/1] PCI/IOV: Add reentrant locking in sriov_add_vfs/sriov_del_vfs Ionut Nechita (Wind River)
@ 2026-02-19 21:26 ` Ionut Nechita (Wind River)
2026-02-25 18:43 ` Dragos Tatulea
0 siblings, 1 reply; 3+ messages in thread
From: Ionut Nechita (Wind River) @ 2026-02-19 21:26 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Sebastian Ott, Niklas Schnelle, Benjamin Block,
Farhan Ali, Julian Ruess, Ionut Nechita, linux-pci, linux-kernel,
stable, Ionut Nechita, Ionut Nechita
From: Ionut Nechita <ionut.nechita@windriver.com>
After reverting commit 05703271c3cd ("PCI/IOV: Add PCI rescan-remove
locking when enabling/disabling SR-IOV") and moving the lock to
sriov_numvfs_store(), the path through driver .remove() (e.g. rmmod,
or manual unbind) that calls pci_disable_sriov() directly remains
unprotected against concurrent hotplug events. This affects any SR-IOV
capable driver that calls pci_disable_sriov() from its .remove()
callback (i40e, ice, mlx5, bnxt, etc.).
On s390, platform-generated hot-unplug events for VFs can race with
sriov_del_vfs() when a PF driver is being unloaded. The platform event
handler takes pci_rescan_remove_lock, but sriov_del_vfs() does not,
leading to double removal and list corruption.
We cannot use a plain mutex_lock() here because sriov_del_vfs() may also
be called from paths that already hold pci_rescan_remove_lock (e.g.
remove_store -> pci_stop_and_remove_bus_device_locked, or
sriov_numvfs_store with the lock taken by the previous patch). Using
mutex_lock() in those cases would deadlock.
Instead, introduce owner tracking for pci_rescan_remove_lock via a new
pci_lock_rescan_remove_reentrant() helper. This function checks if the
current task already holds the lock:
- If the lock is not held: acquires it and returns true, providing
full serialization against concurrent hotplug events (including
platform-generated events on s390).
- If the lock is already held by the current task (reentrant call from
remove_store or sriov_numvfs_store paths): returns false without
re-acquiring, avoiding deadlock while the caller already provides
the necessary serialization.
- If the lock is held by another task (concurrent hotplug): blocks
until the lock is released, then acquires it, providing complete
serialization. This is the key improvement over a trylock approach.
A matching pci_unlock_rescan_remove_reentrant() helper takes the return
value of the lock function as argument, so callers don't need to
open-code the conditional unlock.
The "reentrant" naming is chosen to avoid confusion with existing
mutex_lock_nested() which is a lockdep annotation concept, not actual
reentrant locking.
Note: owner-tracking patterns for reentrant lock behavior exist elsewhere
in the kernel, for example in the regulator core (drivers/regulator/core.c)
with rdev->mutex_owner, and in the PPP subsystem (drivers/net/ppp/
ppp_generic.c) with xmit_recursion->owner.
The declarations are placed in include/linux/pci.h alongside the existing
pci_lock_rescan_remove()/pci_unlock_rescan_remove() declarations to
maintain API consistency and allow use by external drivers if needed.
Fixes: 18f9e9d150fc ("PCI/IOV: Factor out sriov_add_vfs()")
Cc: stable@vger.kernel.org
Signed-off-by: Ionut Nechita <ionut_n2001@yahoo.com>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
Changes in v2:
- Renamed from pci_lock_rescan_remove_nested() to
pci_lock_rescan_remove_reentrant() to avoid confusion with
mutex_lock_nested() lockdep annotations (Benjamin Block)
- Added pci_unlock_rescan_remove_reentrant(const bool locked) helper
to avoid open-coding conditional unlock at each call site
(Benjamin Block)
- Moved declarations from drivers/pci/pci.h to include/linux/pci.h
alongside existing lock/unlock declarations (Benjamin Block)
- Simplified callers: removed negation of return value and manual
conditional unlock in favor of the paired lock/unlock helpers
drivers/pci/iov.c | 7 +++++++
drivers/pci/probe.c | 19 +++++++++++++++++++
include/linux/pci.h | 2 ++
3 files changed, 28 insertions(+)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 4a659c34935e1..9a2784863be7f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -629,19 +629,23 @@ static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
{
unsigned int i;
int rc;
+ bool locked;
if (dev->no_vf_scan)
return 0;
+ locked = pci_lock_rescan_remove_reentrant();
for (i = 0; i < num_vfs; i++) {
rc = pci_iov_add_virtfn(dev, i);
if (rc)
goto failed;
}
+ pci_unlock_rescan_remove_reentrant(locked);
return 0;
failed:
while (i--)
pci_iov_remove_virtfn(dev, i);
+ pci_unlock_rescan_remove_reentrant(locked);
return rc;
}
@@ -764,10 +768,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
static void sriov_del_vfs(struct pci_dev *dev)
{
struct pci_sriov *iov = dev->sriov;
+ bool locked;
int i;
+ locked = pci_lock_rescan_remove_reentrant();
for (i = 0; i < iov->num_VFs; i++)
pci_iov_remove_virtfn(dev, i);
+ pci_unlock_rescan_remove_reentrant(locked);
}
static void sriov_disable(struct pci_dev *dev)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 41183aed8f5d9..f4cdcad7729bb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3540,19 +3540,38 @@ EXPORT_SYMBOL_GPL(pci_rescan_bus);
* routines should always be executed under this mutex.
*/
DEFINE_MUTEX(pci_rescan_remove_lock);
+static struct task_struct *pci_rescan_remove_owner;
void pci_lock_rescan_remove(void)
{
mutex_lock(&pci_rescan_remove_lock);
+ pci_rescan_remove_owner = current;
}
EXPORT_SYMBOL_GPL(pci_lock_rescan_remove);
void pci_unlock_rescan_remove(void)
{
+ pci_rescan_remove_owner = NULL;
mutex_unlock(&pci_rescan_remove_lock);
}
EXPORT_SYMBOL_GPL(pci_unlock_rescan_remove);
+bool pci_lock_rescan_remove_reentrant(void)
+{
+ if (pci_rescan_remove_owner == current)
+ return false;
+ pci_lock_rescan_remove();
+ return true;
+}
+EXPORT_SYMBOL_GPL(pci_lock_rescan_remove_reentrant);
+
+void pci_unlock_rescan_remove_reentrant(const bool locked)
+{
+ if (locked)
+ pci_unlock_rescan_remove();
+}
+EXPORT_SYMBOL_GPL(pci_unlock_rescan_remove_reentrant);
+
static int __init pci_sort_bf_cmp(const struct device *d_a,
const struct device *d_b)
{
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b5cc0c2b99065..0fb1d7709dba2 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1512,6 +1512,8 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev);
unsigned int pci_rescan_bus(struct pci_bus *bus);
void pci_lock_rescan_remove(void);
void pci_unlock_rescan_remove(void);
+bool pci_lock_rescan_remove_reentrant(void);
+void pci_unlock_rescan_remove_reentrant(const bool locked);
/* Vital Product Data routines */
ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread