[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

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

[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

[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

[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

[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 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

[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

[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

[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

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] PCI, IOV: fix a resource leakage on error recovery path

[Jiang Liu]

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
Hi Bjorn,
	Could you please help to apply this small patch onto the
jiang-bus-lock-v3, which addresses a memory leakage issue pointed
out by Yinghai? Or you may fold it into " PCI, IOV: simplify IOV 
implementation".
Thanks!
Gerry
---
 drivers/pci/iov.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 5eb8165..458fb39 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -122,8 +122,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;
-- 
1.8.1.2

Re: [PATCH] PCI, IOV: fix a resource leakage on error recovery path

[Bjorn Helgaas]

On Thu, May 30, 2013 at 12:31:33PM +0800, Jiang Liu wrote:
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
> Hi Bjorn,
> 	Could you please help to apply this small patch onto the
> jiang-bus-lock-v3, which addresses a memory leakage issue pointed
> out by Yinghai? Or you may fold it into " PCI, IOV: simplify IOV 
> implementation".
> Thanks!
> Gerry
> ---
>  drivers/pci/iov.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 5eb8165..458fb39 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -122,8 +122,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;

I fixed this slightly differently to keep the error checking
more linear.  I think it's equivalent to what you did; let me
know if otherwise.


commit f5cfa3a1ccef2d0e9ca862c609cf185f87049177
Author: Jiang Liu <liuj97@gmail.com>
Date:   Sat May 25 21:48:37 2013 +0800

    PCI: Simplify IOV implementation and fix reference count races
    
    Trivial changes to IOV:
    
      1) use new PCI interfaces to simplify IOV implementation
      2) fix some reference count related race windows
    
    [bhelgaas: fix virtfn_add() add bus/alloc dev error paths]
    Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Cc: Donald Dutile <ddutile@redhat.com>
    Cc: Yinghai Lu <yinghai@kernel.org>
    Cc: Ram Pai <linuxram@us.ibm.com>

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 8f1e117..5fffca9 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -51,24 +51,16 @@ 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;
@@ -76,18 +68,15 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
 	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)
+		goto failed;
+
+	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 +125,9 @@ 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));
+failed0:
+	virtfn_remove_bus(dev->bus, bus);
+failed:
 	mutex_unlock(&iov->dev->sriov->lock);
 
 	return rc;
@@ -145,20 +136,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 +162,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 +323,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;
 	}

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 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

[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

[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 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

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()

[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()

[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
> 

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
> 

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

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

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

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

[PATCH 1/2] PCI: fix a double free issue in pci_create_root_bus() error recovery path

[Jiang Liu]

On pci_create_root_bus() error recovery path, device_unregister(&bridge->dev)
should have freed memory used by bridge, so we shouldn't call kfree(bridge)
again, it's a double free.

On the other hand, we should not use kfree() to free memory used by
device object once we have invoked device_register() because it's
reference-counted.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: stable@vger.kernel.org
---
Hi Bjorn,
	This is the patch to fix the kfree() issue, it may be a material
for stable trees.
Thanks!
Gerry
---
 drivers/pci/probe.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8882b5d..2f81a0a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1729,12 +1729,16 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	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;
+	if (error) {
+		kfree(bridge);
+		goto err_out;
+	}
 
 	error = device_register(&bridge->dev);
-	if (error)
-		goto bridge_dev_reg_err;
+	if (error) {
+		kfree(bridge);
+		goto err_out;
+	}
 	b->bridge = get_device(&bridge->dev);
 	device_enable_async_suspend(b->bridge);
 	pci_set_bus_of_node(b);
@@ -1790,8 +1794,6 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 class_dev_reg_err:
 	put_device(&bridge->dev);
 	device_unregister(&bridge->dev);
-bridge_dev_reg_err:
-	kfree(bridge);
 err_out:
 	kfree(b);
 	return NULL;
-- 
1.8.1.2

[PATCH 2/2] 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  | 92 +++++++++++++++++++++++-----------------------------
 drivers/pci/remove.c |  3 +-
 2 files changed, 42 insertions(+), 53 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2f81a0a..9e9cdfe 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -470,7 +470,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;
 
@@ -483,10 +483,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;
@@ -495,6 +517,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;
@@ -647,28 +673,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;
@@ -689,7 +700,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);
@@ -1208,18 +1219,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;
@@ -1707,13 +1706,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 */
@@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 		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) {
-		kfree(bridge);
-		goto err_out;
-	}
+	if (error)
+		goto bridge_dev_reg_err;
+
+	error = device_add(&bridge->dev);
+	if (error)
+		goto bridge_dev_reg_err;
 
-	error = device_register(&bridge->dev);
-	if (error) {
-		kfree(bridge);
-		goto err_out;
-	}
 	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;
 
@@ -1792,10 +1779,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	return b;
 
 class_dev_reg_err:
+	device_del(&bridge->dev);
+bridge_dev_reg_err:
 	put_device(&bridge->dev);
-	device_unregister(&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 2/2] PCI: make PCI host bridge/bus creating and destroying logic symmetric

[Yinghai Lu]

On Thu, Jun 6, 2013 at 10:10 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.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c  | 92 +++++++++++++++++++++++-----------------------------
>  drivers/pci/remove.c |  3 +-
>  2 files changed, 42 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 2f81a0a..9e9cdfe 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -470,7 +470,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;
>
> @@ -483,10 +483,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);
> +}
> +

should split pci_release_host_bridge_dev renaming and moving to another patch.

>  static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
>  {
>         struct pci_host_bridge *bridge;
> @@ -495,6 +517,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;
> @@ -647,28 +673,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;
> @@ -689,7 +700,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);
> @@ -1208,18 +1219,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;
> @@ -1707,13 +1706,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 */
> @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>                 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) {
> -               kfree(bridge);
> -               goto err_out;
> -       }
> +       if (error)
> +               goto bridge_dev_reg_err;
> +
> +       error = device_add(&bridge->dev);
> +       if (error)
> +               goto bridge_dev_reg_err;
>
> -       error = device_register(&bridge->dev);
> -       if (error) {
> -               kfree(bridge);
> -               goto err_out;
> -       }
>         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;
>
> @@ -1792,10 +1779,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>         return b;
>
>  class_dev_reg_err:
> +       device_del(&bridge->dev);
> +bridge_dev_reg_err:
>         put_device(&bridge->dev);
> -       device_unregister(&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);
>

