[PATCH 0/2] PCI/IOV: Fix deadlock when removing PF with enabled SR-IOV

[PATCH 0/2] PCI/IOV: Fix deadlock when removing PF with enabled SR-IOV

[Niklas Schnelle]

Hi Bjorn, Hi Lukas,

Doing additional testing for a distribution backport of commit
05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when
enabling/disabling SR-IOV") Benjamin found a hang with s390's
recover attribute. Further investigation showed this to be a deadlock by
recursively trying to take pci_rescan_remove lock when removing a PF
with enabled SR-IOV.

The issue can be reproduced on both s390 and x86_64 with:

    $ echo <NUM> > /sys/bus/pci/devices/<pf>/sriov_numvfs
    $ echo 1 > /sys/bus/pci/devices/<pf>/remove

As this seems worse than the original, hard to hit, race fixed by the
cited commit I think we first want to revert the broken fix.

Following that patch 2 attempts to fix the original issue by taking the
PCI rescan/remove lock directly before calling into the driver's
sriov_configure() callback establishing the rule that this should only
be called with the pci_rescan_remove_lock held.

Thanks,
Niklas

Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Niklas Schnelle (2):
      Revert "PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV"
      PCI/IOV: Fix race between SR-IOV enable/disable and hotplug

 drivers/pci/iov.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
---
base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
change-id: 20251029-revert_sriov_lock-aef4557f360f

Best regards,
-- 
Niklas Schnelle

[PATCH 1/2] Revert "PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV"

[Niklas Schnelle]

This reverts commit 05703271c3cd ("PCI/IOV: Add PCI rescan-remove
locking when enabling/disabling SR-IOV") which causes a deadlock by
recursively taking pci_rescan_remove_lock when sriov_del_vfs() is called
as part of pci_stop_and_remove_bus_device(). For example with the
following sequence of commands:

$ echo <NUM> > /sys/bus/pci/devices/<pf>/sriov_numvfs
$ echo 1 > /sys/bus/pci/devices/<pf>/remove

A trimmed trace of the deadlock on a mlx5 device is as below:

 mutex_lock_nested+0x3c/0x50
 sriov_disable+0x34/0x140
 mlx5_sriov_disable+0x50/0x80 [mlx5_core]
 remove_one+0x5e/0xf0 [mlx5_core]
 pci_device_remove+0x3c/0xa0
 device_release_driver_internal+0x18e/0x280
 pci_stop_bus_device+0x82/0xa0
 pci_stop_and_remove_bus_device_locked+0x5e/0x80
 remove_store+0x72/0x90

This is not a complete fix as it restores the issue the cited commit
tried to solve.

Cc: stable@vger.kernel.org
Fixes: 05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV")
Reported-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/pci/iov.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 77dee43b785838d215b58db2d22088e9346e0583..ac4375954c9479b5f4a0e666b5215094fdaeefc2 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -629,18 +629,15 @@ static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
 	if (dev->no_vf_scan)
 		return 0;
 
-	pci_lock_rescan_remove();
 	for (i = 0; i < num_vfs; i++) {
 		rc = pci_iov_add_virtfn(dev, i);
 		if (rc)
 			goto failed;
 	}
-	pci_unlock_rescan_remove();
 	return 0;
 failed:
 	while (i--)
 		pci_iov_remove_virtfn(dev, i);
-	pci_unlock_rescan_remove();
 
 	return rc;
 }
@@ -765,10 +762,8 @@ static void sriov_del_vfs(struct pci_dev *dev)
 	struct pci_sriov *iov = dev->sriov;
 	int i;
 
-	pci_lock_rescan_remove();
 	for (i = 0; i < iov->num_VFs; i++)
 		pci_iov_remove_virtfn(dev, i);
-	pci_unlock_rescan_remove();
 }
 
 static void sriov_disable(struct pci_dev *dev)

-- 
2.48.1

Re: [PATCH 1/2] Revert "PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV"

[Benjamin Block]

