public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
From: Niklas Schnelle <schnelle@linux.ibm.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	"Ionut Nechita (Wind River)" <ionut.nechita@windriver.com>,
	Benjamin Block <bblock@linux.ibm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org,
	Ionut Nechita <ionut_n2001@yahoo.com>,
	Farhan Ali <alifm@linux.ibm.com>,
	Julian Ruess <julianr@linux.ibm.com>
Subject: Re: [PATCH] PCI/IOV: Fix recursive locking deadlock on pci_rescan_remove_lock
Date: Mon, 09 Feb 2026 11:12:36 +0100	[thread overview]
Message-ID: <2b6a844619892ecaa11031705808667e0886d8b2.camel@linux.ibm.com> (raw)
In-Reply-To: <20260209082553.1pnF4lr0@linutronix.de>

On Mon, 2026-02-09 at 09:25 +0100, Sebastian Andrzej Siewior wrote:
> On 2026-02-09 09:57:07 [+0200], Ionut Nechita (Wind River) wrote:
> > From: Ionut Nechita <ionut.nechita@windriver.com>
> > 
> > When a PCI device is hot-removed via sysfs (e.g., echo 1 > /sys/.../remove),
> > pci_stop_and_remove_bus_device_locked() acquires pci_rescan_remove_lock and
> > then recursively walks the bus hierarchy calling driver .remove() callbacks.
> > 
> > If the removed device is a PF with SR-IOV enabled (e.g., i40e, ice), the
> > driver's .remove() calls pci_disable_sriov() -> sriov_disable() ->
> > sriov_del_vfs() which also tries to acquire pci_rescan_remove_lock.
> > Since this is a non-recursive mutex and the same thread already holds it,
> > this results in a deadlock.
> > 
> > On PREEMPT_RT kernels, where mutexes are backed by rtmutex with deadlock
> > detection, this immediately triggers:
> > 
> >   WARNING: CPU: 15 PID: 11730 at kernel/locking/rtmutex.c:1663
> >   Call Trace:
> >    mutex_lock+0x47/0x60
> >    sriov_disable+0x2a/0x100
> >    i40e_free_vfs+0x415/0x470 [i40e]
> >    i40e_remove+0x38d/0x3e0 [i40e]
> >    pci_device_remove+0x3b/0xb0
> >    device_release_driver_internal+0x193/0x200
> >    pci_stop_bus_device+0x81/0xb0
> >    pci_stop_and_remove_bus_device_locked+0x16/0x30
> >    remove_store+0x79/0x90
> > 
> > On non-RT kernels the same recursive acquisition silently hangs the calling
> > process, eventually causing netdev watchdog TX timeout splats.
> > 
> > This affects all drivers that call pci_disable_sriov() from their .remove()
> > callback (i40e, ice, and others).
> > 
> > Fix this by tracking the owner of pci_rescan_remove_lock and skipping the
> > redundant acquisition in sriov_del_vfs() when the current thread already
> > holds it.  The VF removal is still serialized correctly because the caller
> > already holds the lock.
> 
> This looks like the result of commit 05703271c3cdc ("PCI/IOV: Add PCI
> rescan-remove locking when enabling/disabling SR-IOV").
> 
> > Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
> 
> Sebastian

Agree, this looks related to the deadlock I later found with that
commit and that lead to this revert+new fix that has now been queued
for the v6.20/v7.00 here:

https://lore.kernel.org/linux-pci/20251216-revert_sriov_lock-v3-0-dac4925a7621@linux.ibm.com/

That said I do find this approach interesting. Benjamin and I are
actually still looking into a related problem with not taking the
rescan/remove lock as part of vfio-pci tear down and there this
approach could work better than just moving the locking up into the
sysfs handler. So far we haven't found a good place to take the lock in
that path that doesn't suffer from the recursive locking in other
paths. On the other hand conditionally taking a mutex is always a
little ugly in my opinion.

Thanks,
Niklas

  reply	other threads:[~2026-02-09 10:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-09  7:57 [PATCH] PCI/IOV: Fix recursive locking deadlock on pci_rescan_remove_lock Ionut Nechita (Wind River)
2026-02-09  8:25 ` Sebastian Andrzej Siewior
2026-02-09 10:12   ` Niklas Schnelle [this message]
2026-02-11  7:37     ` Sebastian Andrzej Siewior
2026-02-09 16:14 ` Bjorn Helgaas
2026-02-22 11:29   ` Ionut Nechita (Wind River)

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=2b6a844619892ecaa11031705808667e0886d8b2.camel@linux.ibm.com \
    --to=schnelle@linux.ibm.com \
    --cc=alifm@linux.ibm.com \
    --cc=bblock@linux.ibm.com \
    --cc=bhelgaas@google.com \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=ionut.nechita@windriver.com \
    --cc=ionut_n2001@yahoo.com \
    --cc=julianr@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=rostedt@goodmis.org \
    /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