looks like we don't need to split device_register to device_del and put_device.

we can make root bus removal to use device_unregister too, in the patches,
  https://patchwork.kernel.org/patch/2562431/
    [1/7] PCI: move back pci_proc_attach_devices calling
  https://patchwork.kernel.org/patch/2562461/
    [2/7] PCI: move resources and bus_list releasing to pci_release_dev
  https://patchwork.kernel.org/patch/2562451/
    [3/7] PCI: Detach driver in pci_stop_device

Yinghai

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

Re: [PATCH 1/2] PCI: fix a double free issue in pci_create_root_bus() error recovery path

[Bjorn Helgaas]

On Fri, Jun 07, 2013 at 01:10:08AM +0800, Jiang Liu wrote:
> On pci_create_root_bus() error recovery path, device_unregister(&bridge->dev)
> should have freed memory used by bridge, so we shouldn't call kfree(bridge)
> again, it's a double free.
> 
> On the other hand, we should not use kfree() to free memory used by
> device object once we have invoked device_register() because it's
> reference-counted.
> 
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: stable@vger.kernel.org
> ---
> Hi Bjorn,
> 	This is the patch to fix the kfree() issue, it may be a material
> for stable trees.
> Thanks!
> Gerry
> ---
>  drivers/pci/probe.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8882b5d..2f81a0a 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1729,12 +1729,16 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	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;
> +	if (error) {
> +		kfree(bridge);
> +		goto err_out;
> +	}
>  
>  	error = device_register(&bridge->dev);
> -	if (error)
> -		goto bridge_dev_reg_err;
> +	if (error) {
> +		kfree(bridge);

Per device_register() comment, this should be a put_device().
I added this patch with that change to my pci/jiang-bus-lock-v3 branch.

I know a subsequent patch removes this anyway.  I might be a little
obsessive.

> +		goto err_out;
> +	}
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> @@ -1790,8 +1794,6 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  class_dev_reg_err:
>  	put_device(&bridge->dev);
>  	device_unregister(&bridge->dev);
> -bridge_dev_reg_err:
> -	kfree(bridge);
>  err_out:
>  	kfree(b);
>  	return NULL;
> -- 
> 1.8.1.2
> 

Re: [PATCH 2/2] PCI: make PCI host bridge/bus creating and destroying logic symmetric

[Bjorn Helgaas]

On Thu, Jun 06, 2013 at 01:01:23PM -0700, Yinghai Lu wrote:
> On Thu, Jun 6, 2013 at 10:10 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.
> >
> > Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> > Cc: Yinghai Lu <yinghai@kernel.org>
> > ---
> >  drivers/pci/probe.c  | 92 +++++++++++++++++++++++-----------------------------
> >  drivers/pci/remove.c |  3 +-
> >  2 files changed, 42 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 2f81a0a..9e9cdfe 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > ...
> > +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);
> > +}
> > +
> 
> should split pci_release_host_bridge_dev renaming and moving to another patch.

