[PATCH 2/4] s390/pci: Fix circular/recursive deadlocks in PCI-bus and -device release
From: Benjamin Block
Date: Fri Mar 06 2026 - 11:57:28 EST
When removing PCI device or PCI bus objects there are a couple of
call-chains where it is possible that the kernel runs into a circular or
recursive deadlock involving taking the central `pci_rescan_remove_lock`.
For example:
(A) Thread α receives a CRW event notifying the kernel that a PCI
virtual function has been moved into Reserved state, and so the PCI
subsystem will try to remove that PCI function. The call-chain for that
looks like this:
__zpci_event_availability()
-> zpci_zdev_put() # will lock(zpci_add_remove_lock),
# and lock(zpci_list_lock)
-> zpci_release_device() # will unlock(zpci_list_lock)
-> zpci_cleanup_bus_resources() # will lock(pci_rescan_remove_lock)
Thread β is triggered by userspace writing 0 into the SysFS attribute
`sriov_numvfs` of the parent PCI physical function of the same function
we just try to remove. This will also try to release the PCI virtual
function; but this time the call-chain looks like this:
sriov_numvfs_store() # will lock(pci_rescan_remove_lock)
-> ... (deep chain)
-> pci_release_dev()
-> pcibios_release_device()
-> zpci_zdev_put() # will lock(zpci_add_remove_lock)
If thread α and β coincide, this will result in a cyclic deadlock.
(B) Consider thread β from above; if the thread was to follow the same
outlined call-chain for thread α, and not fall into the cyclic deadlock,
it would recursive deadlock:
sriov_numvfs_store() # will lock(pci_rescan_remove_lock)
-> ... (deep chain)
-> pci_release_dev()
-> pcibios_release_device()
-> zpci_zdev_put() # will lock(zpci_add_remove_lock),
# and lock(zpci_list_lock)
-> zpci_release_device() # will unlock(zpci_list_lock)
-> zpci_cleanup_bus_resources() # will lock(pci_rescan_remove_lock)
(C) And even in case `pci_rescan_remove_lock` was removed from
zpci_cleanup_bus_resources(), the resulting call-chain would recursive
deadlock when it tries to release the associated PCI bus:
sriov_numvfs_store() # will lock(pci_rescan_remove_lock)
-> ... (deep chain)
-> pci_release_dev()
-> pcibios_release_device()
-> zpci_zdev_put() # will lock(zpci_add_remove_lock),
# and lock(zpci_list_lock)
-> zpci_release_device() # will unlock(zpci_list_lock)
-> zpci_bus_device_unregister()
-> zpci_bus_put() # will lock(zbus_list_lock)
-> zpci_bus_release() # will unlock(zbus_list_lock),
# will lock(pci_rescan_remove_lock)
It can also easily be seen that scenario (C) offers the same risk as (A)
for a cyclic deadlock in cases where `pci_rescan_remove_lock` is first
locked, and the PCI bus released under its protection.
`pci_rescan_remove_lock` has to be and is taken at a "high level" in
most call-chains since it is intended to protect/mutual exclude all
rescan and/or removal actions taken in the PCI subsystem. So to prevent
the outlined deadlock scenarios above remove it instead from the "low
level" release function for both the PCI device and PCI bus objects.
Instead, lock `pci_rescan_remove_lock` in all call-chains leading to
those release functions:
* initialization of the PCI subsystem;
* processing of availability events (CRWs) for PCI functions;
* processing of error events (CRWs) for PCI functions;
* architecture specific release PCI device implementation.
Additionally, remove `pci_rescan_remove_lock` from zpci_bus_scan_bus()
since it is now always already taken when called.
Lastly, document the new locking expectations after these changes. Add
sparse and lockdep annotations to functions that previously locked
`pci_rescan_remove_lock` explicitly, making sure the lock is now
already held when called. Additionally also add the annotations to
zpci_zdev_put() and zpci_bus_put() to make sure that every function that
potentially drops the last reference already holds the lock to prevent
surprises.
Signed-off-by: Benjamin Block <bblock@xxxxxxxxxxxxx>
---
arch/s390/pci/pci.c | 11 +++++++++--
arch/s390/pci/pci_bus.c | 12 +++++++-----
arch/s390/pci/pci_event.c | 21 +++++++++++++++++++--
3 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 2a430722cbe4..fd16e6ad21c1 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -71,9 +71,11 @@ struct airq_iv *zpci_aif_sbv;
EXPORT_SYMBOL_GPL(zpci_aif_sbv);
void zpci_zdev_put(struct zpci_dev *zdev)
+ __must_hold(&pci_rescan_remove_lock)
{
if (!zdev)
return;
+ lockdep_assert_held(&pci_rescan_remove_lock);
mutex_lock(&zpci_add_remove_lock);
kref_put_lock(&zdev->kref, zpci_release_device, &zpci_list_lock);
mutex_unlock(&zpci_add_remove_lock);
@@ -582,11 +584,13 @@ int zpci_setup_bus_resources(struct zpci_dev *zdev)
}
static void zpci_cleanup_bus_resources(struct zpci_dev *zdev)
+ __must_hold(&pci_rescan_remove_lock)
{
struct resource *res;
int i;
- pci_lock_rescan_remove();
+ lockdep_assert_held(&pci_rescan_remove_lock);
+
for (i = 0; i < PCI_STD_NUM_BARS; i++) {
res = zdev->bars[i].res;
if (!res)
@@ -599,7 +603,6 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev)
kfree(res);
}
zdev->has_resources = 0;
- pci_unlock_rescan_remove();
}
int pcibios_device_add(struct pci_dev *pdev)
@@ -629,8 +632,10 @@ void pcibios_release_device(struct pci_dev *pdev)
{
struct zpci_dev *zdev = to_zpci(pdev);
+ pci_lock_rescan_remove();
zpci_unmap_resources(pdev);
zpci_zdev_put(zdev);
+ pci_unlock_rescan_remove();
}
int pcibios_enable_device(struct pci_dev *pdev, int mask)
@@ -1208,7 +1213,9 @@ static int __init pci_base_init(void)
if (rc)
goto out_irq;
+ pci_lock_rescan_remove();
rc = zpci_scan_devices();
+ pci_unlock_rescan_remove();
if (rc)
goto out_find;
diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c
index 36a4807285fa..2b598222c621 100644
--- a/arch/s390/pci/pci_bus.c
+++ b/arch/s390/pci/pci_bus.c
@@ -132,10 +132,13 @@ void zpci_bus_remove_device(struct zpci_dev *zdev, bool set_error)
* Return: 0 on success, an error value otherwise
*/
int zpci_bus_scan_bus(struct zpci_bus *zbus)
+ __must_hold(&pci_rescan_remove_lock)
{
struct zpci_dev *zdev;
int devfn, rc, ret = 0;
+ lockdep_assert_held(&pci_rescan_remove_lock);
+
for (devfn = 0; devfn < ZPCI_FUNCTIONS_PER_BUS; devfn++) {
zdev = zbus->function[devfn];
if (zdev && zdev->state == ZPCI_FN_STATE_CONFIGURED) {
@@ -145,10 +148,8 @@ int zpci_bus_scan_bus(struct zpci_bus *zbus)
}
}
- pci_lock_rescan_remove();
pci_scan_child_bus(zbus->bus);
pci_bus_add_devices(zbus->bus);
- pci_unlock_rescan_remove();
return ret;
}
@@ -214,11 +215,12 @@ static int zpci_bus_create_pci_bus(struct zpci_bus *zbus, struct zpci_dev *fr, s
* run of the function.
*/
static inline void zpci_bus_release(struct kref *kref)
- __releases(&zbus_list_lock)
+ __releases(&zbus_list_lock) __must_hold(&pci_rescan_remove_lock)
{
struct zpci_bus *zbus = container_of(kref, struct zpci_bus, kref);
lockdep_assert_held(&zbus_list_lock);
+ lockdep_assert_held(&pci_rescan_remove_lock);
list_del(&zbus->bus_next);
mutex_unlock(&zbus_list_lock);
@@ -229,14 +231,12 @@ static inline void zpci_bus_release(struct kref *kref)
*/
if (zbus->bus) {
- pci_lock_rescan_remove();
pci_stop_root_bus(zbus->bus);
zpci_free_domain(zbus->domain_nr);
pci_free_resource_list(&zbus->resources);
pci_remove_root_bus(zbus->bus);
- pci_unlock_rescan_remove();
}
zpci_remove_parent_msi_domain(zbus);
@@ -250,7 +250,9 @@ static inline void __zpci_bus_get(struct zpci_bus *zbus)
}
static inline void zpci_bus_put(struct zpci_bus *zbus)
+ __must_hold(&pci_rescan_remove_lock)
{
+ lockdep_assert_held(&pci_rescan_remove_lock);
kref_put_mutex(&zbus->kref, zpci_bus_release, &zbus_list_lock);
}
diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 839bd91c056e..edfaeed737ac 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -342,7 +342,9 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
no_pdev:
if (zdev)
mutex_unlock(&zdev->state_lock);
+ pci_lock_rescan_remove();
zpci_zdev_put(zdev);
+ pci_unlock_rescan_remove();
}
void zpci_event_error(void *data)
@@ -387,6 +389,7 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
bool existing_zdev = !!zdev;
enum zpci_state state;
+ int rc;
zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
ccdf->fid, ccdf->fh, ccdf->pec);
@@ -400,7 +403,10 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
if (IS_ERR(zdev))
break;
- if (zpci_add_device(zdev)) {
+ pci_lock_rescan_remove();
+ rc = zpci_add_device(zdev);
+ pci_unlock_rescan_remove();
+ if (rc) {
kfree(zdev);
break;
}
@@ -419,7 +425,10 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
if (IS_ERR(zdev))
break;
- if (zpci_add_device(zdev)) {
+ pci_lock_rescan_remove();
+ rc = zpci_add_device(zdev);
+ pci_unlock_rescan_remove();
+ if (rc) {
kfree(zdev);
break;
}
@@ -450,25 +459,33 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
/* The 0x0304 event may immediately reserve the device */
if (!clp_get_state(zdev->fid, &state) &&
state == ZPCI_FN_STATE_RESERVED) {
+ pci_lock_rescan_remove();
zpci_device_reserved(zdev);
+ pci_unlock_rescan_remove();
}
}
break;
case 0x0306: /* 0x308 or 0x302 for multiple devices */
+ pci_lock_rescan_remove();
zpci_remove_reserved_devices();
zpci_scan_devices();
+ pci_unlock_rescan_remove();
break;
case 0x0308: /* Standby -> Reserved */
if (!zdev)
break;
+ pci_lock_rescan_remove();
zpci_device_reserved(zdev);
+ pci_unlock_rescan_remove();
break;
default:
break;
}
if (existing_zdev) {
mutex_unlock(&zdev->state_lock);
+ pci_lock_rescan_remove();
zpci_zdev_put(zdev);
+ pci_unlock_rescan_remove();
}
}
--
2.53.0