Re: [PATCH v15 0/5] PCI/IOV: Fix SR-IOV locking races and AB-BA deadlocks

[Benjamin Block]

Hello Bjorn,

On Thu, Sep 24, 2026 at 06:29:26PM +0200, Benjamin Block wrote:
> This is the continuation and combination of two patchsets from earlier
> this year that try to fix races and various deadlocks involving the
> global mutex `pci_rescan_remove_owner`:
> https://lore.kernel.org/linux-pci/cover.1776839248.git.ionut.nechita%40windriver.com/ [v14]
> https://lore.kernel.org/linux-pci/cover.1776868550.git.bblock%40linux.ibm.com/ [v4]
> 
> Since we couldn't make any progress since then I decided to rebase and combine
> them, hoping this would "revitalize" the topic.
> 
> v14 -> v15:
>     * Rebased the patchset on v7.3-rc4
>     * Patch 1/5: changed type of `pci_rescan_remove_depth` to `size_t`
>     * Patch 1/5: changed accesses of `pci_rescan_remove_owner` to use
>       READ_ONCE() and/or WRITE_ONCE() since they might happen outside of
>       the lock, and so might be subject to tearing or some such
>     * Patch 1/5: added comments clarifying that access to
>       `pci_rescan_remove_depth` is always protected by
>       `pci_rescan_remove_lock`
>     * Patch 1/5: since the only Reviewed-by was by myself on the last iteration
>       I removed it
>     * Patch 5/5: fixed a bug with the use of scoped_guard() in `switch`
>       statements -> `break` doesn't work as it seems it should, because
>       `scoped_guard()` is implemented as `for` statement
>     * Patch 5/5: fixed a bug in recover_store() where it wouldn't reverse the
>       call to sysfs_break_active_protection() in case of an early error-return
>     * Patch 5/5: since I had to make these bug-fixes I removed Niklas'
>       Reviewed-by
> 
> Overview:
> 
> Patch 1/5: Fixes a regression introduced by
>            05703271c3cd ("PCI/IOV: Add PCI rescan-remove locking when enabling/disabling SR-IOV")
>            that moves the locking of `pci_rescan_remove_lock` to a higher
>            level in the call-chain when enabling/disabling SR-IOV, but by
>            doing so removed it entirely from the driver's .remove() call-back
>            call-chain.
>            To add the lock back into that call-chain and also prevent
>            resulting recursive deadlocks make the lock reentrant via the
>            existing functions
>            pci_lock_rescan_remove()/pci_unlock_rescan_remove().
> Patch 2/5: With the locking back in the .remove() call-back call-chain
>            it is now possible to trigger an AB-BA deadlock between device
>            driver unbinds and concurrent PCI device removes. Fix this by
>            unbinding the device driver before starting PCI device
>            removal (and locking `pci_rescan_remove_lock`).
> Patch 3/5: In order to use annotations like __must_hold() or assertions
>            like lockdep_assert_held() on `pci_rescan_remove_lock` in
>            code outside the PCI core (such as PCI architecture
>            implementations) move its declaration to the global linux/pci.h.
>            This doesn't increase the "surface" from where it is possible
>            to use the lock, since it is already possible today to grab
>            the lock from said code with the functions
>            pci_lock_rescan_remove()/pci_unlock_rescan_remove().
> Patch 4/5: Provide a lock guard for `pci_rescan_remove_lock` so it is
>            possible to be used in the linux/cleanup.h macros.
> Patch 5/5: Fix several AB-BA deadlocks in the s390 PCI implementation
>            involving `pci_rescan_remove_lock` by moving the lock to
>            higher levels in the respective call-chains.

is there any feedback from you for this? How can we make progress with this
problem?

Last I checked Sashiko had no more complaints about this patch-set, and I also
didn't get any other feedback - negative or positive.

Especially the bugs this series fixes in Patch 5 are a serious concern for us
on s390, and it's basically been stalled for over half a year now. I need some
form of feedback whether this approach has any chance of getting accepted, or
whether and why it has not; so that we can look for alternatives if necessary.

-- 
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.: Ehningen     /     Registergericht: AmtsG Stuttgart, HRB 243294