RE: [RFC PATCH] PCI: fix concurrent pci_bus_add_device() execution during SR-IOV VF creation

[Shashank Gupta <shashankg@marvell.com>]

<bbhushan2@marvell.com>, Sunil Kovvuri Goutham <sgoutham@marvell.com> 
Bcc: 
Subject: Re: [RFC PATCH] PCI: fix concurrent pci_bus_add_device() execution
 during SR-IOV VF creation
Reply-To: 
In-Reply-To: <SN7PR18MB53145C30677F872D61EE49AEE3FCA@SN7PR18MB5314.namprd18.prod.outlook.com>

> > -----Original Message-----
> > From: Shashank Gupta <shashankg@marvell.com>
> > Sent: Thursday, September 11, 2025 4:41 PM
> > To: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Cc: Shashank Gupta <shashankg@marvell.com>; Bharat Bhushan
> > <bbhushan2@marvell.com>; Sunil Kovvuri Goutham
> > <sgoutham@marvell.com>
> > Subject: [RFC PATCH] PCI: fix concurrent pci_bus_add_device() execution
> > during SR-IOV VF creation
> > 
> > When enabling SR-IOV VFs via sriov_numvfs, a race can occur between VF
> > creation (`pci_iov_add_virtfn()`) and a parallel PCI bus rescan.
> > This may cause duplicate sysfs resource files to be created for the same VF.
> > `pci_device_add()` links the VF into the bus list before calling
> > `pci_bus_add_device()`. During this window, a parallel pci rescan may iterate
> > over the same VF and call `pci_bus_add_device()` before the PCI_DEV_ADDED
> > flag is set by sriov_numvfs, leading to duplicate sysfs entries.
> > 
> > sysfs: cannot create duplicate filename
> > '/devices/platform/0.soc/878020000000.pci/pci0002:00/0002:00:02.0/000
> > 2:03:00.3/resource2'
> > CPU: 10 PID: 1787 Comm: tee Tainted: G        W
> > 6.1.67-00053-g785627de1dee #150
> > Hardware name: Marvell CN106XX board (DT) Call trace:
> >  dump_backtrace.part.0+0xe0/0xf0
> >  show_stack+0x18/0x30
> >  dump_stack_lvl+0x68/0x84
> >  dump_stack+0x18/0x34
> >  sysfs_warn_dup+0x64/0x80
> >  sysfs_add_bin_file_mode_ns+0xd4/0x100
> >  sysfs_create_bin_file+0x74/0xa4
> >  pci_create_attr+0xf0/0x190
> >  pci_create_resource_files+0x48/0xc0
> >  pci_create_sysfs_dev_files+0x1c/0x30
> >  pci_bus_add_device+0x30/0xc0
> >  pci_bus_add_devices+0x38/0x84
> >  pci_bus_add_devices+0x64/0x84
> >  pci_rescan_bus+0x30/0x44
> >  rescan_store+0x7c/0xa0
> >  bus_attr_store+0x28/0x3c
> >  sysfs_kf_write+0x44/0x54
> >  kernfs_fop_write_iter+0x118/0x1b0
> >  vfs_write+0x20c/0x294
> >  ksys_write+0x6c/0x100
> >  __arm64_sys_write+0x1c/0x30
> > 
> > To prevent this, introduce a new in-progress private flag
> > (PCI_DEV_ADD_INPROG) in struct pci_dev and use it as an atomic guard
> > around pci_bus_add_device(). This ensures only one thread can progress with
> > device addition at a time.
> > 
> > The flag is cleared once the device has been added or the attempt has finished,
> > avoiding duplicate sysfs entries.
> 
> Please provide feedback to this RFC patch. 
> 
> Thanks
> -Bharat
>
Hi,

Please take a look to this RFC patch,

