2013-05-25 13:51:06

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 00/10] Prepare for introducing PCI bus lock interfaces

This is a preparation patchset for introducing PCI bus lock mechanisms
to protect PCI subsystem from concurrent hotplug operations.

Patch 1:
Introduce pci_bus_{get|put}() to manage PCI bus reference count
Patch 2-3:
pci_alloc_dev() patchset from Gu Zheng
Patch 6:
Make PCI bus creating/destroying logic symmetric
Patch 8-10:
Cleanup and bugfix for IOV
Other:
Minor code improvements/cleanups

Gu Zheng (2):
PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace
alloc_pci_dev()
PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

Jiang Liu (8):
PCI: introduce pci_bus_{get|put}() to manage PCI bus reference count
PCI: mark pci_scan_bus_parented() as __deprecated
PCI, IA64: minor code clean up
PCI: make PCI host bridge/bus creating and destroying logic symmetric
PCI, unicore, m68k: remove redundant call of pci_bus_add_devices()
PCI, IOV: don't touch bus->is_added flag
PCI, IOV: simplify IOV implementation
PCI, IOV: hide remove and rescan sysfs interfaces for SR-IOV virtual
functions

arch/ia64/sn/kernel/io_init.c | 11 +---
arch/m68k/platform/coldfire/pci.c | 2 +-
arch/powerpc/kernel/pci_of_scan.c | 3 +-
arch/sparc/kernel/pci.c | 3 +-
arch/tile/kernel/pci.c | 3 --
arch/unicore32/kernel/pci.c | 5 --
drivers/char/agp/alpha-agp.c | 2 +-
drivers/char/agp/parisc-agp.c | 2 +-
drivers/pci/bus.c | 15 ++++++
drivers/pci/iov.c | 64 +++++++++--------------
drivers/pci/pci-sysfs.c | 30 ++++++++++-
drivers/pci/probe.c | 105 ++++++++++++++++++--------------------
drivers/pci/remove.c | 3 +-
drivers/scsi/megaraid.c | 2 +-
include/linux/pci.h | 9 ++--
15 files changed, 134 insertions(+), 125 deletions(-)

--
1.8.1.2


2013-05-25 13:51:17

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 01/10] PCI: introduce pci_bus_{get|put}() to manage PCI bus reference count

Introduce helper functions pci_bus_{get|put}() to manage PCI bus
reference count.

Signed-off-by: Jiang Liu <[email protected]>
Signed-off-by: Yijing Wang <[email protected]>
Signed-off-by: Gu Zheng <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/bus.c | 15 +++++++++++++++
include/linux/pci.h | 2 ++
2 files changed, 17 insertions(+)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 32e66a6..b1ff02a 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -283,6 +283,21 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *),
}
EXPORT_SYMBOL_GPL(pci_walk_bus);

+struct pci_bus *pci_bus_get(struct pci_bus *bus)
+{
+ if (bus)
+ get_device(&bus->dev);
+ return bus;
+}
+EXPORT_SYMBOL(pci_bus_get);
+
+void pci_bus_put(struct pci_bus *bus)
+{
+ if (bus)
+ put_device(&bus->dev);
+}
+EXPORT_SYMBOL(pci_bus_put);
+
EXPORT_SYMBOL(pci_bus_alloc_resource);
EXPORT_SYMBOL_GPL(pci_bus_add_device);
EXPORT_SYMBOL(pci_bus_add_devices);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3a24e4f..7556c59 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1018,6 +1018,8 @@ int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char *);
void pci_release_selected_regions(struct pci_dev *, int);

/* drivers/pci/bus.c */
+struct pci_bus *pci_bus_get(struct pci_bus *bus);
+void pci_bus_put(struct pci_bus *bus);
void pci_add_resource(struct list_head *resources, struct resource *res);
void pci_add_resource_offset(struct list_head *resources, struct resource *res,
resource_size_t offset);
--
1.8.1.2

2013-05-25 13:51:29

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 02/10] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

From: Gu Zheng <[email protected]>

Now here we introduce a new interface to replace alloc_pci_dev():
struct pci_dev *pci_alloc_dev(struct pci_bus *bus)

It take a "struct pci_bus *" argument, so we can alloc a pci device
on a target pci bus, and it acquire the reference of the pci_bus.
We use pci_alloc_dev(NULL) to simplify the old alloc_pci_dev(),
and keep it for a while but mark it as __deprecated.

Jiang Liu <[email protected]>
This also makes it safe to reference pci_dev->bus when holding a
reference on a pci_dev.

Also change
__deprecated struct pci_dev * alloc_pci_dev(void);
to
struct pci_dev * __deprecated alloc_pci_dev(void);
to follow common coding style.

Signed-off-by: Gu Zheng <[email protected]>
Signed-off-by: Jiang Liu <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/probe.c | 18 ++++++++++++------
include/linux/pci.h | 3 ++-
2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 70f10fa..26df9c8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1200,19 +1200,25 @@ static void pci_release_bus_bridge_dev(struct device *dev)
kfree(bridge);
}

-struct pci_dev *alloc_pci_dev(void)
+struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
{
struct pci_dev *dev;

dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
- if (!dev)
- return NULL;
-
- INIT_LIST_HEAD(&dev->bus_list);
- dev->dev.type = &pci_dev_type;
+ if (dev) {
+ INIT_LIST_HEAD(&dev->bus_list);
+ dev->dev.type = &pci_dev_type;
+ dev->bus = pci_bus_get(bus);
+ }

return dev;
}
+EXPORT_SYMBOL(pci_alloc_dev);
+
+struct pci_dev *alloc_pci_dev(void)
+{
+ return pci_alloc_dev(NULL);
+}
EXPORT_SYMBOL(alloc_pci_dev);

bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 7556c59..b0f4a82 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -364,7 +364,8 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
return dev;
}

-struct pci_dev *alloc_pci_dev(void);
+struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
+struct pci_dev * __deprecated alloc_pci_dev(void);

#define to_pci_dev(n) container_of(n, struct pci_dev, dev)
#define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
--
1.8.1.2

2013-05-25 13:51:43

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

Mark pci_scan_bus_parented() as __deprecated and clean up outdated
comments.

Signed-off-by: Jiang Liu <[email protected]>
Cc: Chris Metcalf <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/tile/kernel/pci.c | 3 ---
include/linux/pci.h | 4 ++--
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 67237d3..936e087 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -309,9 +309,6 @@ int __init pcibios_init(void)
*
* It reads the PCI tree for this bus into the Linux
* data structures.
- *
- * This is inlined in linux/pci.h and calls into
- * pci_scan_bus_parented() in probe.c.
*/
pci_add_resource(&resources, &ioport_resource);
pci_add_resource(&resources, &iomem_resource);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b0f4a82..7b23fa0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -720,8 +720,8 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
void pcibios_scan_specific_bus(int busn);
struct pci_bus *pci_find_bus(int domain, int busnr);
void pci_bus_add_devices(const struct pci_bus *bus);
-struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
- struct pci_ops *ops, void *sysdata);
+struct pci_bus * __deprecated pci_scan_bus_parented(struct device *parent,
+ int bus, struct pci_ops *ops, void *sysdata);
struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
struct pci_ops *ops, void *sysdata,
--
1.8.1.2

2013-05-25 13:51:38

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 03/10] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

From: Gu Zheng <[email protected]>

Use the new pci_alloc_dev(bus) to replace the existing using of
alloc_pci_dev(void).

v2:
Follow Bjorn's correction to move pci_bus_put() to
pci_release_dev() instead.

v3:
release reference to bus on error recovery path

Signed-off-by: Gu Zheng <[email protected]>
Signed-off-by: Jiang Liu <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Neela Syam Kolli <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/powerpc/kernel/pci_of_scan.c | 3 +--
arch/sparc/kernel/pci.c | 3 +--
drivers/char/agp/alpha-agp.c | 2 +-
drivers/char/agp/parisc-agp.c | 2 +-
drivers/pci/iov.c | 8 +++++---
drivers/pci/probe.c | 5 +++--
drivers/scsi/megaraid.c | 2 +-
7 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index d2d407d..6b0ba58 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -128,7 +128,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,
const char *type;
struct pci_slot *slot;

