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

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

[Jiang Liu]

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

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

[Jiang Liu]

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

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Jiang Liu]

From: Gu Zheng <guz.fnst@cn.fujitsu.com>

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 <jiang.liu@huawei.com>
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 <guz.fnst@cn.fujitsu.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Jiang Liu]

From: Gu Zheng <guz.fnst@cn.fujitsu.com>

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 <guz.fnst@cn.fujitsu.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Airlie <airlied@linux.ie>
Cc: Neela Syam Kolli <megaraidlinux@lsi.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
 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

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

[Bjorn Helgaas]

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <liuj97@gmail.com> wrote:
> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>
> 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

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

[Jiang Liu]

On Thu 06 Jun 2013 04:07:18 AM CST, Bjorn Helgaas wrote:
> On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <liuj97@gmail.com> wrote:
>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>>
>> 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

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

[Jiang Liu]

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

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thierry Reding <thierry.reding@avionic-design.de>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
---
 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

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

[Chris Metcalf]

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 <jiang.liu@huawei.com>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org

Acked-by: Chris Metcalf <cmetcalf@tilera.com> [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

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

[Bjorn Helgaas]

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

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <liuj97@gmail.com> wrote:
> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
> comments.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> ---
>  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

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

[Jiang Liu]

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 <liuj97@gmail.com> wrote:
>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>> comments.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> Cc: Chris Metcalf <cmetcalf@tilera.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Thierry Reding <thierry.reding@avionic-design.de>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-pci@vger.kernel.org
>> ---
>>  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

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

[Konrad Rzeszutek Wilk]

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 <liuj97@gmail.com> wrote:
> >> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
> >> comments.
> >>
> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> >> Cc: Chris Metcalf <cmetcalf@tilera.com>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-pci@vger.kernel.org
> >> ---
> >>  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 <liuj97@gmail.com>  2013-05-25 09:48:38
    
=== > Signed-off-by: Jiang Liu <jiang.liu@huawei.com>

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

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

[Jiang Liu]

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 <liuj97@gmail.com> wrote:
>>>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>>>> comments.
>>>>
>>>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>>>> Cc: Chris Metcalf <cmetcalf@tilera.com>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: Thierry Reding <thierry.reding@avionic-design.de>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: linux-pci@vger.kernel.org
>>>> ---
>>>>  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 <liuj97@gmail.com>  2013-05-25 09:48:38
>     
> === > Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> 
> 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

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

[Jiang Liu]

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 <liuj97@gmail.com> wrote:
>>>> Mark pci_scan_bus_parented() as __deprecated and clean up outdated
>>>> comments.
>>>>
>>>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>>>> Cc: Chris Metcalf <cmetcalf@tilera.com>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: Thierry Reding <thierry.reding@avionic-design.de>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: linux-pci@vger.kernel.org
>>>> ---
>>>>  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 <liuj97@gmail.com>  2013-05-25 09:48:38
>     
> === > Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> 
> You can add in the gmail an new 'email alias' so that the emails will
> look as they come from your huawei address.
> 

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

[Bjorn Helgaas]

On Fri, Jun 7, 2013 at 9:30 AM, Jiang Liu <liuj97@gmail.com> 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 <liuj97@gmail.com> 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

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

[Jiang Liu]

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 <jiang.liu@huawei.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Jiang Liu]

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 <jiang.liu@huawei.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 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

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

[Bjorn Helgaas]

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <liuj97@gmail.com> 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

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

[Jiang Liu]

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 <jiang.liu@huawei.com>
---
 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

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

[Bjorn Helgaas]

[+cc m68k and unicore32 maintainers]

On Sat, May 25, 2013 at 7:48 AM, Jiang Liu <liuj97@gmail.com> 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 <jiang.liu@huawei.com>
> ---
>  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
>

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

[Greg Ungerer]

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 <liuj97@gmail.com> 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 <jiang.liu@huawei.com>
>> ---
>>  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 <gerg@uclinux.org>

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Jiang Liu]

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 <jiang.liu@huawei.com>
Cc: Donald Dutile <ddutile@redhat.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Ram Pai <linuxram@us.ibm.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Jiang Liu]

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 <jiang.liu@huawei.com>
Cc: Donald Dutile <ddutile@redhat.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Ram Pai <linuxram@us.ibm.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Yinghai Lu]

On Sat, May 25, 2013 at 6:48 AM, Jiang Liu <liuj97@gmail.com> 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 <jiang.liu@huawei.com>
> Cc: Donald Dutile <ddutile@redhat.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> Cc: Ram Pai <linuxram@us.ibm.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  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
>

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

[Liu Jiang]

On Tue 28 May 2013 10:00:26 AM CST, Yinghai Lu wrote:
> On Sat, May 25, 2013 at 6:48 AM, Jiang Liu <liuj97@gmail.com> 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;

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

[Jiang Liu]

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 <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Donald Dutile <ddutile@redhat.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Ram Pai <linuxram@us.ibm.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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