I split the rename/move to another patch and added both to my
pci/jiang-bus-lock-v3 branch.

> > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> >                 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) {
> > -               kfree(bridge);
> > -               goto err_out;
> > -       }
> > +       if (error)
> > +               goto bridge_dev_reg_err;
> > +
> > +       error = device_add(&bridge->dev);
> > +       if (error)
> > +               goto bridge_dev_reg_err;
> >
> > -       error = device_register(&bridge->dev);
> > -       if (error) {
> > -               kfree(bridge);
> > -               goto err_out;
> > -       }

I think this function would be better if we created the host bridge
first, then the bus.  But your patch is an improvement, so we can look
at doing that later.

> looks like we don't need to split device_register to device_del and put_device.
> 
> we can make root bus removal to use device_unregister too, in the patches,

If we can use device_register()/device_unregister() directly instead of
splitting them into initialize/add/del/put, that would be awesome.

Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3
is the current state of what I've got.  If you want to iterate on this some
more and improve things, that's great.  But please start with what's on my
branch and post your updates as a complete v4 because I did tweak some
changelogs and code formatting, and I don't want to lose that work.

Bjorn

Re: [PATCH 2/2] PCI: make PCI host bridge/bus creating and destroying logic symmetric

[Yinghai Lu]

On Fri, Jun 7, 2013 at 6:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>> should split pci_release_host_bridge_dev renaming and moving to another patch.
>
> I split the rename/move to another patch and added both to my
> pci/jiang-bus-lock-v3 branch.

good.

>
>> > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct ..
...
>
> I think this function would be better if we created the host bridge
> first, then the bus.  But your patch is an improvement, so we can look
> at doing that later.

Yes, that will more straightforward, create hostbidge before root bus,
like we have pci bridge before subordinate bus under it.

current logic is

        b = pci_alloc_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 */
                dev_dbg(&b2->dev, "bus already known\n");
                goto err_out;
        }

        bridge = pci_alloc_host_bridge(b);

We need to find the duplicated hostbridge or racing.

If we need call pci_alloc_host_bridge at first, we will need my
for_each_host_bridge patchset, and use it to check if there is
existing hostbridge.

>
>> looks like we don't need to split device_register to device_del and put_device.
>>
>> we can make root bus removal to use device_unregister too, in the patches,
>
> If we can use device_register()/device_unregister() directly instead of
> splitting them into initialize/add/del/put, that would be awesome.
>
> Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3
> is the current state of what I've got.  If you want to iterate on this some
> more and improve things, that's great.  But please start with what's on my
> branch and post your updates as a complete v4 because I did tweak some
> changelogs and code formatting, and I don't want to lose that work.

ok, please consider drop
commit d751727a1792262cb464d8d9608500facb763301
    PCI: Make host bridge/bus creating and destroying logic symmetric
from pci/jiang-bus-lock-v3, and apply my three patches at

  https://patchwork.kernel.org/patch/2562431/
    [1/7] PCI: move back pci_proc_attach_devices calling
  https://patchwork.kernel.org/patch/2562461/
    [2/7] PCI: move resources and bus_list releasing to pci_release_dev
  https://patchwork.kernel.org/patch/2562451/
    [3/7] PCI: Detach driver in pci_stop_device

Thanks

Yinghai

Re: [PATCH 2/2] PCI: make PCI host bridge/bus creating and destroying logic symmetric

[Bjorn Helgaas]