- dev = alloc_pci_dev();
+ dev = pci_alloc_dev(bus);
if (!dev)
return NULL;
type = of_get_property(node, "device_type", NULL);
@@ -137,7 +137,6 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,

pr_debug(" create device, devfn: %x, type: %s\n", devfn, type);

- dev->bus = bus;
dev->dev.of_node = of_node_get(node);
dev->dev.parent = bus->bridge;
dev->dev.bus = &pci_bus_type;
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 972892a..b16f624 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -254,7 +254,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
const char *type;
u32 class;

- dev = alloc_pci_dev();
+ dev = pci_alloc_dev(bus);
if (!dev)
return NULL;

@@ -281,7 +281,6 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
printk(" create device, devfn: %x, type: %s\n",
devfn, type);

- dev->bus = bus;
dev->sysdata = node;
dev->dev.parent = bus->bridge;
dev->dev.bus = &pci_bus_type;
diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c
index dd84af4..199b8e9 100644
--- a/drivers/char/agp/alpha-agp.c
+++ b/drivers/char/agp/alpha-agp.c
@@ -174,7 +174,7 @@ alpha_core_agp_setup(void)
/*
* Build a fake pci_dev struct
*/
- pdev = alloc_pci_dev();
+ pdev = pci_alloc_dev(NULL);
if (!pdev)
return -ENOMEM;
pdev->vendor = 0xffff;
diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
index 94821ab..bf5d247 100644
--- a/drivers/char/agp/parisc-agp.c
+++ b/drivers/char/agp/parisc-agp.c
@@ -333,7 +333,7 @@ parisc_agp_setup(void __iomem *ioc_hpa, void __iomem *lba_hpa)
struct agp_bridge_data *bridge;
int error = 0;

- fake_bridge_dev = alloc_pci_dev();
+ fake_bridge_dev = pci_alloc_dev(NULL);
if (!fake_bridge_dev) {
error = -ENOMEM;
goto fail;
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index c93071d..2652ca0 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -75,18 +75,20 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
struct pci_dev *virtfn;
struct resource *res;
struct pci_sriov *iov = dev->sriov;
+ struct pci_bus *bus;

- virtfn = alloc_pci_dev();
+ virtfn = pci_alloc_dev(NULL);
if (!virtfn)
return -ENOMEM;

mutex_lock(&iov->dev->sriov->lock);
- virtfn->bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id));
- if (!virtfn->bus) {
+ bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id));
+ if (!bus) {
kfree(virtfn);
mutex_unlock(&iov->dev->sriov->lock);
return -ENOMEM;
}
+ virtfn->bus = pci_bus_get(bus);
virtfn->devfn = virtfn_devfn(dev, id);
virtfn->vendor = dev->vendor;
pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 26df9c8..d5d18a0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1130,6 +1130,7 @@ static void pci_release_dev(struct device *dev)
struct pci_dev *pci_dev;

pci_dev = to_pci_dev(dev);
+ pci_bus_put(pci_dev->bus);
pci_release_capabilities(pci_dev);
pci_release_of_node(pci_dev);
kfree(pci_dev);
@@ -1269,11 +1270,10 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
if (!pci_bus_read_dev_vendor_id(bus, devfn, &l, 60*1000))
return NULL;

- dev = alloc_pci_dev();
+ dev = pci_alloc_dev(bus);
if (!dev)
return NULL;

- dev->bus = bus;
dev->devfn = devfn;
dev->vendor = l & 0xffff;
dev->device = (l >> 16) & 0xffff;
@@ -1281,6 +1281,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
pci_set_of_node(dev);

if (pci_setup_device(dev)) {
+ pci_bus_put(dev->bus);
kfree(dev);
return NULL;
}
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 846f475f..90c95a3 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -2026,7 +2026,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor)
static inline int
make_local_pdev(adapter_t *adapter, struct pci_dev **pdev)
{
- *pdev = alloc_pci_dev();
+ *pdev = pci_alloc_dev(NULL);

if( *pdev == NULL ) return -1;

--
1.8.1.2

2013-05-25 13:51:48

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 05/10] PCI, IA64: minor code clean up

pci_scan_root_bus() already set bus->sysdata, so no need to explicitly
set it again in function sn_pci_controller_fixup();

Signed-off-by: Jiang Liu <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/ia64/sn/kernel/io_init.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.c
index 238e2c5..e2c7733 100644
--- a/arch/ia64/sn/kernel/io_init.c
+++ b/arch/ia64/sn/kernel/io_init.c
@@ -326,16 +326,7 @@ sn_pci_controller_fixup(int segment, int busnum, struct pci_bus *bus)
bus = pci_scan_root_bus(NULL, busnum, &pci_root_ops, controller,
&resources);
if (bus == NULL)
- goto error_return; /* error, or bus already scanned */
-
- bus->sysdata = controller;
-
- return;
-
-error_return:
-
- kfree(controller);
- return;
+ kfree(controller);
}

/*
--
1.8.1.2

2013-05-25 13:51:58

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 07/10] PCI, unicore, m68k: remove redundant call of pci_bus_add_devices()

pci_scan_bus() and pci_scan_root_bus() has called pci_bus_add_devices()
once, so remove the redundant call of pci_bus_add_devices().
On the other handle, subsys_init() callbacks will be invoked before
device_init() callbacks, so it should be safe to remove the redundant
calls.

Signed-off-by: Jiang Liu <[email protected]>
---
arch/m68k/platform/coldfire/pci.c | 2 +-
arch/unicore32/kernel/pci.c | 5 -----
2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/m68k/platform/coldfire/pci.c b/arch/m68k/platform/coldfire/pci.c
index 8572246..2345972 100644
--- a/arch/m68k/platform/coldfire/pci.c
+++ b/arch/m68k/platform/coldfire/pci.c
@@ -320,7 +320,7 @@ static int __init mcf_pci_init(void)
pci_bus_size_bridges(rootbus);
pci_bus_assign_resources(rootbus);
pci_enable_bridges(rootbus);
- pci_bus_add_devices(rootbus);
+
return 0;
}

diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index ef69c0c..374a055 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -277,11 +277,6 @@ static int __init pci_common_init(void)
pci_bus_assign_resources(puv3_bus);
}

- /*
- * Tell drivers about devices found.
- */
- pci_bus_add_devices(puv3_bus);
-
return 0;
}
subsys_initcall(pci_common_init);
--
1.8.1.2

2013-05-25 13:52:10

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 09/10] PCI, IOV: simplify IOV implementation

Trivial changes to IOV:
1) use new PCI interfaces to simplify IOV implementation
2) fix some reference count related race windows

Signed-off-by: Jiang Liu <[email protected]>
Cc: Donald Dutile <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Ram Pai <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/iov.c | 58 +++++++++++++++++++++----------------------------------
1 file changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 8f1e117..3e33499 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -51,43 +51,30 @@ static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
return child;
}

-static void virtfn_remove_bus(struct pci_bus *bus, int busnr)
+static void virtfn_remove_bus(struct pci_bus *physbus, struct pci_bus *virtbus)
{
- struct pci_bus *child;
-
- if (bus->number == busnr)
- return;
-
- child = pci_find_bus(pci_domain_nr(bus), busnr);
- BUG_ON(!child);
-
- if (list_empty(&child->devices))
- pci_remove_bus(child);
+ if (physbus != virtbus && list_empty(&virtbus->devices))
+ pci_remove_bus(virtbus);
}

