Enable SRIOV concurrently with many different PFs in userspace
will get warnings below:
Warning 1:
sysfs: cannot create duplicate filename
'/devices/pci0000:30/0000:30:02.0/pci_bus/0000:32'
Call Trace:
dump_stack+0x6f/0xab
sysfs_warn_dup+0x56/0x70
sysfs_create_dir_ns+0x80/0x90
kobject_add_internal+0xa0/0x2b0
kobject_add+0x71/0xd0
device_add+0x126/0x630
pci_add_new_bus+0x17c/0x4b0
pci_iov_add_virtfn+0x336/0x390
sriov_enable+0x26e/0x450
virtio_pci_sriov_configure+0x61/0xc0 [virtio_pci]
Warning 2:
proc_dir_entry 'pci/33' already registered
WARNING: CPU: 71 PID: 893 at fs/proc/generic.c:360
proc_register+0xf8/0x130
Call Trace:
proc_mkdir_data+0x5d/0x80
pci_proc_attach_device+0xe9/0x120
pci_bus_add_device+0x33/0x90
pci_iov_add_virtfn+0x375/0x390
sriov_enable+0x26e/0x450
virtio_pci_sriov_configure+0x61/0xc0 [virtio_pci]
The reason is:
1）different VFs may create the same pci bus number and try to add new bus
concurrently in virtfn_add_bus.
2）different VFs may create '/proc/bus/pci/bus_number' directory using the
same bus number in pci_proc_attach_device concurrently.
Add new lock when creating new bus, hold device_lock when creating
'/proc/bus/pci/bus_number' directory in pci_proc_attach_device to fix them
Signed-off-by: Zhuang Shengen <[email protected]>
Signed-off-by: Jay Zhou <[email protected]>
---
drivers/pci/iov.c | 7 ++++++-
drivers/pci/probe.c | 26 ++++++++++++++++++++++++++
drivers/pci/proc.c | 6 +++++-
include/linux/pci.h | 2 ++
4 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 952217572113..6d113ea64ba8 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -127,11 +127,16 @@ static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
if (bus->number == busnr)
return bus;
+ /*
+ * vf will find existing bus at most case; if not existing, it should
+ * go through slow path to create new bus with locked to support enable SRIOV
+ * concurrently with different PFs in userspace.
+ */
child = pci_find_bus(pci_domain_nr(bus), busnr);
if (child)
return child;
- child = pci_add_new_bus(bus, NULL, busnr);
+ child = pci_add_new_bus_locked(bus, NULL, busnr);
if (!child)
return NULL;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c5286b027f00..bfbd47a3ba73 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -25,6 +25,8 @@
#define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */
#define CARDBUS_RESERVE_BUSNR 3
+DEFINE_MUTEX(add_bus_mutex);
+
static struct resource busn_resource = {
.name = "PCI busn",
.start = 0,
@@ -1170,6 +1172,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
}
EXPORT_SYMBOL(pci_add_new_bus);
+struct pci_bus *pci_add_new_bus_locked(struct pci_bus *parent, struct pci_dev *dev,
+ int busnr)
+{
+ struct pci_bus *child;
+
+ mutex_lock(&add_bus_mutex);
+ child = pci_find_bus(pci_domain_nr(parent), busnr);
+ if (child) {
+ mutex_unlock(&add_bus_mutex);
+ return child;
+ }
+
+ child = pci_alloc_child_bus(parent, dev, busnr);
+ if (child) {
+ down_write(&pci_bus_sem);
+ list_add_tail(&child->node, &parent->children);
+ up_write(&pci_bus_sem);
+ }
+ mutex_unlock(&add_bus_mutex);
+
+ return child;
+}
+EXPORT_SYMBOL(pci_add_new_bus_locked);
+
static void pci_enable_crs(struct pci_dev *pdev)
{
u16 root_cap = 0;
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index f967709082d6..f927263c2fe0 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -421,6 +421,7 @@ int pci_proc_attach_device(struct pci_dev *dev)
if (!proc_initialized)
return -EACCES;
+ device_lock(&bus->dev);
if (!bus->procdir) {
if (pci_proc_domain(bus)) {
sprintf(name, "%04x:%02x", pci_domain_nr(bus),
@@ -429,9 +430,12 @@ int pci_proc_attach_device(struct pci_dev *dev)
sprintf(name, "%02x", bus->number);
}
bus->procdir = proc_mkdir(name, proc_bus_pci_dir);
- if (!bus->procdir)
+ if (!bus->procdir) {
+ device_unlock(&bus->dev);
return -ENOMEM;
+ }
}
+ device_unlock(&bus->dev);
sprintf(name, "%02x.%x", PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
e = proc_create_data(name, S_IFREG | S_IRUGO | S_IWUSR, bus->procdir,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5da0846aa3c1..bfe2a2e74af6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1112,6 +1112,8 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
int pci_scan_root_bus_bridge(struct pci_host_bridge *bridge);
struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
int busnr);
+struct pci_bus *pci_add_new_bus_locked(struct pci_bus *parent, struct pci_dev *dev,
+ int busnr);
struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
const char *name,
struct hotplug_slot *hotplug);
--
2.27.0
Hi Zhuang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on linus/master v6.0 next-20221007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zhuang-Shengen/iov-support-enable-pci-sriov-concurrently/20221008-104400
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-s022
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/89e62ddba75cc10e9511515cd761773beaa0f8db
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Zhuang-Shengen/iov-support-enable-pci-sriov-concurrently/20221008-104400
git checkout 89e62ddba75cc10e9511515cd761773beaa0f8db
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/pci/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
>> drivers/pci/probe.c:28:1: sparse: sparse: symbol 'add_bus_mutex' was not declared. Should it be static?
--
0-DAY CI Kernel Test Service
https://01.org/lkp