[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<SN7PR18MB53145C30677F872D61EE49AEE3FCA@SN7PR18MB5314.namprd18.prod.outlook.com>
Date: Mon, 27 Oct 2025 14:45:21 +0000
From: Bharat Bhushan <bbhushan2@...vell.com>
To: Shashank Gupta <shashankg@...vell.com>,
Bjorn Helgaas
<bhelgaas@...gle.com>,
"linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
CC: Shashank Gupta <shashankg@...vell.com>,
Sunil Kovvuri Goutham
<sgoutham@...vell.com>
Subject: RE: [RFC PATCH] PCI: fix concurrent pci_bus_add_device() execution
during SR-IOV VF creation
> -----Original Message-----
> From: Shashank Gupta <shashankg@...vell.com>
> Sent: Thursday, September 11, 2025 4:41 PM
> To: Bjorn Helgaas <bhelgaas@...gle.com>; linux-pci@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Cc: Shashank Gupta <shashankg@...vell.com>; Bharat Bhushan
> <bbhushan2@...vell.com>; Sunil Kovvuri Goutham
> <sgoutham@...vell.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
>
> Signed-off-by: Shashank Gupta <shashankg@...vell.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
Powered by blists - more mailing lists