static int virtfn_add(struct pci_dev *dev, int id, int reset)
{
int i;
- int rc;
+ int rc = -ENOMEM;
u64 size;
char buf[VIRTFN_ID_LEN];
- struct pci_dev *virtfn;
+ struct pci_dev *virtfn = NULL;
struct resource *res;
struct pci_sriov *iov = dev->sriov;
struct pci_bus *bus;

- virtfn = pci_alloc_dev(NULL);
- if (!virtfn)
- return -ENOMEM;
-
mutex_lock(&iov->dev->sriov->lock);
bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id));
- if (!bus) {
- kfree(virtfn);
- mutex_unlock(&iov->dev->sriov->lock);
- return -ENOMEM;
- }
- virtfn->bus = pci_bus_get(bus);
+ if (bus)
+ virtfn = pci_alloc_dev(bus);
+ if (!virtfn)
+ goto failed0;
+
virtfn->devfn = virtfn_devfn(dev, id);
virtfn->vendor = dev->vendor;
pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
@@ -136,7 +123,8 @@ failed1:
pci_dev_put(dev);
mutex_lock(&iov->dev->sriov->lock);
pci_stop_and_remove_bus_device(virtfn);
- virtfn_remove_bus(dev->bus, virtfn_bus(dev, id));
+ virtfn_remove_bus(dev->bus, bus);
+failed0:
mutex_unlock(&iov->dev->sriov->lock);

return rc;
@@ -145,20 +133,15 @@ failed1:
static void virtfn_remove(struct pci_dev *dev, int id, int reset)
{
char buf[VIRTFN_ID_LEN];
- struct pci_bus *bus;
struct pci_dev *virtfn;
struct pci_sriov *iov = dev->sriov;

- bus = pci_find_bus(pci_domain_nr(dev->bus), virtfn_bus(dev, id));
- if (!bus)
- return;
-
- virtfn = pci_get_slot(bus, virtfn_devfn(dev, id));
+ virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
+ virtfn_bus(dev, id),
+ virtfn_devfn(dev, id));
if (!virtfn)
return;

- pci_dev_put(virtfn);
-
if (reset) {
device_release_driver(&virtfn->dev);
__pci_reset_function(virtfn);
@@ -176,9 +159,11 @@ static void virtfn_remove(struct pci_dev *dev, int id, int reset)

mutex_lock(&iov->dev->sriov->lock);
pci_stop_and_remove_bus_device(virtfn);
- virtfn_remove_bus(dev->bus, virtfn_bus(dev, id));
+ virtfn_remove_bus(dev->bus, virtfn->bus);
mutex_unlock(&iov->dev->sriov->lock);

+ /* balance pci_get_domain_bus_and_slot() */
+ pci_dev_put(virtfn);
pci_dev_put(dev);
}

@@ -335,13 +320,14 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
if (!pdev)
return -ENODEV;

- pci_dev_put(pdev);
-
- if (!pdev->is_physfn)
+ if (!pdev->is_physfn) {
+ pci_dev_put(pdev);
return -ENODEV;
+ }

rc = sysfs_create_link(&dev->dev.kobj,
&pdev->dev.kobj, "dep_link");
+ pci_dev_put(pdev);
if (rc)
return rc;
}
--
1.8.1.2

2013-05-25 13:52:06

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 08/10] PCI, IOV: don't touch bus->is_added flag

The flag pci_bus->is_added is used to guard invoke of
pcibios_fixup_bus(pci_bus). And when virtfn_add_bus() is called,
pci_bus->is_added flag has already been set, so remove the redandunt
bus->is_added = 1;

Signed-off-by: Jiang Liu <[email protected]>
Cc: Donald Dutile <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Ram Pai <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/iov.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 2652ca0..8f1e117 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -47,7 +47,6 @@ static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
return NULL;

pci_bus_insert_busn_res(child, busnr, busnr);
- bus->is_added = 1;

return child;
}
--
1.8.1.2

2013-05-25 13:52:16

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 10/10] PCI, IOV: hide remove and rescan sysfs interfaces for SR-IOV virtual functions

PCI devices for SR-IOV virtual functions should only be created/
destroyed by pci_enable_sriov()/pci_disable_sriov() because special
data structures are assoicated with SR-IOV virtual functions.
So hide hotplug related sysfs interfaces "remove" and "rescan" for
SR-IOV virtual functions, otherwise it may causes memory leakage
and other issues.

Signed-off-by: Jiang Liu <[email protected]>
Signed-off-by: Yijing Wang <[email protected]>
Cc: Donald Dutile <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Ram Pai <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/iov.c | 5 ++---
drivers/pci/pci-sysfs.c | 30 ++++++++++++++++++++++++++++--
2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 3e33499..5eb8165 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -80,6 +80,8 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
pci_setup_device(virtfn);
virtfn->dev.parent = dev->dev.parent;
+ virtfn->physfn = pci_dev_get(dev);
+ virtfn->is_virtfn = 1;

for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
res = dev->resource + PCI_IOV_RESOURCES + i;
@@ -101,9 +103,6 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
pci_device_add(virtfn, virtfn->bus);
mutex_unlock(&iov->dev->sriov->lock);

- virtfn->physfn = pci_dev_get(dev);
- virtfn->is_virtfn = 1;
-
rc = pci_bus_add_device(virtfn);
sprintf(buf, "virtfn%u", id);
rc = sysfs_create_link(&dev->dev.kobj, &virtfn->dev.kobj, buf);
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5b4a9d9..403da60 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -325,6 +325,8 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
}
return count;
}
+struct device_attribute dev_rescan_attr = __ATTR(rescan, (S_IWUSR|S_IWGRP),
+ NULL, dev_rescan_store);

static void remove_callback(struct device *dev)
{
@@ -354,6 +356,8 @@ remove_store(struct device *dev, struct device_attribute *dummy,
count = ret;
return count;
}
+struct device_attribute dev_remove_attr = __ATTR(remove, (S_IWUSR|S_IWGRP),
+ NULL, remove_store);

static ssize_t
dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
@@ -504,8 +508,6 @@ struct device_attribute pci_dev_attrs[] = {
__ATTR(broken_parity_status,(S_IRUGO|S_IWUSR),
broken_parity_status_show,broken_parity_status_store),
__ATTR(msi_bus, 0644, msi_bus_show, msi_bus_store),
- __ATTR(remove, (S_IWUSR|S_IWGRP), NULL, remove_store),
- __ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, dev_rescan_store),
#if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI)
__ATTR(d3cold_allowed, 0644, d3cold_allowed_show, d3cold_allowed_store),
#endif
@@ -1463,6 +1465,29 @@ static umode_t pci_dev_attrs_are_visible(struct kobject *kobj,
return a->mode;
}

+static struct attribute *pci_dev_hp_attrs[] = {
+ &dev_remove_attr.attr,
+ &dev_rescan_attr.attr,
+ NULL,
+};
+
+static umode_t pci_dev_hp_attrs_are_visible(struct kobject *kobj,
+ struct attribute *a, int n)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ if (pdev->is_virtfn)
+ return 0;
+
+ return a->mode;
+}
+
+static struct attribute_group pci_dev_hp_attr_group = {
+ .attrs = pci_dev_hp_attrs,
+ .is_visible = pci_dev_hp_attrs_are_visible,
+};
+
#ifdef CONFIG_PCI_IOV
static struct attribute *sriov_dev_attrs[] = {
&sriov_totalvfs_attr.attr,
@@ -1494,6 +1519,7 @@ static struct attribute_group pci_dev_attr_group = {

static const struct attribute_group *pci_dev_attr_groups[] = {
&pci_dev_attr_group,
+ &pci_dev_hp_attr_group,
#ifdef CONFIG_PCI_IOV
&sriov_dev_attr_group,
#endif
--
1.8.1.2

2013-05-25 13:51:55

by Jiang Liu

[permalink] [raw]
Subject: [PATCH v3, part1 06/10] PCI: make PCI host bridge/bus creating and destroying logic symmetric

This patch makes PCI host bridge/bus creating and destroying logic
symmetric by using device_initialize()/device_add()/device_del()/put_device()
pairs as discussed in thread at:
http://comments.gmane.org/gmane.linux.kernel.pci/22073

It also fixes a bug in error recovery path in pci_create_root_bus()
which may kfree() an in-use host bridge object.

Signed-off-by: Jiang Liu <[email protected]>
Cc: Yinghai Lu <[email protected]>
---
drivers/pci/probe.c | 84 +++++++++++++++++++++++-----------------------------
drivers/pci/remove.c | 3 +-
2 files changed, 39 insertions(+), 48 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index d5d18a0..c9a5ce7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -451,7 +451,7 @@ void pci_read_bridge_bases(struct pci_bus *child)
}
}

-static struct pci_bus * pci_alloc_bus(void)
+static struct pci_bus *pci_alloc_bus(struct pci_ops *ops, void *sd, int bus)
{
struct pci_bus *b;

@@ -464,10 +464,32 @@ static struct pci_bus * pci_alloc_bus(void)
INIT_LIST_HEAD(&b->resources);
b->max_bus_speed = PCI_SPEED_UNKNOWN;
b->cur_bus_speed = PCI_SPEED_UNKNOWN;
+ b->sysdata = sd;
+ b->ops = ops;
+ b->number = bus;
+ b->busn_res.start = bus;
+ b->busn_res.end = 0xff;
+ b->busn_res.flags = IORESOURCE_BUS;
+ b->dev.class = &pcibus_class;
+ dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
+ device_initialize(&b->dev);
}
+
return b;
}