On Sat, Jun 8, 2013 at 1:01 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Jun 7, 2013 at 6:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>
>>> should split pci_release_host_bridge_dev renaming and moving to another patch.
>>
>> I split the rename/move to another patch and added both to my
>> pci/jiang-bus-lock-v3 branch.
>
> good.
>
>>
>>> > @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct ..
> ...
>>
>> I think this function would be better if we created the host bridge
>> first, then the bus.  But your patch is an improvement, so we can look
>> at doing that later.
>
> Yes, that will more straightforward, create hostbidge before root bus,
> like we have pci bridge before subordinate bus under it.
>
> current logic is
>
>         b = pci_alloc_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 */
>                 dev_dbg(&b2->dev, "bus already known\n");
>                 goto err_out;
>         }
>
>         bridge = pci_alloc_host_bridge(b);
>
> We need to find the duplicated hostbridge or racing.
>
> If we need call pci_alloc_host_bridge at first, we will need my
> for_each_host_bridge patchset, and use it to check if there is
> existing hostbridge.
>
>>
>>> looks like we don't need to split device_register to device_del and put_device.
>>>
>>> we can make root bus removal to use device_unregister too, in the patches,
>>
>> If we can use device_register()/device_unregister() directly instead of
>> splitting them into initialize/add/del/put, that would be awesome.
>>
>> Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3
>> is the current state of what I've got.  If you want to iterate on this some
>> more and improve things, that's great.  But please start with what's on my
>> branch and post your updates as a complete v4 because I did tweak some
>> changelogs and code formatting, and I don't want to lose that work.
>
> ok, please consider drop
> commit d751727a1792262cb464d8d9608500facb763301
>     PCI: Make host bridge/bus creating and destroying logic symmetric
> from pci/jiang-bus-lock-v3, and apply my three patches at
>
>   https://patchwork.kernel.org/patch/2562431/
>     [1/7] PCI: move back pci_proc_attach_devices calling
>   https://patchwork.kernel.org/patch/2562461/
>     [2/7] PCI: move resources and bus_list releasing to pci_release_dev
>   https://patchwork.kernel.org/patch/2562451/
>     [3/7] PCI: Detach driver in pci_stop_device

I dropped the "Make host bridge/bus creating and destroying logic
symmetric" patch for now.  I don't think the subsequent patches depend
on that one, so I merged the rest to my -next branch for v3.11, so we
can make at least a tiny bit of progress in this cycle.

Bjorn

Re: [PATCH 2/2] PCI: make PCI host bridge/bus creating and destroying logic symmetric

[Jiang Liu]

On Sat 15 Jun 2013 07:53:48 AM CST, Bjorn Helgaas wrote:
> On Sat, Jun 8, 2013 at 1:01 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Jun 7, 2013 at 6:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>>
>>>> should split pci_release_host_bridge_dev renaming and moving to another patch.
>>>
>>> I split the rename/move to another patch and added both to my
>>> pci/jiang-bus-lock-v3 branch.
>>
>> good.
>>
>>>
>>>>> @@ -1726,30 +1722,21 @@ struct pci_bus *pci_create_root_bus(struct ..
>> ...
>>>
>>> I think this function would be better if we created the host bridge
>>> first, then the bus.  But your patch is an improvement, so we can look
>>> at doing that later.
>>
>> Yes, that will more straightforward, create hostbidge before root bus,
>> like we have pci bridge before subordinate bus under it.
>>
>> current logic is
>>
>>         b = pci_alloc_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 */
>>                 dev_dbg(&b2->dev, "bus already known\n");
>>                 goto err_out;
>>         }
>>
>>         bridge = pci_alloc_host_bridge(b);
>>
>> We need to find the duplicated hostbridge or racing.
>>
>> If we need call pci_alloc_host_bridge at first, we will need my
>> for_each_host_bridge patchset, and use it to check if there is
>> existing hostbridge.
>>
>>>
>>>> looks like we don't need to split device_register to device_del and put_device.
>>>>
>>>> we can make root bus removal to use device_unregister too, in the patches,
>>>
>>> If we can use device_register()/device_unregister() directly instead of
>>> splitting them into initialize/add/del/put, that would be awesome.
>>>
>>> Anyway, http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/jiang-bus-lock-v3
>>> is the current state of what I've got.  If you want to iterate on this some
>>> more and improve things, that's great.  But please start with what's on my
>>> branch and post your updates as a complete v4 because I did tweak some
>>> changelogs and code formatting, and I don't want to lose that work.
>>
>> ok, please consider drop
>> commit d751727a1792262cb464d8d9608500facb763301
>>     PCI: Make host bridge/bus creating and destroying logic symmetric
>> from pci/jiang-bus-lock-v3, and apply my three patches at
>>
>>   https://patchwork.kernel.org/patch/2562431/
>>     [1/7] PCI: move back pci_proc_attach_devices calling
>>   https://patchwork.kernel.org/patch/2562461/
>>     [2/7] PCI: move resources and bus_list releasing to pci_release_dev
>>   https://patchwork.kernel.org/patch/2562451/
>>     [3/7] PCI: Detach driver in pci_stop_device
>
> I dropped the "Make host bridge/bus creating and destroying logic
> symmetric" patch for now.  I don't think the subsequent patches depend
> on that one, so I merged the rest to my -next branch for v3.11, so we
> can make at least a tiny bit of progress in this cycle.
>
> Bjorn
Hi Bjorn,
      Sorry for the delay, busy with the dock station regression and 
bugfix these
days, will try to find time slot this weekend to handle this.
Regards!
Gerry