On Thu, Oct 30, 2025 at 11:26:01AM +0100, Niklas Schnelle wrote:
> This reverts commit 05703271c3cd ("PCI/IOV: Add PCI rescan-remove
> locking when enabling/disabling SR-IOV") which causes a deadlock by
> recursively taking pci_rescan_remove_lock when sriov_del_vfs() is called
> as part of pci_stop_and_remove_bus_device(). For example with the
> following sequence of commands:
> 
> $ echo <NUM> > /sys/bus/pci/devices/<pf>/sriov_numvfs
> $ echo 1 > /sys/bus/pci/devices/<pf>/remove
> 
> A trimmed trace of the deadlock on a mlx5 device is as below:
> 
>  mutex_lock_nested+0x3c/0x50
>  sriov_disable+0x34/0x140
>  mlx5_sriov_disable+0x50/0x80 [mlx5_core]
>  remove_one+0x5e/0xf0 [mlx5_core]
>  pci_device_remove+0x3c/0xa0
>  device_release_driver_internal+0x18e/0x280
>  pci_stop_bus_device+0x82/0xa0
>  pci_stop_and_remove_bus_device_locked+0x5e/0x80
>  remove_store+0x72/0x90
> 
> This is not a complete fix as it restores the issue the cited commit
> tried to solve.
> 

Reviewed-by: Benjamin Block <bblock@linux.ibm.com>

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Böblingen     /    Registergericht: AmtsG Stuttgart, HRB 243294

[PATCH 2/2] PCI/IOV: Fix race between SR-IOV enable/disable and hotplug

[Niklas Schnelle]

Commit 05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when
enabling/disabling SR-IOV") tried to fix a race between the VF removal
inside sriov_del_vfs() and concurrent hot unplug by taking the PCI
rescan/remove lock in sriov_del_vfs(). Similarly the PCI rescan/remove
lock was also taken in sriov_add_vfs() to protect addition of VFs.

This approach however causes deadlock on trying to remove PFs with
SR-IOV enabled because PFs disable SR-IOV during removal and this
removal happens under the PCI rescan/remove lock. So the original fix
had to be reverted.

Instead of taking the PCI rescan/remove lock in sriov_add_vfs() and
sriov_del_vfs(), fix the race that occurs with SR-IOV enable and disable
vs hotplug higher up in the callchain by taking the lock in
sriov_numvfs_store() before calling into the driver's sriov_configure()
callback.

Cc: stable@vger.kernel.org
Fixes: 05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV")
Reported-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/pci/iov.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index ac4375954c9479b5f4a0e666b5215094fdaeefc2..c6dc1b44bf602a0b1785b684f768fcd563f5b2aa 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -495,7 +495,9 @@ static ssize_t sriov_numvfs_store(struct device *dev,
 
 	if (num_vfs == 0) {
 		/* disable VFs */
+		pci_lock_rescan_remove();
 		ret = pdev->driver->sriov_configure(pdev, 0);
+		pci_unlock_rescan_remove();
 		goto exit;
 	}
 
@@ -507,7 +509,9 @@ static ssize_t sriov_numvfs_store(struct device *dev,
 		goto exit;
 	}
 
+	pci_lock_rescan_remove();
 	ret = pdev->driver->sriov_configure(pdev, num_vfs);
+	pci_unlock_rescan_remove();
 	if (ret < 0)
 		goto exit;
 

-- 
2.48.1

Re: [PATCH 2/2] PCI/IOV: Fix race between SR-IOV enable/disable and hotplug

[Benjamin Block]

Hey Niklas,

On Thu, Oct 30, 2025 at 11:26:02AM +0100, Niklas Schnelle wrote:
> Commit 05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when
> enabling/disabling SR-IOV") tried to fix a race between the VF removal
> inside sriov_del_vfs() and concurrent hot unplug by taking the PCI
> rescan/remove lock in sriov_del_vfs(). Similarly the PCI rescan/remove
> lock was also taken in sriov_add_vfs() to protect addition of VFs.
> 
> This approach however causes deadlock on trying to remove PFs with
> SR-IOV enabled because PFs disable SR-IOV during removal and this
> removal happens under the PCI rescan/remove lock. So the original fix
> had to be reverted.
> 
> Instead of taking the PCI rescan/remove lock in sriov_add_vfs() and
> sriov_del_vfs(), fix the race that occurs with SR-IOV enable and disable
> vs hotplug higher up in the callchain by taking the lock in
> sriov_numvfs_store() before calling into the driver's sriov_configure()
> callback.
> 
> Cc: stable@vger.kernel.org
> Fixes: 05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV")
> Reported-by: Benjamin Block <bblock@linux.ibm.com>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/pci/iov.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 

It fixes the original bug (kernel panic) AFAICT, insofar:

Reviewed-by: Benjamin Block <bblock@linux.ibm.com>


But tbh., I find it a bit "scary" to take one global lock at such a high
level, when it is also used at much lower levels throughout the stack.

IIRC my original (internal) report boiled down to that pci_destroy_dev() is
called essentially unlocked, and without taking any of the attached reference
counts into account.. which seems fishy IMHO. The actual freeing of the memory
is hidden behind a reference, but stuff like device_del() is just.. done..
and that seems strange to me.

Having a bit more granularity here might not hurt; but I can't say I have a
good idea on top of my mind in how to do that, so... rather fix the panic.

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Böblingen     /    Registergericht: AmtsG Stuttgart, HRB 243294