+static void pci_release_host_bridge_dev(struct device *dev)
+{
+ struct pci_host_bridge *bridge = to_pci_host_bridge(dev);
+
+ if (bridge->release_fn)
+ bridge->release_fn(bridge);
+
+ pci_free_resource_list(&bridge->windows);
+
+ kfree(bridge);
+}
+
static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
{
struct pci_host_bridge *bridge;
@@ -476,6 +498,10 @@ static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
if (bridge) {
INIT_LIST_HEAD(&bridge->windows);
bridge->bus = b;
+ bridge->dev.release = pci_release_host_bridge_dev;
+ dev_set_name(&bridge->dev, "pci%04x:%02x",
+ pci_domain_nr(b), b->number);
+ device_initialize(&bridge->dev);
}

return bridge;
@@ -628,28 +654,13 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
/*
* Allocate a new bus, and inherit stuff from the parent..
*/
- child = pci_alloc_bus();
+ child = pci_alloc_bus(parent->ops, parent->sysdata, busnr);
if (!child)
return NULL;

child->parent = parent;
- child->ops = parent->ops;
- child->sysdata = parent->sysdata;
child->bus_flags = parent->bus_flags;
-
- /* initialize some portions of the bus device, but don't register it
- * now as the parent is not properly set up yet.
- */
- child->dev.class = &pcibus_class;
- dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
-
- /*
- * Set up the primary, secondary and subordinate
- * bus numbers.
- */
- child->number = child->busn_res.start = busnr;
child->primary = parent->busn_res.start;
- child->busn_res.end = 0xff;

if (!bridge) {
child->dev.parent = parent->bridge;
@@ -670,7 +681,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
bridge->subordinate = child;

add_dev:
- ret = device_register(&child->dev);
+ ret = device_add(&child->dev);
WARN_ON(ret < 0);

pcibios_add_bus(child);
@@ -1189,18 +1200,6 @@ int pci_cfg_space_size(struct pci_dev *dev)
return PCI_CFG_SPACE_SIZE;
}

-static void pci_release_bus_bridge_dev(struct device *dev)
-{
- struct pci_host_bridge *bridge = to_pci_host_bridge(dev);
-
- if (bridge->release_fn)
- bridge->release_fn(bridge);
-
- pci_free_resource_list(&bridge->windows);
-
- kfree(bridge);
-}
-
struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
{
struct pci_dev *dev;
@@ -1688,13 +1687,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
char bus_addr[64];
char *fmt;

- b = pci_alloc_bus();
+ b = pci_alloc_bus(ops, sysdata, bus);
if (!b)
return NULL;

- b->sysdata = sysdata;
- b->ops = ops;
- b->number = b->busn_res.start = bus;
b2 = pci_find_bus(pci_domain_nr(b), bus);
if (b2) {
/* If we already got to this bus through a different bridge, ignore it */
@@ -1706,27 +1702,22 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
if (!bridge)
goto err_out;

- bridge->dev.parent = parent;
- bridge->dev.release = pci_release_bus_bridge_dev;
- dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);
error = pcibios_root_bridge_prepare(bridge);
if (error)
goto bridge_dev_reg_err;

- error = device_register(&bridge->dev);
+ bridge->dev.parent = parent;
+ error = device_add(&bridge->dev);
if (error)
goto bridge_dev_reg_err;
+
b->bridge = get_device(&bridge->dev);
device_enable_async_suspend(b->bridge);
pci_set_bus_of_node(b);
-
if (!parent)
set_dev_node(b->bridge, pcibus_to_node(b));
-
- b->dev.class = &pcibus_class;
b->dev.parent = b->bridge;
- dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
- error = device_register(&b->dev);
+ error = device_add(&b->dev);
if (error)
goto class_dev_reg_err;

@@ -1769,12 +1760,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
return b;

class_dev_reg_err:
- put_device(&bridge->dev);
- device_unregister(&bridge->dev);
+ device_del(&bridge->dev);
bridge_dev_reg_err:
- kfree(bridge);
+ put_device(&bridge->dev);
err_out:
- kfree(b);
+ put_device(&b->dev);
return NULL;
}

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 8fc54b7..b0ce875 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -52,7 +52,8 @@ void pci_remove_bus(struct pci_bus *bus)
up_write(&pci_bus_sem);
pci_remove_legacy_files(bus);
pcibios_remove_bus(bus);
- device_unregister(&bus->dev);
+ device_del(&bus->dev);
+ put_device(&bus->dev);
}
EXPORT_SYMBOL(pci_remove_bus);

--
1.8.1.2

2013-05-28 02:00:28

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH v3, part1 09/10] PCI, IOV: simplify IOV implementation

On Sat, May 25, 2013 at 6:48 AM, Jiang Liu <[email protected]> wrote:
> Trivial changes to IOV:
> 1) use new PCI interfaces to simplify IOV implementation
> 2) fix some reference count related race windows
>
> Signed-off-by: Jiang Liu <[email protected]>
> Cc: Donald Dutile <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Cc: Ram Pai <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/pci/iov.c | 58 +++++++++++++++++++++----------------------------------
> 1 file changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 8f1e117..3e33499 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -51,43 +51,30 @@ static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
> return child;
> }
>
> -static void virtfn_remove_bus(struct pci_bus *bus, int busnr)
> +static void virtfn_remove_bus(struct pci_bus *physbus, struct pci_bus *virtbus)
> {
> - struct pci_bus *child;
> -
> - if (bus->number == busnr)
> - return;
> -
> - child = pci_find_bus(pci_domain_nr(bus), busnr);
> - BUG_ON(!child);
> -
> - if (list_empty(&child->devices))
> - pci_remove_bus(child);
> + if (physbus != virtbus && list_empty(&virtbus->devices))
> + pci_remove_bus(virtbus);
> }
>
> static int virtfn_add(struct pci_dev *dev, int id, int reset)
> {
> int i;
> - int rc;
> + int rc = -ENOMEM;
> u64 size;
> char buf[VIRTFN_ID_LEN];
> - struct pci_dev *virtfn;
> + struct pci_dev *virtfn = NULL;
> struct resource *res;
> struct pci_sriov *iov = dev->sriov;
> struct pci_bus *bus;
>
> - virtfn = pci_alloc_dev(NULL);
> - if (!virtfn)
> - return -ENOMEM;
> -
> mutex_lock(&iov->dev->sriov->lock);
> bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id));
> - if (!bus) {
> - kfree(virtfn);
> - mutex_unlock(&iov->dev->sriov->lock);
> - return -ENOMEM;
> - }
> - virtfn->bus = pci_bus_get(bus);
> + if (bus)
> + virtfn = pci_alloc_dev(bus);
> + if (!virtfn)
> + goto failed0;
> +
> virtfn->devfn = virtfn_devfn(dev, id);
> virtfn->vendor = dev->vendor;
> pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
> @@ -136,7 +123,8 @@ failed1:
> pci_dev_put(dev);
> mutex_lock(&iov->dev->sriov->lock);
> pci_stop_and_remove_bus_device(virtfn);
> - virtfn_remove_bus(dev->bus, virtfn_bus(dev, id));
> + virtfn_remove_bus(dev->bus, bus);
> +failed0:

