mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Block <bblock@linux.ibm.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	linux-intel-xe <intel-xe@lists.freedesktop.org>,
	piotr.piorkowski@intel.com, Farhan Ali <alifm@linux.ibm.com>,
	Halil Pasic <pasic@linux.ibm.com>,
	Gerd Bayer <gbayer@linux.ibm.com>, Lukas Wunner <lukas@wunner.de>,
	Guenter Roeck <linux@roeck-us.net>,
	Manivannan Sadhasivam <mani@kernel.org>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Alexander Gordeev <agordeev@linux.ibm.com>,
	Ionut Nechita <ionut_n2001@yahoo.com>,
	Tobias Schumacher <ts@linux.ibm.com>,
	Niklas Schnelle <schnelle@linux.ibm.com>,
	Ramesh Errabolu <ramesh@linux.ibm.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Sven Schnelle <svens@linux.ibm.com>,
	Keith Busch <kbusch@kernel.org>,
	Andreas Krebbel <krebbel@linux.ibm.com>,
	Julian Ruess <julianr@linux.ibm.com>,
	Matthew Brost <matthew.brost@intel.com>,
	Ionut Nechita <ionut.nechita@windriver.com>,
	Omar Elghoul <oelghoul@linux.ibm.com>,
	Michal Wajdeczko <michal.wajdeczko@intel.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	Ionut Nechita <sunlightlinux@gmail.com>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	linux-s390 <linux-s390@vger.kernel.org>,
	Dragos Tatulea <dtatulea@nvidia.com>,
	Benjamin Block <bebl@ategam.org>
Subject: Re: [PATCH v15 0/5] PCI/IOV: Fix SR-IOV locking races and AB-BA deadlocks
Date: Wed, 4 Nov 2026 12:16:31 +0100	[thread overview]
Message-ID: <20261104111631.GA19494@p14sgen6-pf6akexs> (raw)
In-Reply-To: <cover.1790267348.git.bblock@linux.ibm.com>

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

      parent reply	other threads:[~2026-11-04 11:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24 16:29 Benjamin Block
2026-09-24 16:29 ` [PATCH v15 1/5] PCI/IOV: Make pci_lock_rescan_remove() reentrant and protect sriov_add_vfs/sriov_del_vfs Benjamin Block
2026-09-24 16:29 ` [PATCH v15 2/5] PCI: Fix AB-BA deadlock between device_lock and pci_rescan_remove_lock in remove_store Benjamin Block
2026-09-24 16:29 ` [PATCH v15 3/5] PCI: Move declaration of pci_rescan_remove_lock into public pci.h Benjamin Block
2026-09-24 16:29 ` [PATCH v15 4/5] PCI: Provide lock guard for pci_rescan_remove_lock Benjamin Block
2026-09-24 16:29 ` [PATCH v15 5/5] s390/pci: Fix circular/recursive deadlocks in PCI-bus and -device release Benjamin Block
2026-10-02 12:55   ` [PATCH v16 " Benjamin Block
2026-11-04 11:16 ` Benjamin Block [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261104111631.GA19494@p14sgen6-pf6akexs \
    --to=bblock@linux.ibm.com \
    --cc=agordeev@linux.ibm.com \
    --cc=alifm@linux.ibm.com \
    --cc=bebl@ategam.org \
    --cc=bhelgaas@google.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=dtatulea@nvidia.com \
    --cc=gbayer@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=ionut.nechita@windriver.com \
    --cc=ionut_n2001@yahoo.com \
    --cc=julianr@linux.ibm.com \
    --cc=kbusch@kernel.org \
    --cc=krebbel@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lukas@wunner.de \
    --cc=mani@kernel.org \
    --cc=matthew.brost@intel.com \
    --cc=michal.wajdeczko@intel.com \
    --cc=mjrosato@linux.ibm.com \
    --cc=oelghoul@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    --cc=piotr.piorkowski@intel.com \
    --cc=ramesh@linux.ibm.com \
    --cc=schnelle@linux.ibm.com \
    --cc=sunlightlinux@gmail.com \
    --cc=svens@linux.ibm.com \
    --cc=ts@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®