Re: [PATCH 2/2] PCI: Add lockdep assertion in pci_stop_and_remove_bus_device()

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Wed, 2025-09-24 at 11:06 -0700, Farhan Ali wrote:
> On 8/26/2025 1:52 AM, Niklas Schnelle wrote:
> > Removing a PCI devices requires holding pci_rescan_remove_lock. Prompted
> > by this being missed in sriov_disable() and going unnoticed since its
> > inception add a lockdep assert so this doesn't get missed again in the
> > future.
> > 
> > Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> >   drivers/pci/pci.h    | 2 ++
> >   drivers/pci/probe.c  | 2 +-
> >   drivers/pci/remove.c | 1 +
> >   3 files changed, 4 insertions(+), 1 deletion(-)
> > 
--- snip ---
> > diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> > index 445afdfa6498edc88f1ef89df279af1419025495..0b9a609392cecba36a818bc496a0af64061c259a 100644
> > --- a/drivers/pci/remove.c
> > +++ b/drivers/pci/remove.c
> > @@ -138,6 +138,7 @@ static void pci_remove_bus_device(struct pci_dev *dev)
> >    */
> >   void pci_stop_and_remove_bus_device(struct pci_dev *dev)
> >   {
> > +	lockdep_assert_held(&pci_rescan_remove_lock);
> >   	pci_stop_bus_device(dev);
> >   	pci_remove_bus_device(dev);
> >   }
> 
> We also have the function pci_stop_and_remove_bus_device_locked() as 
> Gerd mentioned, so is pci_stop_and_remove_bus_device meant to be called 
> without the rescan_remove_lock held? This is a little confusing as we 
> shouldn't be adding/removing from the bus without the lock AFAIU, but 
> maybe I am missing something?
> 
> Thanks
> Farhan

As far as I understand one would use pci_stop_and_remove_bus_device()
if one is already holding the pci_rescan_remove_lock and
pci_stop_and_remove_bus_device_locked() if one only needs to take it
for that call. I think this is kind of easy to get confused about so
this lockdep assertion is even more useful.