need to swap above two lines. otherwise virtual bus could not be removed.

> mutex_unlock(&iov->dev->sriov->lock);
>
> return rc;
> @@ -145,20 +133,15 @@ failed1:
> static void virtfn_remove(struct pci_dev *dev, int id, int reset)
> {
> char buf[VIRTFN_ID_LEN];
> - struct pci_bus *bus;
> struct pci_dev *virtfn;
> struct pci_sriov *iov = dev->sriov;
>
> - bus = pci_find_bus(pci_domain_nr(dev->bus), virtfn_bus(dev, id));
> - if (!bus)
> - return;
> -
> - virtfn = pci_get_slot(bus, virtfn_devfn(dev, id));
> + virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
> + virtfn_bus(dev, id),
> + virtfn_devfn(dev, id));
> if (!virtfn)
> return;
>
> - pci_dev_put(virtfn);
> -
> if (reset) {
> device_release_driver(&virtfn->dev);
> __pci_reset_function(virtfn);
> @@ -176,9 +159,11 @@ static void virtfn_remove(struct pci_dev *dev, int id, int reset)
>
> mutex_lock(&iov->dev->sriov->lock);
> pci_stop_and_remove_bus_device(virtfn);
> - virtfn_remove_bus(dev->bus, virtfn_bus(dev, id));
> + virtfn_remove_bus(dev->bus, virtfn->bus);
> mutex_unlock(&iov->dev->sriov->lock);
>
> + /* balance pci_get_domain_bus_and_slot() */
> + pci_dev_put(virtfn);
> pci_dev_put(dev);
> }
>
> @@ -335,13 +320,14 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> if (!pdev)
> return -ENODEV;
>
> - pci_dev_put(pdev);
> -
> - if (!pdev->is_physfn)
> + if (!pdev->is_physfn) {
> + pci_dev_put(pdev);
> return -ENODEV;
> + }
>
> rc = sysfs_create_link(&dev->dev.kobj,
> &pdev->dev.kobj, "dep_link");
> + pci_dev_put(pdev);
> if (rc)
> return rc;
> }
> --
> 1.8.1.2
>

2013-05-28 14:49:06

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH v3, part1 09/10] PCI, IOV: simplify IOV implementation

On Tue 28 May 2013 10:00:26 AM CST, Yinghai Lu wrote:
> On Sat, May 25, 2013 at 6:48 AM, Jiang Liu <[email protected]> wrote:
>> Trivial changes to IOV:
....
>> @@ -136,7 +123,8 @@ failed1:
>> pci_dev_put(dev);
>> mutex_lock(&iov->dev->sriov->lock);
>> pci_stop_and_remove_bus_device(virtfn);
>> - virtfn_remove_bus(dev->bus, virtfn_bus(dev, id));
>> + virtfn_remove_bus(dev->bus, bus);
>> +failed0:
>
> need to swap above two lines. otherwise virtual bus could not be removed.
Hi Yinghai,
Good catch, will merge following patch in next version.

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 3e33499..bf0a32e 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -123,8 +123,9 @@ failed1:
pci_dev_put(dev);
mutex_lock(&iov->dev->sriov->lock);
pci_stop_and_remove_bus_device(virtfn);
- virtfn_remove_bus(dev->bus, bus);
failed0:
+ if (bus)
+ virtfn_remove_bus(dev->bus, bus);
mutex_unlock(&iov->dev->sriov->lock);

return rc;

2013-05-31 20:12:43

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

On 5/25/2013 9:48 AM, Jiang Liu wrote:
> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
> comments.
>
> Signed-off-by: Jiang Liu <[email protected]>
> Cc: Chris Metcalf <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: [email protected]
> Cc: [email protected]

Acked-by: Chris Metcalf <[email protected]> [for tile]

Boy, that's one stale comment - it was put in internally in our tree back in 2008 when we were still on 2.6.18.

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

2013-06-05 20:05:01

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

[+cc Konrad, Jeremy, xen-devel (users of interface you're deprecating)]

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
> comments.
>
> Signed-off-by: Jiang Liu <[email protected]>
> Cc: Chris Metcalf <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/tile/kernel/pci.c | 3 ---
> include/linux/pci.h | 4 ++--
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 67237d3..936e087 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -309,9 +309,6 @@ int __init pcibios_init(void)
> *
> * It reads the PCI tree for this bus into the Linux
> * data structures.
> - *
> - * This is inlined in linux/pci.h and calls into
> - * pci_scan_bus_parented() in probe.c.
> */
> pci_add_resource(&resources, &ioport_resource);
> pci_add_resource(&resources, &iomem_resource);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index b0f4a82..7b23fa0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -720,8 +720,8 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
> void pcibios_scan_specific_bus(int busn);
> struct pci_bus *pci_find_bus(int domain, int busnr);
> void pci_bus_add_devices(const struct pci_bus *bus);
> -struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
> - struct pci_ops *ops, void *sysdata);
> +struct pci_bus * __deprecated pci_scan_bus_parented(struct device *parent,
> + int bus, struct pci_ops *ops, void *sysdata);
> struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
> struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> struct pci_ops *ops, void *sysdata,

I think this patch is a good idea, but I think we need to change the
only existing user of pci_scan_bus_parented() (pcifront_scan_root() in
drivers/pci/xen-pcifront.c) at the same time, so xen doesn't start
getting build warnings. The deprecation warnings are really intended
for out-of-tree users that we aren't able to fix ourselves.

I'm getting this series queued up in
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3,
so if you want to send a patch just for xen, I can fold that in.

Bjorn

2013-06-05 20:07:42

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3, part1 03/10] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
> From: Gu Zheng <[email protected]>
>
> Use the new pci_alloc_dev(bus) to replace the existing using of
> alloc_pci_dev(void).
> ...

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 26df9c8..d5d18a0 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1130,6 +1130,7 @@ static void pci_release_dev(struct device *dev)
> struct pci_dev *pci_dev;
>
> pci_dev = to_pci_dev(dev);
> + pci_bus_put(pci_dev->bus);
> pci_release_capabilities(pci_dev);
> pci_release_of_node(pci_dev);
> kfree(pci_dev);

I think we should drop the pci_bus reference *last* (before the
kfree). Otherwise, we have to audit pci_release_capabilities() and
pci_release_of_node() to make sure they don't use pci_dev->bus.

I made this change on my branch already; just let me know if you object.

Bjorn

2013-06-05 20:15:01

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3, part1 06/10] PCI: make PCI host bridge/bus creating and destroying logic symmetric

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
> This patch makes PCI host bridge/bus creating and destroying logic
> symmetric by using device_initialize()/device_add()/device_del()/put_device()
> pairs as discussed in thread at:
> http://comments.gmane.org/gmane.linux.kernel.pci/22073
>
> It also fixes a bug in error recovery path in pci_create_root_bus()
> which may kfree() an in-use host bridge object.

Can you split out the bug fix into a separate patch, in case it's
something we want to backport by itself? I'd do it myself, but I
didn't understand at a glance what the problem was.

Bjorn

2013-06-05 21:31:33

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3, part1 07/10] PCI, unicore, m68k: remove redundant call of pci_bus_add_devices()

[+cc m68k and unicore32 maintainers]

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
> pci_scan_bus() and pci_scan_root_bus() has called pci_bus_add_devices()
> once, so remove the redundant call of pci_bus_add_devices().
> On the other handle, subsys_init() callbacks will be invoked before
> device_init() callbacks, so it should be safe to remove the redundant
> calls.
>
> Signed-off-by: Jiang Liu <[email protected]>
> ---
> arch/m68k/platform/coldfire/pci.c | 2 +-
> arch/unicore32/kernel/pci.c | 5 -----
> 2 files changed, 1 insertion(+), 6 deletions(-)

