Re: [PATCH 2/2] pci: fix slot reset device locking

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 28 Jan 2026, Bjorn Helgaas wrote:

> [+cc Dan, Jinhui]
> 
> On Fri, Jan 16, 2026 at 10:41:50AM -0800, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > Like pci_bus_lock, pci_slot_lock needs to lock the bridge device to
> > prevent the warning:
> 
> I *think* this actually refers to pci_bus_trylock() and
> pci_slot_trylock() (not pci_bus_lock() and pci_slot_lock()), since
> that's what this patch changes?
> 
> It's unfortunate that pci_bus_trylock() and pci_slot_trylock() are so
> similar but separate.  If there were combined, this kind of issue
> where one is fixed but the other isn't wouldn't happen.
> 
> But what about pci_bus_lock() and pci_slot_lock()?  They are also
> almost identical, but pci_bus_lock() locks bus->self while
> pci_slot_lock() does not.  Should it?
>
> All these almost-but-not-quite identical paths make my head hurt ;)

My series does attempt to consolidation them (check the pending patches 
from me).

(It doesn't consider this fix patch though but the other one is taken 
into account, the series was build on top of it).

-- 
 i.

> >   pcieport 0000:e2:05.0: unlocked secondary bus reset via: pciehp_reset_slot+0x55/0xa0
> > 
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> >  drivers/pci/pci.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 3378221c5723a..5f8b0d06a1459 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -5460,6 +5460,8 @@ static void pci_slot_lock(struct pci_slot *slot)
> >  {
> >  	struct pci_dev *dev;
> >  
> > +	if (slot->bus->self)
> > +		pci_dev_lock(slot->bus->self);
> >  	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
> >  		if (!dev->slot || dev->slot != slot)
> >  			continue;
> > @@ -5483,12 +5485,17 @@ static void pci_slot_unlock(struct pci_slot *slot)
> >  		else
> >  			pci_dev_unlock(dev);
> >  	}
> > +	if (slot->bus->self)
> > +		pci_dev_unlock(slot->bus->self);
> >  }
> >  
> >  /* Return 1 on successful lock, 0 on contention */
> >  static int pci_slot_trylock(struct pci_slot *slot)
> >  {
> > -	struct pci_dev *dev;
> > +	struct pci_dev *dev, *bridge = slot->bus->self;
> > +
> > +	if (bridge && !pci_dev_trylock(bridge))
> > +		return 0;
> >  
> >  	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
> >  		if (!dev->slot || dev->slot != slot)
> > @@ -5511,6 +5518,9 @@ static int pci_slot_trylock(struct pci_slot *slot)
> >  		else
> >  			pci_dev_unlock(dev);
> >  	}
> > +
> > +	if (bridge)
> > +		pci_dev_unlock(bridge);
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.47.3
> > 
>