From 4cb92fa763823d813d22b45b7f18fcf6e85a72ad Mon Sep 17 00:00:00 2001 From: Benjamin Block Date: Fri, 5 Dec 2025 16:47:17 +0100 Subject: s390/pci: Fix cyclic dead-lock in zpci_zdev_put() and zpci_scan_devices() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When triggering PCI device recovery by writing into the SysFS attribute `recover` of a Physical Function with existing child SR-IOV Virtual Functions, lockdep is reporting a possible deadlock between three threads: Thread (A) Thread (B) Thread (C) | | | recover_store() zpci_scan_devices() zpci_scan_devices() lock(pci_rescan_remove_lock) | | | | | | | zpci_bus_scan_busses() | | lock(zbus_list_lock) | zpci_add_device() | | lock(zpci_add_remove_lock) | | | ┴ | | zpci_bus_scan_bus() | | lock(pci_rescan_remove_lock) ┴ | zpci_zdev_put() | lock(zpci_add_remove_lock) | ┴ zpci_bus_get() lock(zbus_list_lock) In zpci_bus_scan_busses() the `zbus_list_lock` is taken for the whole duration of the function, which also includes taking `pci_rescan_remove_lock`, among other things. But `zbus_list_lock` only really needs to protect the modification of the global registration `zbus_list`, it can be dropped while the functions within the list iteration run; this way we break the cycle above. Break up zpci_bus_scan_busses() into an "iterator" zpci_bus_get_next() that iterates over `zbus_list` element by element, and acquires and releases `zbus_list_lock` as necessary, but never keep holding it. References to `zpci_bus` objects are also acquired and released. The reference counting on `zpci_bus` objects is also changed so that all put() and get() operations are done under the protection of `zbus_list_lock`, and if the operation results in a modification of `zpci_bus_list`, this modification is done in the same critical section (apart the very first initialization). This way objects are never seen on the list that are about to be released and/or half-initialized. Fixes: 14c87ba8123a ("s390/pci: separate zbus registration from scanning") Suggested-by: Niklas Schnelle Signed-off-by: Benjamin Block Reviewed-by: Niklas Schnelle Reviewed-by: Gerd Bayer Signed-off-by: Heiko Carstens --- .clang-format | 1 + 1 file changed, 1 insertion(+) (limited to '.clang-format') diff --git a/.clang-format b/.clang-format index 2ceca764122f..c7060124a47a 100644 --- a/.clang-format +++ b/.clang-format @@ -748,6 +748,7 @@ ForEachMacros: - 'ynl_attr_for_each_nested' - 'ynl_attr_for_each_payload' - 'zorro_for_each_dev' + - 'zpci_bus_for_each' IncludeBlocks: Preserve IncludeCategories: -- cgit v1.2.3