m68k and unicore32 guys:

I plan to apply the patch below. I actually split it into two, one
for m68k and another for unicore32. If you object or would rather
push it through your trees, let me know. I don't think the rest of
the series has any actual dependency on this, so there shouldn't be
any problem if you want to take it yourselves.

Bjorn

> diff --git a/arch/m68k/platform/coldfire/pci.c b/arch/m68k/platform/coldfire/pci.c
> index 8572246..2345972 100644
> --- a/arch/m68k/platform/coldfire/pci.c
> +++ b/arch/m68k/platform/coldfire/pci.c
> @@ -320,7 +320,7 @@ static int __init mcf_pci_init(void)
> pci_bus_size_bridges(rootbus);
> pci_bus_assign_resources(rootbus);
> pci_enable_bridges(rootbus);
> - pci_bus_add_devices(rootbus);
> +
> return 0;
> }
>
> diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
> index ef69c0c..374a055 100644
> --- a/arch/unicore32/kernel/pci.c
> +++ b/arch/unicore32/kernel/pci.c
> @@ -277,11 +277,6 @@ static int __init pci_common_init(void)
> pci_bus_assign_resources(puv3_bus);
> }
>
> - /*
> - * Tell drivers about devices found.
> - */
> - pci_bus_add_devices(puv3_bus);
> -
> return 0;
> }
> subsys_initcall(pci_common_init);
> --
> 1.8.1.2
>

2013-06-06 07:02:39

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH v3, part1 07/10] PCI, unicore, m68k: remove redundant call of pci_bus_add_devices()

Hi Bjorn,

On 06/06/13 07:31, Bjorn Helgaas wrote:
> [+cc m68k and unicore32 maintainers]
>
> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
>> pci_scan_bus() and pci_scan_root_bus() has called pci_bus_add_devices()
>> once, so remove the redundant call of pci_bus_add_devices().
>> On the other handle, subsys_init() callbacks will be invoked before
>> device_init() callbacks, so it should be safe to remove the redundant
>> calls.
>>
>> Signed-off-by: Jiang Liu <[email protected]>
>> ---
>> arch/m68k/platform/coldfire/pci.c | 2 +-
>> arch/unicore32/kernel/pci.c | 5 -----
>> 2 files changed, 1 insertion(+), 6 deletions(-)
>
> m68k and unicore32 guys:
>
> I plan to apply the patch below. I actually split it into two, one
> for m68k and another for unicore32. If you object or would rather
> push it through your trees, let me know. I don't think the rest of
> the series has any actual dependency on this, so there shouldn't be
> any problem if you want to take it yourselves.

I have no objection for m68k, feel free to push it through your tree.
I can't test the change in a board that has a PCI card in it right at
the moment, but otherwise:

Acked-by: Greg Ungerer <[email protected]>

Regards
Greg


> Bjorn
>
>> diff --git a/arch/m68k/platform/coldfire/pci.c b/arch/m68k/platform/coldfire/pci.c
>> index 8572246..2345972 100644
>> --- a/arch/m68k/platform/coldfire/pci.c
>> +++ b/arch/m68k/platform/coldfire/pci.c
>> @@ -320,7 +320,7 @@ static int __init mcf_pci_init(void)
>> pci_bus_size_bridges(rootbus);
>> pci_bus_assign_resources(rootbus);
>> pci_enable_bridges(rootbus);
>> - pci_bus_add_devices(rootbus);
>> +
>> return 0;
>> }
>>
>> diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
>> index ef69c0c..374a055 100644
>> --- a/arch/unicore32/kernel/pci.c
>> +++ b/arch/unicore32/kernel/pci.c
>> @@ -277,11 +277,6 @@ static int __init pci_common_init(void)
>> pci_bus_assign_resources(puv3_bus);
>> }
>>
>> - /*
>> - * Tell drivers about devices found.
>> - */
>> - pci_bus_add_devices(puv3_bus);
>> -
>> return 0;
>> }
>> subsys_initcall(pci_common_init);
>> --
>> 1.8.1.2
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2013-06-06 16:16:54

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH v3, part1 03/10] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

On Thu 06 Jun 2013 04:07:18 AM CST, Bjorn Helgaas wrote:
> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
>> From: Gu Zheng <[email protected]>
>>
>> Use the new pci_alloc_dev(bus) to replace the existing using of
>> alloc_pci_dev(void).
>> ...
>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 26df9c8..d5d18a0 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1130,6 +1130,7 @@ static void pci_release_dev(struct device *dev)
>> struct pci_dev *pci_dev;
>>
>> pci_dev = to_pci_dev(dev);
>> + pci_bus_put(pci_dev->bus);
>> pci_release_capabilities(pci_dev);
>> pci_release_of_node(pci_dev);
>> kfree(pci_dev);
>
> I think we should drop the pci_bus reference *last* (before the
> kfree). Otherwise, we have to audit pci_release_capabilities() and
> pci_release_of_node() to make sure they don't use pci_dev->bus.
>
> I made this change on my branch already; just let me know if you object.
Hi Bjorn,
You are right, thanks for fixing it.

>
> Bjorn

2013-06-06 16:32:39

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