Thanks
-Shashank
> > 
> > Signed-off-by: Shashank Gupta <shashankg@marvell.com>
> > ---
> > 
> > Issue trace:
> > ------------
> > 
> > CPU2 (sriov_numvfs)                          CPU4 (pci rescan)
> > ------------------------------------------  --------------------------
> > pci_iov_add_virtfn()
> >   pci_device_add(virtfn)   # VF linked to bus
> >                                           pci_bus_add_devices()
> >                                             iterates over VF
> > 						PCI_DEV_ADDED not set yet
> > 
> > 
> >   pci_bus_add_device()
> > 	create sysfs file
> >         pci_dev_assign_added() set PCI_DEV_ADDED
> > 						pci_bus_add_device()
> > 						 duplicate sysfs file
> > 
> > 
> > 
> > Issue Log :
> > -----------
> > 
> > [CPU 2] = sriov_numfs creating 9 vfs
> > [CPU 4] = Pci rescan
> > 
> > [   93.486440] [CPU : 2]`==>pci_iov_add_virtfn: bus = PCI Bus 0002:20 slot = 0
> > func= 4 	# sriov_numvfs vf is getting created for vf 4
> > [   93.494002] [CPU : 4]`->pci_bus_add_devices: child-bus =
> > 				    # Pci rescan
> > [   93.494003] [CPU : 4]`->pci_bus_add_devices: bus = PCI Bus 0002:20
> > 
> > [   93.500178] pci 0002:20:00.4: [177d:a0f3] type 00 class 0x108000
> > [   93.507825] [CPU : 4]`->pci_bus_add_devices iterated dev : slot =  0 func = 0
> > # pci rescan iterating on created vfs
> > [   93.513288] [CPU : 2]`->pci_device_add : bus = PCI Bus 0002:20 slot = 0,
> > func= 4     # sriov_numvfs: vf 4 added in the cus list
> > [   93.519388] [CPU : 4]`->pci_bus_add_devices iterated dev : slot =  0 func = 1
> > 	    # pci rescan : vf iterated 1
> > [   93.525438] [CPU : 2]`->pci_bus_add_device: slot = 0 func= 4	                        #
> > sriov_numvfs: enter in adding vf 4 in sys/proc fs
> > [   93.532515] [CPU : 4]`->pci_bus_add_devices iterated dev : slot =  0 func = 2
> > 	    # pci rescan : vf iterated 2
> > [   93.539904] [CPU : 2]`->pci_bus_add_device create sysfs
> > pci_create_sysfs_dev_files: slot = 0 func= 4 # sriov: vf 4 sysfs file created
> > [   93.547032] [CPU : 4]`->pci_bus_add_devices iterated dev : slot =  0 func = 3
> > # pci rescan : vf iterated 3
> > [   93.552714] rvu_cptvf 0002:20:00.4: Adding to iommu group 85
> > [   93.559812] [CPU : 4]`->pci_bus_add_devices iterated dev : slot =  0 func = 4
> > 	    # pci rescan : vf iterated 4
> > [   93.569002] rvu_cptvf 0002:20:00.4: enabling device (0000 -> 0002)
> > [   93.576069] [CPU : 4]`->pci_bus_add_devices PCI_DEV_ADDED not set : slot
> > =  0 func = 4 # pci rescan : vf 4 PCI_DEV_ADDED flag not set by sriov_numvfs
> > [   93.576070] [CPU : 4]`->pci_bus_add_device: slot = 0 func= 4
> > 					  # pci rescan : enter for adding vf 4 in
> > sys/proc fs
> > [   93.576072] [CPU : 4]`->pci_bus_add_device create sysfs
> > pci_create_sysfs_dev_files: slot = 0 func= 4 # pci rescan : going to create sysfs
> > file
> > [   93.576078] sysfs: cannot create duplicate filename
> > '/devices/platform/0.soc/878020000000.pci/pci0002:00/0002:00:1f.0/000
> > 2:20:00.4/resource2' # duplication detected
> > [   93.608709] [CPU : 2]`->pci_dev_assign_added set PCI_DEV_ADDED : slot =
> > 0, func= 4        # sriov_numvfs : PCI_DEV_ADDED is set
> > [   93.617714] CPU: 4 PID: 811 Comm: tee Not tainted 6.1.67-00054-
> > g3acfa4185b96-dirty #159
> > [   93.630399] [CPU : 2]<==pci_iov_add_virtfn: bus = PCI Bus 0002:20 slot = 0
> > func= 4
> > 
> > 
> > Fix trace (with patch):
> > -----------------------
> > 
> > CPU2 (sriov_numvfs)                   CPU4 (pci rescan)
> > ----------------------------------   --------------------------
> > pci_iov_add_virtfn()
> >   pci_device_add(virtfn)   # VF linked to bus
> > 	pci_bus_add_device() enter
> > 		set PCI_DEV_ADD_INPROG
> >                                      pci_bus_add_device() enter
> >                                      PCI_DEV_ADD_INPROG already set
> >                                      return
> > 	pci_dev_assign_added()
> > 	pci_dev_clear_inprog()
> > 
> > Fix log:
> > -------
> > 
> > [CPU 2] = sriov_numfs creating 9 vfs
> > [CPU 4] = Pci rescan
> > 
> > [  178.296167] pci 0002:20:00.5: [177d:a0f3] type 00 class 0x108000 [
> > 178.302680] pci 0002:00:1b.0: PCI bridge to [bus 1c]
> > [  178.307746] [CPU : 2]`->pci_bus_add_device Enter : slot = 0, func= 5   #
> > sriov_numvfs: adding vf5 in sys/proc
> > [  178.313636] pci 0002:00:1c.0: PCI bridge to [bus 1d] [  178.318592] [CPU
> > : 2]`->pci_bus_add_device set PCI_DEV_ADD_INPROG : slot = 0, func= 5  #
> > sriov_numvfs: vf 5 PCI_DEV_ADD_INPROG flag set [  178.324939] pci
> > 0002:00:1d.0: PCI bridge to [bus 1e] [  178.329895] [CPU : 2]`-
> > >pci_bus_add_device PCI_DEV_ADDED is not set: slot = 0, func= 5 #
> > sriov_numvfs: vf 5 check if PCI_DEV_ADDED flag is set before proceed to
> > create sysfs file [  178.337719] pci 0002:00:1e.0: PCI bridge to [bus 1f] [
> > 178.342704] rvu_cptvf 0002:20:00.5: Adding to iommu group 86
> > [  178.350586] [CPU : 4]`->pci_bus_add_device Enter : slot = 0, func= 5     # pci
> > rescan : since PCI_DEV_ADDED flag is not set it enter the pci_bus_add_device
> > for vf 5
> > [  178.355597] rvu_cptvf 0002:20:00.5: enabling device (0000 -> 0002) [
> > 178.361193] [CPU : 4]`->pci_bus_add_device PCI_DEV_ADD_INPROG is
> > already set : slot = 0, func= 5  # pci rescan : check PCI_DEV_ADD_INPROG flag,
> > it is already set
> > [  178.373726] [CPU : 4] <- pci_bus_add_device return : slot = 0, func= 5
> > 	# pri rescan : return
> > [  178.382852] pci_bus 0003:01: busn_res: [bus 01] end is updated to 01
> > [  178.395474] [CPU : 2]`->pci_dev_assign_added set PCI_DEV_ADDED : slot =
> > 0, func= 5    # sriov_numvfs: set PCI_DEV_ADDED for vf5
> > [  178.395721] [CPU : 2]`->pci_dev_clear_inprog unset
> > PCI_DEV_ADD_INPROG : slot = 0, func= 5 # sriov_numvfs : clear
> > PCI_DEV_ADD_INPROG for vf5 [  178.403289] [CPU : 2] <-
> > pci_bus_add_device return : slot = 0, func= 5  drivers/pci/bus.c |  8 ++++++++
> > drivers/pci/pci.h | 11 +++++++++++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index
> > feafa378bf8e..cafce1c4ec3d 100644
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -331,6 +331,13 @@ void pci_bus_add_device(struct pci_dev *dev)  {
> >  	int retval;
> > 
> > +	if (pci_dev_add_inprog_check_and_set(dev))
> > +		return;
> > +
> > +	if (pci_dev_is_added(dev)) {
> > +		pci_dev_clear_inprog(dev);
> > +		return;
> > +	}
> >  	/*
> >  	 * Can not put in pci_device_add yet because resources
> >  	 * are not assigned yet for some devices.
> > @@ -347,6 +354,7 @@ void pci_bus_add_device(struct pci_dev *dev)
> >  		pci_warn(dev, "device attach failed (%d)\n", retval);
> > 
> >  	pci_dev_assign_added(dev, true);
> > +	pci_dev_clear_inprog(dev);
> >  }
> >  EXPORT_SYMBOL_GPL(pci_bus_add_device);
> > 
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index
> > ffccb03933e2..a2d01db8e837 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -366,17 +366,28 @@ static inline bool pci_dev_is_disconnected(const
> > struct pci_dev *dev)  #define PCI_DEV_ADDED 0  #define
> > PCI_DPC_RECOVERED 1  #define PCI_DPC_RECOVERING 2
> > +#define PCI_DEV_ADD_INPROG 3
> > 
> >  static inline void pci_dev_assign_added(struct pci_dev *dev, bool added)  {
> >  	assign_bit(PCI_DEV_ADDED, &dev->priv_flags, added);  }
> > 
> > +static inline void pci_dev_clear_inprog(struct pci_dev *dev) {
> > +	clear_bit(PCI_DEV_ADD_INPROG, &dev->priv_flags); }
> > +
> >  static inline bool pci_dev_is_added(const struct pci_dev *dev)  {
> >  	return test_bit(PCI_DEV_ADDED, &dev->priv_flags);  }
> > 
> > +static inline bool pci_dev_add_inprog_check_and_set(struct pci_dev
> > +*dev) {
> > +	return test_and_set_bit(PCI_DEV_ADD_INPROG, &dev->priv_flags); }
> > +
> >  #ifdef CONFIG_PCIEAER
> >  #include <linux/aer.h>
> > 
> > --
> > 2.34.1
>