On Thu 06 Jun 2013 04:04:36 AM CST, Bjorn Helgaas wrote:
> [+cc Konrad, Jeremy, xen-devel (users of interface you're deprecating)]
>
> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>> comments.
>>
>> Signed-off-by: Jiang Liu <[email protected]>
>> Cc: Chris Metcalf <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Thierry Reding <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> arch/tile/kernel/pci.c | 3 ---
>> include/linux/pci.h | 4 ++--
>> 2 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>> index 67237d3..936e087 100644
>> --- a/arch/tile/kernel/pci.c
>> +++ b/arch/tile/kernel/pci.c
>> @@ -309,9 +309,6 @@ int __init pcibios_init(void)
>> *
>> * It reads the PCI tree for this bus into the Linux
>> * data structures.
>> - *
>> - * This is inlined in linux/pci.h and calls into
>> - * pci_scan_bus_parented() in probe.c.
>> */
>> pci_add_resource(&resources, &ioport_resource);
>> pci_add_resource(&resources, &iomem_resource);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index b0f4a82..7b23fa0 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -720,8 +720,8 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>> void pcibios_scan_specific_bus(int busn);
>> struct pci_bus *pci_find_bus(int domain, int busnr);
>> void pci_bus_add_devices(const struct pci_bus *bus);
>> -struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
>> - struct pci_ops *ops, void *sysdata);
>> +struct pci_bus * __deprecated pci_scan_bus_parented(struct device *parent,
>> + int bus, struct pci_ops *ops, void *sysdata);
>> struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
>> struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>> struct pci_ops *ops, void *sysdata,
>
> I think this patch is a good idea, but I think we need to change the
> only existing user of pci_scan_bus_parented() (pcifront_scan_root() in
> drivers/pci/xen-pcifront.c) at the same time, so xen doesn't start
> getting build warnings. The deprecation warnings are really intended
> for out-of-tree users that we aren't able to fix ourselves.
>
> I'm getting this series queued up in
> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3,
> so if you want to send a patch just for xen, I can fold that in.
>
> Bjorn
Hi Bjorn,
I have posted a big patch for that, but still need help from
Xen experts to review it.
Please refer to https://patchwork.kernel.org/patch/2578551/
Thanks!
Gerry

2013-06-07 14:38:47

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

On Fri, Jun 07, 2013 at 12:32:28AM +0800, Jiang Liu wrote:
> On Thu 06 Jun 2013 04:04:36 AM CST, Bjorn Helgaas wrote:
> > [+cc Konrad, Jeremy, xen-devel (users of interface you're deprecating)]
> >
> > On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
> >> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
> >> comments.
> >>
> >> Signed-off-by: Jiang Liu <[email protected]>
> >> Cc: Chris Metcalf <[email protected]>
> >> Cc: Greg Kroah-Hartman <[email protected]>
> >> Cc: Thierry Reding <[email protected]>
> >> Cc: [email protected]
> >> Cc: [email protected]
> >> ---
> >> arch/tile/kernel/pci.c | 3 ---
> >> include/linux/pci.h | 4 ++--
> >> 2 files changed, 2 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> >> index 67237d3..936e087 100644
> >> --- a/arch/tile/kernel/pci.c
> >> +++ b/arch/tile/kernel/pci.c
> >> @@ -309,9 +309,6 @@ int __init pcibios_init(void)
> >> *
> >> * It reads the PCI tree for this bus into the Linux
> >> * data structures.
> >> - *
> >> - * This is inlined in linux/pci.h and calls into
> >> - * pci_scan_bus_parented() in probe.c.
> >> */
> >> pci_add_resource(&resources, &ioport_resource);
> >> pci_add_resource(&resources, &iomem_resource);
> >> diff --git a/include/linux/pci.h b/include/linux/pci.h
> >> index b0f4a82..7b23fa0 100644
> >> --- a/include/linux/pci.h
> >> +++ b/include/linux/pci.h
> >> @@ -720,8 +720,8 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
> >> void pcibios_scan_specific_bus(int busn);
> >> struct pci_bus *pci_find_bus(int domain, int busnr);
> >> void pci_bus_add_devices(const struct pci_bus *bus);
> >> -struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
> >> - struct pci_ops *ops, void *sysdata);
> >> +struct pci_bus * __deprecated pci_scan_bus_parented(struct device *parent,
> >> + int bus, struct pci_ops *ops, void *sysdata);
> >> struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
> >> struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> >> struct pci_ops *ops, void *sysdata,
> >
> > I think this patch is a good idea, but I think we need to change the
> > only existing user of pci_scan_bus_parented() (pcifront_scan_root() in
> > drivers/pci/xen-pcifront.c) at the same time, so xen doesn't start
> > getting build warnings. The deprecation warnings are really intended
> > for out-of-tree users that we aren't able to fix ourselves.
> >
> > I'm getting this series queued up in
> > http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3,
> > so if you want to send a patch just for xen, I can fold that in.
> >
> > Bjorn
> Hi Bjorn,
> I have posted a big patch for that, but still need help from
> Xen experts to review it.
> Please refer to https://patchwork.kernel.org/patch/2578551/

Hm, I seem to get:

/home/konrad/linux/drivers/pci/xen-pcifront.c: In function ‘pcifront_free_roots’:
/home/konrad/linux/drivers/pci/xen-pcifront.c:559: error: implicit declaration of function ‘for_each_pci_root_bus’
/home/konrad/linux/drivers/pci/xen-pcifront.c:559: error: expected ‘;’ before ‘{’ token
/home/konrad/linux/drivers/pci/xen-pcifront.c:554: warning: unused variable ‘sd’

with it? Is there an up-to-date patch?
I have these in my tree:

311db40 PCI, xen-pcifront: use new PCI interfaces to simplify implementation
4079fee Merge remote-tracking branch 'bjorn/pci/jiang-bus-lock-v3' into testing
ddb7c6b PCI: Hide remove and rescan sysfs interfaces for SR-IOV virtual functions
f5cfa3a PCI: Simplify IOV implementation and fix reference count races
28b4f07 PCI: Drop redundant setting of bus->is_added in virtfn_add_bus()
442655a unicore32/PCI: Remove redundant call of pci_bus_add_devices()
defd601 m68k/PCI: Remove redundant call of pci_bus_add_devices()
340e3fb PCI: Make PCI host bridge/bus creating and destroying logic symmetric
c7025f7 ia64/PCI: Clean up pci_scan_root_bus() usage
c1f41ce PCI: Mark pci_scan_bus_parented() as __deprecated
13e5057 PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
007042a PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
fe830ef PCI: Introduce pci_bus_{get|put}() to manage PCI bus reference count

Also oddly enough your SoB does not match your Author. Here is
what git shows:

=== > Author: Jiang Liu <[email protected]> 2013-05-25 09:48:38

=== > Signed-off-by: Jiang Liu <[email protected]>

You can add in the gmail an new 'email alias' so that the emails will
look as they come from your huawei address.

2013-06-07 15:11:32

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

On 06/07/2013 10:37 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 07, 2013 at 12:32:28AM +0800, Jiang Liu wrote:
>> On Thu 06 Jun 2013 04:04:36 AM CST, Bjorn Helgaas wrote:
>>> [+cc Konrad, Jeremy, xen-devel (users of interface you're deprecating)]
>>>
>>> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
>>>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>>>> comments.
>>>>
>>>> Signed-off-by: Jiang Liu <[email protected]>
>>>> Cc: Chris Metcalf <[email protected]>
>>>> Cc: Greg Kroah-Hartman <[email protected]>
>>>> Cc: Thierry Reding <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> ---
>>>> arch/tile/kernel/pci.c | 3 ---
>>>> include/linux/pci.h | 4 ++--
>>>> 2 files changed, 2 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>>>> index 67237d3..936e087 100644
>>>> --- a/arch/tile/kernel/pci.c
>>>> +++ b/arch/tile/kernel/pci.c
>>>> @@ -309,9 +309,6 @@ int __init pcibios_init(void)
>>>> *
>>>> * It reads the PCI tree for this bus into the Linux
>>>> * data structures.
>>>> - *
>>>> - * This is inlined in linux/pci.h and calls into
>>>> - * pci_scan_bus_parented() in probe.c.
>>>> */
>>>> pci_add_resource(&resources, &ioport_resource);
>>>> pci_add_resource(&resources, &iomem_resource);
>>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>>> index b0f4a82..7b23fa0 100644
>>>> --- a/include/linux/pci.h
>>>> +++ b/include/linux/pci.h
>>>> @@ -720,8 +720,8 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>>>> void pcibios_scan_specific_bus(int busn);
>>>> struct pci_bus *pci_find_bus(int domain, int busnr);
>>>> void pci_bus_add_devices(const struct pci_bus *bus);
>>>> -struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
>>>> - struct pci_ops *ops, void *sysdata);
>>>> +struct pci_bus * __deprecated pci_scan_bus_parented(struct device *parent,
>>>> + int bus, struct pci_ops *ops, void *sysdata);
>>>> struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
>>>> struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>>>> struct pci_ops *ops, void *sysdata,
>>>
>>> I think this patch is a good idea, but I think we need to change the
>>> only existing user of pci_scan_bus_parented() (pcifront_scan_root() in
>>> drivers/pci/xen-pcifront.c) at the same time, so xen doesn't start
>>> getting build warnings. The deprecation warnings are really intended
>>> for out-of-tree users that we aren't able to fix ourselves.
>>>
>>> I'm getting this series queued up in
>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3,
>>> so if you want to send a patch just for xen, I can fold that in.
>>>
>>> Bjorn
>> Hi Bjorn,
>> I have posted a big patch for that, but still need help from
>> Xen experts to review it.
>> Please refer to https://patchwork.kernel.org/patch/2578551/
>
> Hm, I seem to get:
>
> /home/konrad/linux/drivers/pci/xen-pcifront.c: In function ‘pcifront_free_roots’:
> /home/konrad/linux/drivers/pci/xen-pcifront.c:559: error: implicit declaration of function ‘for_each_pci_root_bus’
> /home/konrad/linux/drivers/pci/xen-pcifront.c:559: error: expected ‘;’ before ‘{’ token
> /home/konrad/linux/drivers/pci/xen-pcifront.c:554: warning: unused variable ‘sd’
>
> with it? Is there an up-to-date patch?
> I have these in my tree:
>
> 311db40 PCI, xen-pcifront: use new PCI interfaces to simplify implementation
> 4079fee Merge remote-tracking branch 'bjorn/pci/jiang-bus-lock-v3' into testing
> ddb7c6b PCI: Hide remove and rescan sysfs interfaces for SR-IOV virtual functions
> f5cfa3a PCI: Simplify IOV implementation and fix reference count races
> 28b4f07 PCI: Drop redundant setting of bus->is_added in virtfn_add_bus()
> 442655a unicore32/PCI: Remove redundant call of pci_bus_add_devices()
> defd601 m68k/PCI: Remove redundant call of pci_bus_add_devices()
> 340e3fb PCI: Make PCI host bridge/bus creating and destroying logic symmetric
> c7025f7 ia64/PCI: Clean up pci_scan_root_bus() usage
> c1f41ce PCI: Mark pci_scan_bus_parented() as __deprecated
> 13e5057 PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
> 007042a PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
> fe830ef PCI: Introduce pci_bus_{get|put}() to manage PCI bus reference count
Hi Konrad,
Thanks for review!

The patch "PCI, xen-pcifront: use new PCI interfaces to simplify
implementation" has dependency on part2 of the patch series, which
introduces for_each_pci_root_bus(). I have prepared a git tree for
you at:
git://github.com/jiangliu/linux.git pci_lock_v3

And I will try to split "PCI, xen-pcifront: use new PCI interfaces
to simplify implementation" into two patches, one is to kill
pci_scan_bus_parented(), the other is to use new for_each_pci_root_bus()
interface. But it should have the same effect with current version.

>
> Also oddly enough your SoB does not match your Author. Here is
> what git shows:
>
> === > Author: Jiang Liu <[email protected]> 2013-05-25 09:48:38
>
> === > Signed-off-by: Jiang Liu <[email protected]>
>
> You can add in the gmail an new 'email alias' so that the emails will
> look as they come from your huawei address.

Great thanks!
This issue has puzzled me for a long time! Will try to add an gmail
alias.

Regards!
Gerry

2013-06-07 15:30:27

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

On 06/07/2013 10:37 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 07, 2013 at 12:32:28AM +0800, Jiang Liu wrote:
>> On Thu 06 Jun 2013 04:04:36 AM CST, Bjorn Helgaas wrote:
>>> [+cc Konrad, Jeremy, xen-devel (users of interface you're deprecating)]
>>>
>>> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
>>>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>>>> comments.
>>>>
>>>> Signed-off-by: Jiang Liu <[email protected]>
>>>> Cc: Chris Metcalf <[email protected]>
>>>> Cc: Greg Kroah-Hartman <[email protected]>
>>>> Cc: Thierry Reding <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> ---
>>>> arch/tile/kernel/pci.c | 3 ---
>>>> include/linux/pci.h | 4 ++--
>>>> 2 files changed, 2 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
>>>> index 67237d3..936e087 100644
>>>> --- a/arch/tile/kernel/pci.c
>>>> +++ b/arch/tile/kernel/pci.c
>>>> @@ -309,9 +309,6 @@ int __init pcibios_init(void)
>>>> *
>>>> * It reads the PCI tree for this bus into the Linux
>>>> * data structures.
>>>> - *
>>>> - * This is inlined in linux/pci.h and calls into
>>>> - * pci_scan_bus_parented() in probe.c.
>>>> */
>>>> pci_add_resource(&resources, &ioport_resource);
>>>> pci_add_resource(&resources, &iomem_resource);
>>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>>> index b0f4a82..7b23fa0 100644
>>>> --- a/include/linux/pci.h
>>>> +++ b/include/linux/pci.h
>>>> @@ -720,8 +720,8 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>>>> void pcibios_scan_specific_bus(int busn);
>>>> struct pci_bus *pci_find_bus(int domain, int busnr);
>>>> void pci_bus_add_devices(const struct pci_bus *bus);
>>>> -struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
>>>> - struct pci_ops *ops, void *sysdata);
>>>> +struct pci_bus * __deprecated pci_scan_bus_parented(struct device *parent,
>>>> + int bus, struct pci_ops *ops, void *sysdata);
>>>> struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
>>>> struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>>>> struct pci_ops *ops, void *sysdata,
>>>
>>> I think this patch is a good idea, but I think we need to change the
>>> only existing user of pci_scan_bus_parented() (pcifront_scan_root() in
>>> drivers/pci/xen-pcifront.c) at the same time, so xen doesn't start
>>> getting build warnings. The deprecation warnings are really intended
>>> for out-of-tree users that we aren't able to fix ourselves.
>>>
>>> I'm getting this series queued up in
>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3,
>>> so if you want to send a patch just for xen, I can fold that in.
>>>
>>> Bjorn
>> Hi Bjorn,
>> I have posted a big patch for that, but still need help from
>> Xen experts to review it.
>> Please refer to https://patchwork.kernel.org/patch/2578551/
>
> Hm, I seem to get:
>
> /home/konrad/linux/drivers/pci/xen-pcifront.c: In function ‘pcifront_free_roots’:
> /home/konrad/linux/drivers/pci/xen-pcifront.c:559: error: implicit declaration of function ‘for_each_pci_root_bus’
> /home/konrad/linux/drivers/pci/xen-pcifront.c:559: error: expected ‘;’ before ‘{’ token
> /home/konrad/linux/drivers/pci/xen-pcifront.c:554: warning: unused variable ‘sd’
>
> with it? Is there an up-to-date patch?
> I have these in my tree:
>
> 311db40 PCI, xen-pcifront: use new PCI interfaces to simplify implementation
> 4079fee Merge remote-tracking branch 'bjorn/pci/jiang-bus-lock-v3' into testing
> ddb7c6b PCI: Hide remove and rescan sysfs interfaces for SR-IOV virtual functions
> f5cfa3a PCI: Simplify IOV implementation and fix reference count races
> 28b4f07 PCI: Drop redundant setting of bus->is_added in virtfn_add_bus()
> 442655a unicore32/PCI: Remove redundant call of pci_bus_add_devices()
> defd601 m68k/PCI: Remove redundant call of pci_bus_add_devices()
> 340e3fb PCI: Make PCI host bridge/bus creating and destroying logic symmetric
> c7025f7 ia64/PCI: Clean up pci_scan_root_bus() usage
> c1f41ce PCI: Mark pci_scan_bus_parented() as __deprecated
> 13e5057 PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
> 007042a PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
> fe830ef PCI: Introduce pci_bus_{get|put}() to manage PCI bus reference count
Hi Konrad,
Could you please help to apply this simple patch onto to your tree?
It should fix the build failure issue.

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 79ad229..b5e0e66 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -556,7 +556,7 @@ static void pcifront_free_roots(struct
pcifront_device *pdev)

dev_dbg(&pdev->xdev->dev, "cleaning up root buses\n");

- for_each_pci_root_bus(bus) {
+ list_for_each_entry(bus, &pci_root_buses, node) {
sd = bus->sysdata;
if (sd->pdev == pdev) {
pci_stop_root_bus(bus);



>
> Also oddly enough your SoB does not match your Author. Here is
> what git shows:
>
> === > Author: Jiang Liu <[email protected]> 2013-05-25 09:48:38
>
> === > Signed-off-by: Jiang Liu <[email protected]>
>
> You can add in the gmail an new 'email alias' so that the emails will
> look as they come from your huawei address.
>

2013-06-07 20:37:38

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3, part1 04/10] PCI: mark pci_scan_bus_parented() as __deprecated

On Fri, Jun 7, 2013 at 9:30 AM, Jiang Liu <[email protected]> wrote:
> On 06/07/2013 10:37 PM, Konrad Rzeszutek Wilk wrote:
>> On Fri, Jun 07, 2013 at 12:32:28AM +0800, Jiang Liu wrote:
>>> On Thu 06 Jun 2013 04:04:36 AM CST, Bjorn Helgaas wrote:
>>>> [+cc Konrad, Jeremy, xen-devel (users of interface you're deprecating)]
>>>>
>>>> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <[email protected]> wrote:
>>>>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>>>>> comments.

I dropped this patch (marking pci_scan_bus_parented() as __deprecated)
from the series for now. When the xen-pcifront stuff gets resolved,
we can deprecate the interface at the same time.

Bjorn