[PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Taku Izumi]

 Hi all,

 
 I'd like to merge hostbridge hotplug feature.
 I looked at Yinghai's branch and found that this branch contains 
 many work and can be split into some parts. 
 I believe it is good to merge step by step.

 My idea is splitting into the following 4 parts:
   1. basic hostbirdge hotplug work
   2. acpiphp work
   3. /sysfs interface work (logical hotplug?)
   4. cleanup

 This patchset is 1st step based on the following Yinghai's branch:

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-root-bus-hotplug

  * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
  * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
  * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
  * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
  * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
  * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
  * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

-- 
Best regards,
Taku Izumi <izumi.taku@jp.fujitsu.com>

[PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn

[Taku Izumi]

From: Yinghai Lu <yinghai@kernel.org>

    x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
    
    Non acpi path, or root is not probed from acpi, during root bus removal
    will get warning about leaking from pci_scan_bus_on_node.
    
    Fix it with setting pci_host_bridge release function.
    
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>

---
 arch/x86/pci/common.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Index: Bjorn-next-0808/arch/x86/pci/common.c
===================================================================
--- Bjorn-next-0808.orig/arch/x86/pci/common.c
+++ Bjorn-next-0808/arch/x86/pci/common.c
@@ -634,17 +634,19 @@ int pci_ext_cfg_avail(struct pci_dev *de
 		return 0;
 }
 
+static void release_pci_sysdata(struct pci_host_bridge *bridge)
+{
+	struct pci_sysdata *sd = bridge->release_data;
+
+	kfree(sd);
+}
+
 struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops, int node)
 {
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
 	struct pci_sysdata *sd;
 
-	/*
-	 * Allocate per-root-bus (not per bus) arch-specific data.
-	 * TODO: leak; this memory is never freed.
-	 * It's arguable whether it's worth the trouble to care.
-	 */
 	sd = kzalloc(sizeof(*sd), GFP_KERNEL);
 	if (!sd) {
 		printk(KERN_ERR "PCI: OOM, skipping PCI bus %02x\n", busno);
@@ -654,7 +656,10 @@ struct pci_bus * __devinit pci_scan_bus_
 	x86_pci_root_bus_resources(busno, &resources);
 	printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busno);
 	bus = pci_scan_root_bus(NULL, busno, ops, sd, &resources);
-	if (!bus) {
+	if (bus)
+		pci_set_host_bridge_release(to_pci_host_bridge(bus->bridge),
+					release_pci_sysdata, sd);
+	else {
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}

Re: [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn

[Bjorn Helgaas]

On Thu, Aug 9, 2012 at 11:11 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> From: Yinghai Lu <yinghai@kernel.org>
>
>     x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>
>     Non acpi path, or root is not probed from acpi, during root bus removal
>     will get warning about leaking from pci_scan_bus_on_node.
>
>     Fix it with setting pci_host_bridge release function.
>
>     Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>     Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>
> ---
>  arch/x86/pci/common.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> Index: Bjorn-next-0808/arch/x86/pci/common.c
> ===================================================================
> --- Bjorn-next-0808.orig/arch/x86/pci/common.c
> +++ Bjorn-next-0808/arch/x86/pci/common.c
> @@ -634,17 +634,19 @@ int pci_ext_cfg_avail(struct pci_dev *de
>                 return 0;
>  }
>
> +static void release_pci_sysdata(struct pci_host_bridge *bridge)
> +{
> +       struct pci_sysdata *sd = bridge->release_data;
> +
> +       kfree(sd);
> +}
> +
>  struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops, int node)
>  {
>         LIST_HEAD(resources);
>         struct pci_bus *bus = NULL;
>         struct pci_sysdata *sd;
>
> -       /*
> -        * Allocate per-root-bus (not per bus) arch-specific data.
> -        * TODO: leak; this memory is never freed.
> -        * It's arguable whether it's worth the trouble to care.
> -        */
>         sd = kzalloc(sizeof(*sd), GFP_KERNEL);
>         if (!sd) {
>                 printk(KERN_ERR "PCI: OOM, skipping PCI bus %02x\n", busno);
> @@ -654,7 +656,10 @@ struct pci_bus * __devinit pci_scan_bus_
>         x86_pci_root_bus_resources(busno, &resources);
>         printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busno);
>         bus = pci_scan_root_bus(NULL, busno, ops, sd, &resources);
> -       if (!bus) {
> +       if (bus)
> +               pci_set_host_bridge_release(to_pci_host_bridge(bus->bridge),
> +                                       release_pci_sysdata, sd);
> +       else {
>                 pci_free_resource_list(&resources);
>                 kfree(sd);
>         }
>

The patch itself is OK, but as far as I know, there's no mechanism for
removing a non-ACPI host bridge, so I don't think the release function
will ever be used, will it?

[PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()

[Taku Izumi]

From: Jiang Liu <jiang.liu@huawei.com>

    Correctly clean up pci root buses in function pci_remove_bus()
    
    The function pci_create_root_bus() allocates the pci bus structure,
    registers the bus device and creates the legacy files for a pci root
    bus, but returns without setting the is_added flag. The is_added flag
    for a pci root bus will be set by function pci_scan_child_bus().
    If a pci root bus is destroyed before calling pci_scan_child_bus(),
    the is_added flag will not be set.  So teach function pci_remove_bus()
    to detect such a case and correctly clean up pci root buses.
    
    Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/pci/remove.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: Bjorn-next-0808/drivers/pci/remove.c
===================================================================
--- Bjorn-next-0808.orig/drivers/pci/remove.c
+++ Bjorn-next-0808/drivers/pci/remove.c
@@ -70,11 +70,10 @@ void pci_remove_bus(struct pci_bus *pci_
 	list_del(&pci_bus->node);
 	pci_bus_release_busn_res(pci_bus);
 	up_write(&pci_bus_sem);
-	if (!pci_bus->is_added)
-		return;
-
-	pci_remove_legacy_files(pci_bus);
-	device_unregister(&pci_bus->dev);
+	if (pci_bus->is_added || pci_is_root_bus(pci_bus)) {
+		pci_remove_legacy_files(pci_bus);
+		device_unregister(&pci_bus->dev);
+	}
 }
 EXPORT_SYMBOL(pci_remove_bus);
 

Re: [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()

[Bjorn Helgaas]

On Thu, Aug 9, 2012 at 11:12 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> From: Jiang Liu <jiang.liu@huawei.com>
>
>     Correctly clean up pci root buses in function pci_remove_bus()
>
>     The function pci_create_root_bus() allocates the pci bus structure,
>     registers the bus device and creates the legacy files for a pci root
>     bus, but returns without setting the is_added flag. The is_added flag
>     for a pci root bus will be set by function pci_scan_child_bus().
>     If a pci root bus is destroyed before calling pci_scan_child_bus(),
>     the is_added flag will not be set.

What's the scenario where this happens?  Most of the
pci_scan_child_bus() calls look like this:

    bus = pci_create_root_bus();
    pci_scan_child_bus(bus);

In those cases, there's no opportunity to destroy the root bus before
calling pci_scan_child_bus().

I don't like to add additional tests as you're doing here because it's
telling us that we have more special cases to think about.  Apparently
bus->is_added is telling us that we've created legacy files and
registered the bus device.  EXCEPT for root buses.  Why is it
different for root buses?  Why don't we set root_bus->is_added at the
same time we create the legacy files and register the root bus's
device?

>     So teach function pci_remove_bus()
>     to detect such a case and correctly clean up pci root buses.
>
>     Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>     Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>     Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/pci/remove.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> Index: Bjorn-next-0808/drivers/pci/remove.c
> ===================================================================
> --- Bjorn-next-0808.orig/drivers/pci/remove.c
> +++ Bjorn-next-0808/drivers/pci/remove.c
> @@ -70,11 +70,10 @@ void pci_remove_bus(struct pci_bus *pci_
>         list_del(&pci_bus->node);
>         pci_bus_release_busn_res(pci_bus);
>         up_write(&pci_bus_sem);
> -       if (!pci_bus->is_added)
> -               return;
> -
> -       pci_remove_legacy_files(pci_bus);
> -       device_unregister(&pci_bus->dev);
> +       if (pci_bus->is_added || pci_is_root_bus(pci_bus)) {
> +               pci_remove_legacy_files(pci_bus);
> +               device_unregister(&pci_bus->dev);
> +       }
>  }
>  EXPORT_SYMBOL(pci_remove_bus);
>
>

Re: [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()

[Jiang Liu]

On 08/17/2012 01:11 AM, Bjorn Helgaas wrote:
> On Thu, Aug 9, 2012 at 11:12 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>> From: Jiang Liu <jiang.liu@huawei.com>
>>
>>     Correctly clean up pci root buses in function pci_remove_bus()
>>
>>     The function pci_create_root_bus() allocates the pci bus structure,
>>     registers the bus device and creates the legacy files for a pci root
>>     bus, but returns without setting the is_added flag. The is_added flag
>>     for a pci root bus will be set by function pci_scan_child_bus().
>>     If a pci root bus is destroyed before calling pci_scan_child_bus(),
>>     the is_added flag will not be set.
> 
> What's the scenario where this happens?  Most of the
> pci_scan_child_bus() calls look like this:
> 
>     bus = pci_create_root_bus();
>     pci_scan_child_bus(bus);
> 
> In those cases, there's no opportunity to destroy the root bus before
> calling pci_scan_child_bus().
> 
> I don't like to add additional tests as you're doing here because it's
> telling us that we have more special cases to think about.  Apparently
> bus->is_added is telling us that we've created legacy files and
> registered the bus device.  EXCEPT for root buses.  Why is it
> different for root buses?  Why don't we set root_bus->is_added at the
> same time we create the legacy files and register the root bus's
> device?
Hi Bjorn,
	Previously this patch was used to cover error recovery path in
our new ACPI based hotplug framework. Now thing has changed a lot and
it can't be cleanly applied to pci-next branch, so please ignore it:)
I will resubmit it if needed in future.
	--Gerry

> 
>>     So teach function pci_remove_bus()
>>     to detect such a case and correctly clean up pci root buses.
>>
>>     Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>>     Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>     Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> ---
>>  drivers/pci/remove.c |    9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> Index: Bjorn-next-0808/drivers/pci/remove.c
>> ===================================================================
>> --- Bjorn-next-0808.orig/drivers/pci/remove.c
>> +++ Bjorn-next-0808/drivers/pci/remove.c
>> @@ -70,11 +70,10 @@ void pci_remove_bus(struct pci_bus *pci_
>>         list_del(&pci_bus->node);
>>         pci_bus_release_busn_res(pci_bus);
>>         up_write(&pci_bus_sem);
>> -       if (!pci_bus->is_added)
>> -               return;
>> -
>> -       pci_remove_legacy_files(pci_bus);
>> -       device_unregister(&pci_bus->dev);
>> +       if (pci_bus->is_added || pci_is_root_bus(pci_bus)) {
>> +               pci_remove_legacy_files(pci_bus);
>> +               device_unregister(&pci_bus->dev);
>> +       }
>>  }
>>  EXPORT_SYMBOL(pci_remove_bus);
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver

[Taku Izumi]

From: Jiang Liu <jiang.liu@huawei.com>

    ACPI, PCI: Use normal list for struct acpi_pci_driver
    
    Use normal list for struct acpi_pci_driver to simplify code.
    
    Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>

---
 drivers/acpi/pci_root.c |   16 +++-------------
 include/linux/acpi.h    |    2 +-
 2 files changed, 4 insertions(+), 14 deletions(-)

Index: Bjorn-next-0808/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0808.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0808/drivers/acpi/pci_root.c
@@ -72,8 +72,8 @@ static struct acpi_driver acpi_pci_root_
 };
 
 static LIST_HEAD(acpi_pci_roots);
+static LIST_HEAD(acpi_pci_drivers);
 
-static struct acpi_pci_driver *sub_driver;
 static DEFINE_MUTEX(osc_lock);
 
 int acpi_pci_register_driver(struct acpi_pci_driver *driver)
@@ -81,10 +81,7 @@ int acpi_pci_register_driver(struct acpi
 	int n = 0;
 	struct acpi_pci_root *root;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr)
-		pptr = &(*pptr)->next;
-	*pptr = driver;
+	list_add_tail(&driver->node, &acpi_pci_drivers);
 
 	if (!driver->add)
 		return 0;
@@ -103,14 +100,7 @@ void acpi_pci_unregister_driver(struct a
 {
 	struct acpi_pci_root *root;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr) {
-		if (*pptr == driver)
-			break;
-		pptr = &(*pptr)->next;
-	}
-	BUG_ON(!*pptr);
-	*pptr = (*pptr)->next;
+	list_del(&driver->node);
 
 	if (!driver->remove)
 		return;
Index: Bjorn-next-0808/include/linux/acpi.h
===================================================================
--- Bjorn-next-0808.orig/include/linux/acpi.h
+++ Bjorn-next-0808/include/linux/acpi.h
@@ -138,7 +138,7 @@ void acpi_penalize_isa_irq(int irq, int 
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 struct acpi_pci_driver {
-	struct acpi_pci_driver *next;
+	struct list_head node;
 	int (*add)(acpi_handle handle);
 	void (*remove)(acpi_handle handle);
 };

[PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges

[Taku Izumi]

From: Jiang Liu <jiang.liu@huawei.com>

    ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
    
    When hot-plugging PCI root bridge, acpi_pci_drivers' add()/remove()
    methods should be invoked to notify registered drivers.
    
    -v2: Move add calling to acpi_pci_root_start by Yinghai
    
    Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>

---
 drivers/acpi/pci_root.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: Bjorn-next-0808/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0808.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0808/drivers/acpi/pci_root.c
@@ -626,14 +626,25 @@ end:
 static int acpi_pci_root_start(struct acpi_device *device)
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
+	struct acpi_pci_driver *driver;
+
+	list_for_each_entry(driver, &acpi_pci_drivers, node)
+		if (driver->add)
+			driver->add(device->handle);
 
 	pci_bus_add_devices(root->bus);
+
 	return 0;
 }
 
 static int acpi_pci_root_remove(struct acpi_device *device, int type)
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
+	struct acpi_pci_driver *driver;
+
+	list_for_each_entry(driver, &acpi_pci_drivers, node)
+		if (driver->remove)
+			driver->remove(root->device->handle);
 
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);

[PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c

[Taku Izumi]

From: Jiang Liu <jiang.liu@huawei.com>

    ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
    
    There are two global lists inf file drivers/acpi/pci_root.c.
    One is for registered acpi_pci_drivers, the other is for
    enumerated ACPI PCI root bridge objects. These two global
    lists may change dynamically when registering/deregistering
    acpi_pci_drivers or adding/removing ACPI PCI root bridge
    objects. So protect them by mutex lock and RCU list.
    
    Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    [izumi.taku@jp.fujitsu.com: a bit change at acpi_pci_root_remove()]
    Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>

---
 drivers/acpi/pci_root.c |   86 +++++++++++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 33 deletions(-)

Index: Bjorn-next/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next.orig/drivers/acpi/pci_root.c
+++ Bjorn-next/drivers/acpi/pci_root.c
@@ -27,7 +27,8 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/types.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
+#include <linux/rculist.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/pci.h>
@@ -71,6 +72,8 @@ static struct acpi_driver acpi_pci_root_
 		},
 };
 
+/* Lock to protect both acpi_pci_roots and acpi_pci_drivers lists */
+static DEFINE_MUTEX(acpi_pci_root_lock);
 static LIST_HEAD(acpi_pci_roots);
 static LIST_HEAD(acpi_pci_drivers);
 
@@ -81,47 +84,48 @@ int acpi_pci_register_driver(struct acpi
 	int n = 0;
 	struct acpi_pci_root *root;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_add_tail(&driver->node, &acpi_pci_drivers);
-
-	if (!driver->add)
-		return 0;
-
-	list_for_each_entry(root, &acpi_pci_roots, node) {
-		driver->add(root->device->handle);
-		n++;
-	}
+	if (driver->add)
+		list_for_each_entry_rcu(root, &acpi_pci_roots, node) {
+			driver->add(root->device->handle);
+			n++;
+		}
+	mutex_unlock(&acpi_pci_root_lock);
 
 	return n;
 }
-
 EXPORT_SYMBOL(acpi_pci_register_driver);
 
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver)
 {
 	struct acpi_pci_root *root;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_del(&driver->node);
-
-	if (!driver->remove)
-		return;
-
-	list_for_each_entry(root, &acpi_pci_roots, node)
-		driver->remove(root->device->handle);
+	if (driver->remove)
+		list_for_each_entry_rcu(root, &acpi_pci_roots, node)
+			driver->remove(root->device->handle);
+	mutex_unlock(&acpi_pci_root_lock);
 }
-
 EXPORT_SYMBOL(acpi_pci_unregister_driver);
 
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
 {
 	struct acpi_pci_root *root;
+	struct acpi_handle *handle = NULL;
 	
-	list_for_each_entry(root, &acpi_pci_roots, node)
+	rcu_read_lock();
+	list_for_each_entry_rcu(root, &acpi_pci_roots, node)
 		if ((root->segment == (u16) seg) &&
-		    (root->secondary.start == (u16) bus))
-			return root->device->handle;
-	return NULL;		
-}
+		    (root->secondary.start == (u16) bus)) {
+			handle = root->device->handle;
+			break;
+		}
+	rcu_read_unlock();
 
+	return handle;
+}
 EXPORT_SYMBOL_GPL(acpi_get_pci_rootbridge_handle);
 
 /**
@@ -268,10 +272,15 @@ struct acpi_pci_root *acpi_pci_find_root
 {
 	struct acpi_pci_root *root;
 
-	list_for_each_entry(root, &acpi_pci_roots, node) {
-		if (root->device->handle == handle)
+	rcu_read_lock();
+	list_for_each_entry_rcu(root, &acpi_pci_roots, node) {
+		if (root->device->handle == handle) {
+			rcu_read_unlock();
 			return root;
+		}
 	}
+	rcu_read_unlock();
+
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(acpi_pci_find_root);
@@ -459,7 +468,7 @@ static int __devinit acpi_pci_root_add(s
 	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
 		printk(KERN_ERR PREFIX "can't evaluate _SEG\n");
 		result = -ENODEV;
-		goto end;
+		goto out_free;
 	}
 
 	/* Check _CRS first, then _BBN.  If no _BBN, default to zero. */
@@ -484,7 +493,7 @@ static int __devinit acpi_pci_root_add(s
 		else {
 			printk(KERN_ERR PREFIX "can't evaluate _BBN\n");
 			result = -ENODEV;
-			goto end;
+			goto out_free;
 		}
 	}
 
@@ -508,8 +517,8 @@ static int __devinit acpi_pci_root_add(s
 	 * TBD: Need PCI interface for enumeration/configuration of roots.
 	 */
 
-	/* TBD: Locking */
-	list_add_tail(&root->node, &acpi_pci_roots);
+	mutex_lock(&acpi_pci_root_lock);
+	list_add_tail_rcu(&root->node, &acpi_pci_roots);
 
 	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
@@ -528,7 +537,7 @@ static int __devinit acpi_pci_root_add(s
 			    "Bus %04x:%02x not present in PCI namespace\n",
 			    root->segment, (unsigned int)root->secondary.start);
 		result = -ENODEV;
-		goto end;
+		goto out_del_root;
 	}
 
 	/*
@@ -538,7 +547,7 @@ static int __devinit acpi_pci_root_add(s
 	 */
 	result = acpi_pci_bind_root(device);
 	if (result)
-		goto end;
+		goto out_del_root;
 
 	/*
 	 * PCI Routing Table
@@ -614,11 +623,15 @@ static int __devinit acpi_pci_root_add(s
 	if (device->wakeup.flags.run_wake)
 		device_set_run_wake(root->bus->bridge, true);
 
+	mutex_unlock(&acpi_pci_root_lock);
+
 	return 0;
 
-end:
-	if (!list_empty(&root->node))
-		list_del(&root->node);
+out_del_root:
+	list_del_rcu(&root->node);
+	mutex_unlock(&acpi_pci_root_lock);
+	synchronize_rcu();
+out_free:
 	kfree(root);
 	return result;
 }
@@ -628,11 +641,13 @@ static int acpi_pci_root_start(struct ac
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
 			driver->add(device->handle);
 
 	pci_bus_add_devices(root->bus);
+	mutex_unlock(&acpi_pci_root_lock);
 
 	return 0;
 }
@@ -642,6 +657,8 @@ static int acpi_pci_root_remove(struct a
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	mutex_lock(&acpi_pci_root_lock);
+
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root->device->handle);
@@ -649,6 +666,9 @@ static int acpi_pci_root_remove(struct a
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
+	list_del_rcu(&root->node);
+	mutex_unlock(&acpi_pci_root_lock);
+	synchronize_rcu();
 	kfree(root);
 	return 0;
 }

Re: [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c

[Bjorn Helgaas]

On Thu, Aug 9, 2012 at 11:14 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> From: Jiang Liu <jiang.liu@huawei.com>
>
>     ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>
>     There are two global lists inf file drivers/acpi/pci_root.c.
>     One is for registered acpi_pci_drivers, the other is for
>     enumerated ACPI PCI root bridge objects. These two global
>     lists may change dynamically when registering/deregistering
>     acpi_pci_drivers or adding/removing ACPI PCI root bridge
>     objects. So protect them by mutex lock and RCU list.

Is it possible to split this into two patches, one to add
acpi_pci_drivers protection and another to add acpi_pci_roots
protection?  That would make this easier to review.

>     Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>     Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>     [izumi.taku@jp.fujitsu.com: a bit change at acpi_pci_root_remove()]
>     Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>
> ---
>  drivers/acpi/pci_root.c |   86 +++++++++++++++++++++++++++++-------------------
>  1 file changed, 53 insertions(+), 33 deletions(-)
>
> Index: Bjorn-next/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next/drivers/acpi/pci_root.c
> @@ -27,7 +27,8 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/types.h>
> -#include <linux/spinlock.h>
> +#include <linux/mutex.h>
> +#include <linux/rculist.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pci.h>
> @@ -71,6 +72,8 @@ static struct acpi_driver acpi_pci_root_
>                 },
>  };
>
> +/* Lock to protect both acpi_pci_roots and acpi_pci_drivers lists */
> +static DEFINE_MUTEX(acpi_pci_root_lock);
>  static LIST_HEAD(acpi_pci_roots);
>  static LIST_HEAD(acpi_pci_drivers);
>
> @@ -81,47 +84,48 @@ int acpi_pci_register_driver(struct acpi
>         int n = 0;
>         struct acpi_pci_root *root;
>
> +       mutex_lock(&acpi_pci_root_lock);
>         list_add_tail(&driver->node, &acpi_pci_drivers);
> -
> -       if (!driver->add)
> -               return 0;
> -
> -       list_for_each_entry(root, &acpi_pci_roots, node) {
> -               driver->add(root->device->handle);
> -               n++;
> -       }
> +       if (driver->add)
> +               list_for_each_entry_rcu(root, &acpi_pci_roots, node) {
> +                       driver->add(root->device->handle);
> +                       n++;
> +               }
> +       mutex_unlock(&acpi_pci_root_lock);
>
>         return n;
>  }
> -
>  EXPORT_SYMBOL(acpi_pci_register_driver);
>
>  void acpi_pci_unregister_driver(struct acpi_pci_driver *driver)
>  {
>         struct acpi_pci_root *root;
>
> +       mutex_lock(&acpi_pci_root_lock);
>         list_del(&driver->node);
> -
> -       if (!driver->remove)
> -               return;
> -
> -       list_for_each_entry(root, &acpi_pci_roots, node)
> -               driver->remove(root->device->handle);
> +       if (driver->remove)
> +               list_for_each_entry_rcu(root, &acpi_pci_roots, node)
> +                       driver->remove(root->device->handle);
> +       mutex_unlock(&acpi_pci_root_lock);
>  }
> -
>  EXPORT_SYMBOL(acpi_pci_unregister_driver);
>
>  acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
>  {
>         struct acpi_pci_root *root;
> +       struct acpi_handle *handle = NULL;
>
> -       list_for_each_entry(root, &acpi_pci_roots, node)
> +       rcu_read_lock();
> +       list_for_each_entry_rcu(root, &acpi_pci_roots, node)
>                 if ((root->segment == (u16) seg) &&
> -                   (root->secondary.start == (u16) bus))
> -                       return root->device->handle;
> -       return NULL;
> -}
> +                   (root->secondary.start == (u16) bus)) {
> +                       handle = root->device->handle;
> +                       break;
> +               }
> +       rcu_read_unlock();
>
> +       return handle;
> +}
>  EXPORT_SYMBOL_GPL(acpi_get_pci_rootbridge_handle);
>
>  /**
> @@ -268,10 +272,15 @@ struct acpi_pci_root *acpi_pci_find_root
>  {
>         struct acpi_pci_root *root;
>
> -       list_for_each_entry(root, &acpi_pci_roots, node) {
> -               if (root->device->handle == handle)
> +       rcu_read_lock();
> +       list_for_each_entry_rcu(root, &acpi_pci_roots, node) {
> +               if (root->device->handle == handle) {
> +                       rcu_read_unlock();
>                         return root;
> +               }
>         }
> +       rcu_read_unlock();
> +
>         return NULL;
>  }
>  EXPORT_SYMBOL_GPL(acpi_pci_find_root);
> @@ -459,7 +468,7 @@ static int __devinit acpi_pci_root_add(s
>         if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
>                 printk(KERN_ERR PREFIX "can't evaluate _SEG\n");
>                 result = -ENODEV;
> -               goto end;
> +               goto out_free;
>         }
>
>         /* Check _CRS first, then _BBN.  If no _BBN, default to zero. */
> @@ -484,7 +493,7 @@ static int __devinit acpi_pci_root_add(s
>                 else {
>                         printk(KERN_ERR PREFIX "can't evaluate _BBN\n");
>                         result = -ENODEV;
> -                       goto end;
> +                       goto out_free;
>                 }
>         }
>
> @@ -508,8 +517,8 @@ static int __devinit acpi_pci_root_add(s
>          * TBD: Need PCI interface for enumeration/configuration of roots.
>          */
>
> -       /* TBD: Locking */
> -       list_add_tail(&root->node, &acpi_pci_roots);
> +       mutex_lock(&acpi_pci_root_lock);
> +       list_add_tail_rcu(&root->node, &acpi_pci_roots);
>
>         printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
>                acpi_device_name(device), acpi_device_bid(device),
> @@ -528,7 +537,7 @@ static int __devinit acpi_pci_root_add(s
>                             "Bus %04x:%02x not present in PCI namespace\n",
>                             root->segment, (unsigned int)root->secondary.start);
>                 result = -ENODEV;
> -               goto end;
> +               goto out_del_root;
>         }
>
>         /*
> @@ -538,7 +547,7 @@ static int __devinit acpi_pci_root_add(s
>          */
>         result = acpi_pci_bind_root(device);
>         if (result)
> -               goto end;
> +               goto out_del_root;
>
>         /*
>          * PCI Routing Table
> @@ -614,11 +623,15 @@ static int __devinit acpi_pci_root_add(s
>         if (device->wakeup.flags.run_wake)
>                 device_set_run_wake(root->bus->bridge, true);
>
> +       mutex_unlock(&acpi_pci_root_lock);
> +
>         return 0;
>
> -end:
> -       if (!list_empty(&root->node))
> -               list_del(&root->node);
> +out_del_root:
> +       list_del_rcu(&root->node);
> +       mutex_unlock(&acpi_pci_root_lock);
> +       synchronize_rcu();
> +out_free:
>         kfree(root);
>         return result;
>  }
> @@ -628,11 +641,13 @@ static int acpi_pci_root_start(struct ac
>         struct acpi_pci_root *root = acpi_driver_data(device);
>         struct acpi_pci_driver *driver;
>
> +       mutex_lock(&acpi_pci_root_lock);
>         list_for_each_entry(driver, &acpi_pci_drivers, node)
>                 if (driver->add)
>                         driver->add(device->handle);
>
>         pci_bus_add_devices(root->bus);
> +       mutex_unlock(&acpi_pci_root_lock);
>
>         return 0;
>  }
> @@ -642,6 +657,8 @@ static int acpi_pci_root_remove(struct a
>         struct acpi_pci_root *root = acpi_driver_data(device);
>         struct acpi_pci_driver *driver;
>
> +       mutex_lock(&acpi_pci_root_lock);
> +
>         list_for_each_entry(driver, &acpi_pci_drivers, node)
>                 if (driver->remove)
>                         driver->remove(root->device->handle);
> @@ -649,6 +666,9 @@ static int acpi_pci_root_remove(struct a
>         device_set_run_wake(root->bus->bridge, false);
>         pci_acpi_remove_bus_pm_notifier(device);
>
> +       list_del_rcu(&root->node);
> +       mutex_unlock(&acpi_pci_root_lock);
> +       synchronize_rcu();
>         kfree(root);
>         return 0;
>  }
>

[PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function

[Taku Izumi]

Currently there's no PCI-related clean-up code
in acpi_pci_root_remove() function.
This patch introduces function for hostbridge removal.

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_bind.c     |    7 +++++++
 drivers/acpi/pci_root.c     |    8 ++++++++
 drivers/pci/remove.c        |   18 +++++++++++++++++-
 include/acpi/acpi_drivers.h |    1 +
 include/linux/pci.h         |    2 ++
 5 files changed, 35 insertions(+), 1 deletion(-)

Index: Bjorn-next-0718/drivers/pci/remove.c
===================================================================
--- Bjorn-next-0718.orig/drivers/pci/remove.c
+++ Bjorn-next-0718/drivers/pci/remove.c
@@ -143,7 +143,7 @@ void pci_stop_and_remove_behind_bridge(s
 	__pci_remove_behind_bridge(dev);
 }
 
-static void pci_stop_bus_devices(struct pci_bus *bus)
+void pci_stop_bus_devices(struct pci_bus *bus)
 {
 	struct list_head *l, *n;
 
@@ -179,4 +179,20 @@ void pci_stop_bus_device(struct pci_dev 
 
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
 EXPORT_SYMBOL(pci_stop_and_remove_behind_bridge);
+EXPORT_SYMBOL(pci_stop_bus_devices);
 EXPORT_SYMBOL_GPL(pci_stop_bus_device);
+
+void pci_remove_host_bridge(struct pci_host_bridge *bridge)
+{
+	struct list_head *l, *n;
+	struct pci_bus *root = bridge->bus;
+
+	list_for_each_safe(l, n, &root->devices)
+		__pci_remove_bus_device(pci_dev_b(l));
+
+	pci_remove_bus(root);
+
+	device_unregister(&bridge->dev);
+}
+EXPORT_SYMBOL(pci_remove_host_bridge);
+
Index: Bjorn-next-0718/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0718.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0718/drivers/acpi/pci_root.c
@@ -656,9 +656,12 @@ static int acpi_pci_root_remove(struct a
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
+	struct pci_host_bridge *bridge = to_pci_host_bridge(root->bus->bridge);
 
 	mutex_lock(&acpi_pci_root_lock);
 
+	pci_stop_bus_devices(root->bus);
+
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root->device->handle);
@@ -666,6 +669,11 @@ static int acpi_pci_root_remove(struct a
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
+	acpi_pci_irq_del_prt(root->bus);
+	acpi_pci_unbind_root(device);
+
+	pci_remove_host_bridge(bridge);
+
 	list_del_rcu(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
 	synchronize_rcu();
Index: Bjorn-next-0718/include/linux/pci.h
===================================================================
--- Bjorn-next-0718.orig/include/linux/pci.h
+++ Bjorn-next-0718/include/linux/pci.h
@@ -736,7 +736,9 @@ extern void pci_dev_put(struct pci_dev *
 extern void pci_remove_bus(struct pci_bus *b);
 extern void __pci_remove_bus_device(struct pci_dev *dev);
 extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
+extern void pci_stop_bus_devices(struct pci_bus *bus);
 extern void pci_stop_bus_device(struct pci_dev *dev);
+extern void pci_remove_host_bridge(struct pci_host_bridge *bridge);
 void pci_setup_cardbus(struct pci_bus *bus);
 extern void pci_sort_breadthfirst(void);
 #define dev_is_pci(d) ((d)->bus == &pci_bus_type)
Index: Bjorn-next-0718/drivers/acpi/pci_bind.c
===================================================================
--- Bjorn-next-0718.orig/drivers/acpi/pci_bind.c
+++ Bjorn-next-0718/drivers/acpi/pci_bind.c
@@ -118,3 +118,10 @@ int acpi_pci_bind_root(struct acpi_devic
 
 	return 0;
 }
+
+void  acpi_pci_unbind_root(struct acpi_device *device)
+{
+	device->ops.bind = NULL;
+	device->ops.unbind = NULL;
+}
+
Index: Bjorn-next-0718/include/acpi/acpi_drivers.h
===================================================================
--- Bjorn-next-0718.orig/include/acpi/acpi_drivers.h
+++ Bjorn-next-0718/include/acpi/acpi_drivers.h
@@ -101,6 +101,7 @@ struct pci_bus;
 
 struct pci_dev *acpi_get_pci_dev(acpi_handle);
 int acpi_pci_bind_root(struct acpi_device *device);
+void acpi_pci_unbind_root(struct acpi_device *device);
 
 /* Arch-defined function to add a bus to the system */
 

[PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

[Taku Izumi]

Devices under hot-added hostbridge have no chance to assign resources
and to configure them, so this patch adds such code for hot-added 
hostbridges at acpi_pci_root_start().

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   18 ++++++++++++++++++
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 19 insertions(+)

Index: Bjorn-next-0808/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0808.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0808/drivers/acpi/pci_root.c
@@ -39,6 +39,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/apei.h>
+#include <linux/pci_hotplug.h>
 
 #define PREFIX "ACPI: "
 
@@ -462,6 +463,10 @@ static int __devinit acpi_pci_root_add(s
 	if (!root)
 		return -ENOMEM;
 
+	if (system_state != SYSTEM_BOOTING) {
+		root->hot_added = true;
+	}
+
 	segment = 0;
 	status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
 				       &segment);
@@ -640,12 +645,25 @@ static int acpi_pci_root_start(struct ac
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
+	struct pci_dev *pdev;
 
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
 			driver->add(device->handle);
 
+	/*
+ 	 * Devices under hot-added hostbridge have no chance to assign
+ 	 * resources and to configure them, so do that here
+ 	 */
+	if (root->hot_added) {
+		pci_bus_size_bridges(root->bus);
+		pci_bus_assign_resources(root->bus);
+		list_for_each_entry(pdev, &root->bus->devices, bus_list)
+			pci_configure_slot(pdev);
+		pci_enable_bridges(root->bus);
+	}
+
 	pci_bus_add_devices(root->bus);
 	mutex_unlock(&acpi_pci_root_lock);
 
Index: Bjorn-next-0808/include/acpi/acpi_bus.h
===================================================================
--- Bjorn-next-0808.orig/include/acpi/acpi_bus.h
+++ Bjorn-next-0808/include/acpi/acpi_bus.h
@@ -407,6 +407,7 @@ struct acpi_pci_root {
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */
 	phys_addr_t mcfg_addr;
+	bool hot_added;
 };
 
 /* helper */

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Yinghai Lu]

On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>  Hi all,
>
>
>  I'd like to merge hostbridge hotplug feature.
>  I looked at Yinghai's branch and found that this branch contains
>  many work and can be split into some parts.
>  I believe it is good to merge step by step.
>
>  My idea is splitting into the following 4 parts:
>    1. basic hostbirdge hotplug work
>    2. acpiphp work
>    3. /sysfs interface work (logical hotplug?)
>    4. cleanup
>
>  This patchset is 1st step based on the following Yinghai's branch:

Hi,

Are you going to attend PCI mini-summit 8/28? We should discuss there.

Thanks

Yinghai

RE: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Taku Izumi]

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] On Behalf Of Yinghai Lu
> Sent: Saturday, August 11, 2012 1:41 AM
> To: Taku Izumi
> Cc: linux-pci@vger.kernel.org; bhelgaas@google.com; linux-acpi@vger.kernel.org; kaneshige.kenji@jp.fujitsu.com;
> jiang.liu@huawei.com
> Subject: Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support
> 
> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> >  Hi all,
> >
> >
> >  I'd like to merge hostbridge hotplug feature.
> >  I looked at Yinghai's branch and found that this branch contains
> >  many work and can be split into some parts.
> >  I believe it is good to merge step by step.
> >
> >  My idea is splitting into the following 4 parts:
> >    1. basic hostbirdge hotplug work
> >    2. acpiphp work
> >    3. /sysfs interface work (logical hotplug?)
> >    4. cleanup
> >
> >  This patchset is 1st step based on the following Yinghai's branch:
> 
> Hi,
> 
> Are you going to attend PCI mini-summit 8/28? We should discuss there.

  Sounds great. I'll attend PCI mini-summit.
 
  Best regards,
  Taku Izumi


> 
> Thanks
> 
> Yinghai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Taku Izumi]

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] On Behalf Of Yinghai Lu
> Sent: Saturday, August 11, 2012 1:41 AM
> To: Taku Izumi
> Cc: linux-pci@vger.kernel.org; bhelgaas@google.com; linux-acpi@vger.kernel.org; kaneshige.kenji@jp.fujitsu.com;
> jiang.liu@huawei.com
> Subject: Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support
> 
> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> >  Hi all,
> >
> >
> >  I'd like to merge hostbridge hotplug feature.
> >  I looked at Yinghai's branch and found that this branch contains
> >  many work and can be split into some parts.
> >  I believe it is good to merge step by step.
> >
> >  My idea is splitting into the following 4 parts:
> >    1. basic hostbirdge hotplug work
> >    2. acpiphp work
> >    3. /sysfs interface work (logical hotplug?)
> >    4. cleanup
> >
> >  This patchset is 1st step based on the following Yinghai's branch:
> 
> Hi,
> 
> Are you going to attend PCI mini-summit 8/28? We should discuss there.

  Sounds great. I'll attend PCI mini-summit.
 
  Best regards,
  Taku Izumi


> 
> Thanks
> 
> Yinghai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Bjorn Helgaas]

On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>  Hi all,
>
>
>  I'd like to merge hostbridge hotplug feature.
>  I looked at Yinghai's branch and found that this branch contains
>  many work and can be split into some parts.
>  I believe it is good to merge step by step.
>
>  My idea is splitting into the following 4 parts:
>    1. basic hostbirdge hotplug work
>    2. acpiphp work
>    3. /sysfs interface work (logical hotplug?)
>    4. cleanup
>
>  This patchset is 1st step based on the following Yinghai's branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
> for-pci-root-bus-hotplug
>
>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

I looked at merging these tonight.  But I noticed there are still a
few unanswered questions I asked earlier.

1) For patch [1/7], I pointed out that there is currently no way to
remove a non-ACPI host bridge, which means the fact that we don't free
the pci_sysdata is not really a leak.  If you want to add the
release_fn so that you can add support for removing and adding these
non-ACPI host bridges in the future, I do not understand that.  It
just doesn't make sense to me to try to support hotplug for those
bridges.

2) For patch [2/7], I'm still curious about the scenario where this
patch makes a difference (see my previous response for details).

3) Patch [5/7], adds mutual exclusion to two different lists, using
two different mechanisms (mutex and RCU).  It would be easier for
people to review if this were done in two separate patches.

Bjorn

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-8-30 14:23, Bjorn Helgaas wrote:
> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>  Hi all,
>>
>>
>>  I'd like to merge hostbridge hotplug feature.
>>  I looked at Yinghai's branch and found that this branch contains
>>  many work and can be split into some parts.
>>  I believe it is good to merge step by step.
>>
>>  My idea is splitting into the following 4 parts:
>>    1. basic hostbirdge hotplug work
>>    2. acpiphp work
>>    3. /sysfs interface work (logical hotplug?)
>>    4. cleanup
>>
>>  This patchset is 1st step based on the following Yinghai's branch:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>> for-pci-root-bus-hotplug
>>
>>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
> 
> I looked at merging these tonight.  But I noticed there are still a
> few unanswered questions I asked earlier.
> 
> 1) For patch [1/7], I pointed out that there is currently no way to
> remove a non-ACPI host bridge, which means the fact that we don't free
> the pci_sysdata is not really a leak.  If you want to add the
> release_fn so that you can add support for removing and adding these
> non-ACPI host bridges in the future, I do not understand that.  It
> just doesn't make sense to me to try to support hotplug for those
> bridges.
> 
> 2) For patch [2/7], I'm still curious about the scenario where this
> patch makes a difference (see my previous response for details).
> 
> 3) Patch [5/7], adds mutual exclusion to two different lists, using
> two different mechanisms (mutex and RCU).  It would be easier for
> people to review if this were done in two separate patches.
Hi Bjorn,
	Sorry, I guessed you won't merge this into 3.6, so delayed reply. Will handle it tonight.
	--Gerry

> 
> Bjorn
> 
> 

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-8-30 14:23, Bjorn Helgaas wrote:
> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>  Hi all,
>>
>>
>>  I'd like to merge hostbridge hotplug feature.
>>  I looked at Yinghai's branch and found that this branch contains
>>  many work and can be split into some parts.
>>  I believe it is good to merge step by step.
>>
>>  My idea is splitting into the following 4 parts:
>>    1. basic hostbirdge hotplug work
>>    2. acpiphp work
>>    3. /sysfs interface work (logical hotplug?)
>>    4. cleanup
>>
>>  This patchset is 1st step based on the following Yinghai's branch:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>> for-pci-root-bus-hotplug
>>
>>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
> 
> I looked at merging these tonight.  But I noticed there are still a
> few unanswered questions I asked earlier.
> 
> 1) For patch [1/7], I pointed out that there is currently no way to
> remove a non-ACPI host bridge, which means the fact that we don't free
> the pci_sysdata is not really a leak.  If you want to add the
> release_fn so that you can add support for removing and adding these
> non-ACPI host bridges in the future, I do not understand that.  It
> just doesn't make sense to me to try to support hotplug for those
> bridges.
> 
> 2) For patch [2/7], I'm still curious about the scenario where this
> patch makes a difference (see my previous response for details).
> 
> 3) Patch [5/7], adds mutual exclusion to two different lists, using
> two different mechanisms (mutex and RCU).  It would be easier for
> people to review if this were done in two separate patches.
Hi Bjorn,
	Sorry, I guessed you won't merge this into 3.6, so delayed reply. Will handle it tonight.
	--Gerry

> 
> Bjorn
> 
> 

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Yinghai Lu]

On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>  Hi all,
>>
>>
>>  I'd like to merge hostbridge hotplug feature.
>>  I looked at Yinghai's branch and found that this branch contains
>>  many work and can be split into some parts.
>>  I believe it is good to merge step by step.
>>
>>  My idea is splitting into the following 4 parts:
>>    1. basic hostbirdge hotplug work
>>    2. acpiphp work
>>    3. /sysfs interface work (logical hotplug?)
>>    4. cleanup
>>
>>  This patchset is 1st step based on the following Yinghai's branch:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>> for-pci-root-bus-hotplug
>>
>>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
>
> I looked at merging these tonight.  But I noticed there are still a
> few unanswered questions I asked earlier.
>
> 1) For patch [1/7], I pointed out that there is currently no way to
> remove a non-ACPI host bridge, which means the fact that we don't free
> the pci_sysdata is not really a leak.  If you want to add the
> release_fn so that you can add support for removing and adding these
> non-ACPI host bridges in the future, I do not understand that.  It
> just doesn't make sense to me to try to support hotplug for those
> bridges.

for Intel Nehalem and westmere -ex system, there will be root bus from
0xf8 to 0xff for cpus.
and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
pcibios_last_bus.
so those but get probed via pcibios_fixup_peer_bridges.

I hope I could use /sys to remove non-acpi root bus.

As for merging, I suggest following sequence:
1. fw resource allocate per bus.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=patch;h=3b0a9d26e1838b7ddd2b2273ee38ff4ea3bcc89e

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=bb6436eb378c2b86478d3d6dfae3b032288ce68a

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=48e4eb57f72cba194486ed8b264148820a4165ec

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f01ef104f823ec8fbe0eb0313d1677ef596eeb74

2. separate out pci host bridge hotplug add support from acpiphp

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f00a5bdd7f0460eef573e2ef8f2a96a408300c75

3. add pci host bridge remove support
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=eb46eaf0369464e4fab003f8721dc80423b34ee1
...

will rebase
busn_alloc and root bus branch and send whole patchset for review.

Thanks

Yinghai

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 08/30/2012 11:48 PM, Yinghai Lu wrote:
> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>>  Hi all,
>>>
>>>
>>>  I'd like to merge hostbridge hotplug feature.
>>>  I looked at Yinghai's branch and found that this branch contains
>>>  many work and can be split into some parts.
>>>  I believe it is good to merge step by step.
>>>
>>>  My idea is splitting into the following 4 parts:
>>>    1. basic hostbirdge hotplug work
>>>    2. acpiphp work
>>>    3. /sysfs interface work (logical hotplug?)
>>>    4. cleanup
>>>
>>>  This patchset is 1st step based on the following Yinghai's branch:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>> for-pci-root-bus-hotplug
>>>
>>>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>>>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>>>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>>>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>>>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
>>
>> I looked at merging these tonight.  But I noticed there are still a
>> few unanswered questions I asked earlier.
>>
>> 1) For patch [1/7], I pointed out that there is currently no way to
>> remove a non-ACPI host bridge, which means the fact that we don't free
>> the pci_sysdata is not really a leak.  If you want to add the
>> release_fn so that you can add support for removing and adding these
>> non-ACPI host bridges in the future, I do not understand that.  It
>> just doesn't make sense to me to try to support hotplug for those
>> bridges.
> 
> for Intel Nehalem and westmere -ex system, there will be root bus from
> 0xf8 to 0xff for cpus.
> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
> pcibios_last_bus.
> so those but get probed via pcibios_fixup_peer_bridges.
Hi Yinghai,
	For system does support physical processor hotplug, I think
BIOS should expose those special PCI buses, otherwise we may run into
trouble after removing a physical processor.
	BTW, do we really want to support logical PCI host bridge hotplug
or only support physical host bridge hotplug? Here logical hotplug means
adding a PCI bus into or removing a PCI bus from the running kernel.
	--Gerry 

> 
> I hope I could use /sys to remove non-acpi root bus.
> 
> As for merging, I suggest following sequence:
> 1. fw resource allocate per bus.
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=patch;h=3b0a9d26e1838b7ddd2b2273ee38ff4ea3bcc89e
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=bb6436eb378c2b86478d3d6dfae3b032288ce68a
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=48e4eb57f72cba194486ed8b264148820a4165ec
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f01ef104f823ec8fbe0eb0313d1677ef596eeb74
> 
> 2. separate out pci host bridge hotplug add support from acpiphp
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f00a5bdd7f0460eef573e2ef8f2a96a408300c75
> 
> 3. add pci host bridge remove support
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=eb46eaf0369464e4fab003f8721dc80423b34ee1
> ...
> 
> will rebase
> busn_alloc and root bus branch and send whole patchset for review.
> 
> Thanks
> 
> Yinghai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Yinghai Lu]

On Thu, Aug 30, 2012 at 9:38 AM, Jiang Liu <liuj97@gmail.com> wrote:
> On 08/30/2012 11:48 PM, Yinghai Lu wrote:
>         For system does support physical processor hotplug, I think
> BIOS should expose those special PCI buses, otherwise we may run into
> trouble after removing a physical processor.
No, not do the physical removing.

I just want to remove those not needed pci root bus.

lspci output get filled of all the those useless cpu pci devices.

>         BTW, do we really want to support logical PCI host bridge hotplug
> or only support physical host bridge hotplug? Here logical hotplug means
> adding a PCI bus into or removing a PCI bus from the running kernel.

get logical done at first, and then via acpi we could do real physical removal.

Thanks

Yinghai

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Bjorn Helgaas]

On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:

>> 1) For patch [1/7], I pointed out that there is currently no way to
>> remove a non-ACPI host bridge, which means the fact that we don't free
>> the pci_sysdata is not really a leak.  If you want to add the
>> release_fn so that you can add support for removing and adding these
>> non-ACPI host bridges in the future, I do not understand that.  It
>> just doesn't make sense to me to try to support hotplug for those
>> bridges.
>
> for Intel Nehalem and westmere -ex system, there will be root bus from
> 0xf8 to 0xff for cpus.
> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
> pcibios_last_bus.
> so those but get probed via pcibios_fixup_peer_bridges.

I understand how these buses get scanned.  What I don't understand is
what value we get from trying to make these buses hot-pluggable.

> I hope I could use /sys to remove non-acpi root bus.

Why?  I don't think there's any value in being able to remove non-ACPI
host bridges.  Any x86 host bridge hotplug that's actually useful to
users will be done via ACPI.

You mention later that you want to remove these buses because they
only contain CPU devices that don't seem to be good for anything.  I
would rather do this by simply not scanning for peer bridges in the
first place.  That's simpler than scanning the bridge, deciding we
don't care about what we found, then trying to hot-remove it.

Bjorn

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-8-31 8:43, Bjorn Helgaas wrote:
> On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> 
>>> 1) For patch [1/7], I pointed out that there is currently no way to
>>> remove a non-ACPI host bridge, which means the fact that we don't free
>>> the pci_sysdata is not really a leak.  If you want to add the
>>> release_fn so that you can add support for removing and adding these
>>> non-ACPI host bridges in the future, I do not understand that.  It
>>> just doesn't make sense to me to try to support hotplug for those
>>> bridges.
>>
>> for Intel Nehalem and westmere -ex system, there will be root bus from
>> 0xf8 to 0xff for cpus.
>> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
>> pcibios_last_bus.
>> so those but get probed via pcibios_fixup_peer_bridges.
> 
> I understand how these buses get scanned.  What I don't understand is
> what value we get from trying to make these buses hot-pluggable.
> 
>> I hope I could use /sys to remove non-acpi root bus.
> 
> Why?  I don't think there's any value in being able to remove non-ACPI
> host bridges.  Any x86 host bridge hotplug that's actually useful to
> users will be done via ACPI.
> 
> You mention later that you want to remove these buses because they
> only contain CPU devices that don't seem to be good for anything.  I
> would rather do this by simply not scanning for peer bridges in the
> first place.  That's simpler than scanning the bridge, deciding we
> don't care about what we found, then trying to hot-remove it.
Actually some memory error handling mechanism may depend on those CPU
devices to do memory address decoding. For example, EDAC needs to access
them. And we are working on a project to do memory address translation
by reading memory controller information from those CPU devices too.

> 
> Bjorn
> 
> 

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-8-31 8:43, Bjorn Helgaas wrote:
> On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> 
>>> 1) For patch [1/7], I pointed out that there is currently no way to
>>> remove a non-ACPI host bridge, which means the fact that we don't free
>>> the pci_sysdata is not really a leak.  If you want to add the
>>> release_fn so that you can add support for removing and adding these
>>> non-ACPI host bridges in the future, I do not understand that.  It
>>> just doesn't make sense to me to try to support hotplug for those
>>> bridges.
>>
>> for Intel Nehalem and westmere -ex system, there will be root bus from
>> 0xf8 to 0xff for cpus.
>> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
>> pcibios_last_bus.
>> so those but get probed via pcibios_fixup_peer_bridges.
> 
> I understand how these buses get scanned.  What I don't understand is
> what value we get from trying to make these buses hot-pluggable.
> 
>> I hope I could use /sys to remove non-acpi root bus.
> 
> Why?  I don't think there's any value in being able to remove non-ACPI
> host bridges.  Any x86 host bridge hotplug that's actually useful to
> users will be done via ACPI.
> 
> You mention later that you want to remove these buses because they
> only contain CPU devices that don't seem to be good for anything.  I
> would rather do this by simply not scanning for peer bridges in the
> first place.  That's simpler than scanning the bridge, deciding we
> don't care about what we found, then trying to hot-remove it.
Actually some memory error handling mechanism may depend on those CPU
devices to do memory address decoding. For example, EDAC needs to access
them. And we are working on a project to do memory address translation
by reading memory controller information from those CPU devices too.

> 
> Bjorn
> 
> 

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Bjorn Helgaas]

On Thu, Aug 30, 2012 at 6:03 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
> On 2012-8-31 8:43, Bjorn Helgaas wrote:
>> On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>>>> 1) For patch [1/7], I pointed out that there is currently no way to
>>>> remove a non-ACPI host bridge, which means the fact that we don't free
>>>> the pci_sysdata is not really a leak.  If you want to add the
>>>> release_fn so that you can add support for removing and adding these
>>>> non-ACPI host bridges in the future, I do not understand that.  It
>>>> just doesn't make sense to me to try to support hotplug for those
>>>> bridges.
>>>
>>> for Intel Nehalem and westmere -ex system, there will be root bus from
>>> 0xf8 to 0xff for cpus.
>>> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
>>> pcibios_last_bus.
>>> so those but get probed via pcibios_fixup_peer_bridges.
>>
>> I understand how these buses get scanned.  What I don't understand is
>> what value we get from trying to make these buses hot-pluggable.
>>
>>> I hope I could use /sys to remove non-acpi root bus.
>>
>> Why?  I don't think there's any value in being able to remove non-ACPI
>> host bridges.  Any x86 host bridge hotplug that's actually useful to
>> users will be done via ACPI.
>>
>> You mention later that you want to remove these buses because they
>> only contain CPU devices that don't seem to be good for anything.  I
>> would rather do this by simply not scanning for peer bridges in the
>> first place.  That's simpler than scanning the bridge, deciding we
>> don't care about what we found, then trying to hot-remove it.

> Actually some memory error handling mechanism may depend on those CPU
> devices to do memory address decoding. For example, EDAC needs to access
> them. And we are working on a project to do memory address translation
> by reading memory controller information from those CPU devices too.

If you depend on those CPU-related PCI devices for EDAC or any other
reason, your BIOS should expose a host bridge leading to them.  If it
does, we'll enumerate them just like we do today, and whatever host
bridge hotplug support we add should work for those bridges just like
any other host bridge.

The issue we're struggling with here is how we handle non-ACPI host
bridges, i.e., bridges the BIOS doesn't tell us about but we assume
are present because we find things when we blindly probe for devices.
I think we should not even try to do hotplug of bridges like this.
Hotplug for these non-ACPI bridges would be completely
platform-dependent, since there would be no ACPI notifications for
removal or rescan.  For example, we'd have to have platform-dependent
code to turn off the power to a node before removing it.

Yinghai proposed making sysfs hooks for doing "hotplug" of these
non-ACPI bridges by just removing the pci_bus and pci_dev topology for
"remove" and by blindly scanning again for "add."  I think that's
pointless work since it's only shuffling kernel data structures --
it's not related to physically removing a node or anything like that.
If we want to test this path (and we should), we should do it on ACPI
bridges where we can exercise the whole path, including ejecting the
ACPI bridge device for "remove" and rescanning the ACPI namespace for
"add."

Bjorn

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-8-31 13:04, Bjorn Helgaas wrote:
> On Thu, Aug 30, 2012 at 6:03 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
>> On 2012-8-31 8:43, Bjorn Helgaas wrote:
>>> On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>
>>>>> 1) For patch [1/7], I pointed out that there is currently no way to
>>>>> remove a non-ACPI host bridge, which means the fact that we don't free
>>>>> the pci_sysdata is not really a leak.  If you want to add the
>>>>> release_fn so that you can add support for removing and adding these
>>>>> non-ACPI host bridges in the future, I do not understand that.  It
>>>>> just doesn't make sense to me to try to support hotplug for those
>>>>> bridges.
>>>>
>>>> for Intel Nehalem and westmere -ex system, there will be root bus from
>>>> 0xf8 to 0xff for cpus.
>>>> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
>>>> pcibios_last_bus.
>>>> so those but get probed via pcibios_fixup_peer_bridges.
>>>
>>> I understand how these buses get scanned.  What I don't understand is
>>> what value we get from trying to make these buses hot-pluggable.
>>>
>>>> I hope I could use /sys to remove non-acpi root bus.
>>>
>>> Why?  I don't think there's any value in being able to remove non-ACPI
>>> host bridges.  Any x86 host bridge hotplug that's actually useful to
>>> users will be done via ACPI.
>>>
>>> You mention later that you want to remove these buses because they
>>> only contain CPU devices that don't seem to be good for anything.  I
>>> would rather do this by simply not scanning for peer bridges in the
>>> first place.  That's simpler than scanning the bridge, deciding we
>>> don't care about what we found, then trying to hot-remove it.
> 
>> Actually some memory error handling mechanism may depend on those CPU
>> devices to do memory address decoding. For example, EDAC needs to access
>> them. And we are working on a project to do memory address translation
>> by reading memory controller information from those CPU devices too.
> 
> If you depend on those CPU-related PCI devices for EDAC or any other
> reason, your BIOS should expose a host bridge leading to them.  If it
> does, we'll enumerate them just like we do today, and whatever host
> bridge hotplug support we add should work for those bridges just like
> any other host bridge.
Yes, BIOS should export those PCI buses for CPU devices through ACPI,
but some legacy hardware platform doesn't do that.

I feel it's not worth to provide hotplug support for those PCI buses too,
it just hides those PCI devices.
--Gerry

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-8-31 13:04, Bjorn Helgaas wrote:
> On Thu, Aug 30, 2012 at 6:03 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
>> On 2012-8-31 8:43, Bjorn Helgaas wrote:
>>> On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>
>>>>> 1) For patch [1/7], I pointed out that there is currently no way to
>>>>> remove a non-ACPI host bridge, which means the fact that we don't free
>>>>> the pci_sysdata is not really a leak.  If you want to add the
>>>>> release_fn so that you can add support for removing and adding these
>>>>> non-ACPI host bridges in the future, I do not understand that.  It
>>>>> just doesn't make sense to me to try to support hotplug for those
>>>>> bridges.
>>>>
>>>> for Intel Nehalem and westmere -ex system, there will be root bus from
>>>> 0xf8 to 0xff for cpus.
>>>> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
>>>> pcibios_last_bus.
>>>> so those but get probed via pcibios_fixup_peer_bridges.
>>>
>>> I understand how these buses get scanned.  What I don't understand is
>>> what value we get from trying to make these buses hot-pluggable.
>>>
>>>> I hope I could use /sys to remove non-acpi root bus.
>>>
>>> Why?  I don't think there's any value in being able to remove non-ACPI
>>> host bridges.  Any x86 host bridge hotplug that's actually useful to
>>> users will be done via ACPI.
>>>
>>> You mention later that you want to remove these buses because they
>>> only contain CPU devices that don't seem to be good for anything.  I
>>> would rather do this by simply not scanning for peer bridges in the
>>> first place.  That's simpler than scanning the bridge, deciding we
>>> don't care about what we found, then trying to hot-remove it.
> 
>> Actually some memory error handling mechanism may depend on those CPU
>> devices to do memory address decoding. For example, EDAC needs to access
>> them. And we are working on a project to do memory address translation
>> by reading memory controller information from those CPU devices too.
> 
> If you depend on those CPU-related PCI devices for EDAC or any other
> reason, your BIOS should expose a host bridge leading to them.  If it
> does, we'll enumerate them just like we do today, and whatever host
> bridge hotplug support we add should work for those bridges just like
> any other host bridge.
Yes, BIOS should export those PCI buses for CPU devices through ACPI,
but some legacy hardware platform doesn't do that.

I feel it's not worth to provide hotplug support for those PCI buses too,
it just hides those PCI devices.
--Gerry

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Bjorn Helgaas]

On Thu, Aug 30, 2012 at 10:19 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
> On 2012-8-31 13:04, Bjorn Helgaas wrote:
>> On Thu, Aug 30, 2012 at 6:03 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
>>> On 2012-8-31 8:43, Bjorn Helgaas wrote:
>>>> On Thu, Aug 30, 2012 at 8:48 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>>> On Wed, Aug 29, 2012 at 11:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>>
>>>>>> 1) For patch [1/7], I pointed out that there is currently no way to
>>>>>> remove a non-ACPI host bridge, which means the fact that we don't free
>>>>>> the pci_sysdata is not really a leak.  If you want to add the
>>>>>> release_fn so that you can add support for removing and adding these
>>>>>> non-ACPI host bridges in the future, I do not understand that.  It
>>>>>> just doesn't make sense to me to try to support hotplug for those
>>>>>> bridges.
>>>>>
>>>>> for Intel Nehalem and westmere -ex system, there will be root bus from
>>>>> 0xf8 to 0xff for cpus.
>>>>> and BIOS does not put the in ACPI, but __pci_mmcfg_init will set the
>>>>> pcibios_last_bus.
>>>>> so those but get probed via pcibios_fixup_peer_bridges.
>>>>
>>>> I understand how these buses get scanned.  What I don't understand is
>>>> what value we get from trying to make these buses hot-pluggable.
>>>>
>>>>> I hope I could use /sys to remove non-acpi root bus.
>>>>
>>>> Why?  I don't think there's any value in being able to remove non-ACPI
>>>> host bridges.  Any x86 host bridge hotplug that's actually useful to
>>>> users will be done via ACPI.
>>>>
>>>> You mention later that you want to remove these buses because they
>>>> only contain CPU devices that don't seem to be good for anything.  I
>>>> would rather do this by simply not scanning for peer bridges in the
>>>> first place.  That's simpler than scanning the bridge, deciding we
>>>> don't care about what we found, then trying to hot-remove it.
>>
>>> Actually some memory error handling mechanism may depend on those CPU
>>> devices to do memory address decoding. For example, EDAC needs to access
>>> them. And we are working on a project to do memory address translation
>>> by reading memory controller information from those CPU devices too.
>>
>> If you depend on those CPU-related PCI devices for EDAC or any other
>> reason, your BIOS should expose a host bridge leading to them.  If it
>> does, we'll enumerate them just like we do today, and whatever host
>> bridge hotplug support we add should work for those bridges just like
>> any other host bridge.

> Yes, BIOS should export those PCI buses for CPU devices through ACPI,
> but some legacy hardware platform doesn't do that.

That's true.  I think we should blindly probe for devices only when
(1) the system does not have ACPI or (2) the DMI BIOS date is older
than some date, e.g., 1 Jan 2013.  For new systems with ACPI, we
should only look at the host bridges described in ACPI.

Bjorn

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Yinghai Lu]

On Thu, Aug 30, 2012 at 10:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>
> That's true.  I think we should blindly probe for devices only when
> (1) the system does not have ACPI or (2) the DMI BIOS date is older
> than some date, e.g., 1 Jan 2013.  For new systems with ACPI, we
> should only look at the host bridges described in ACPI.

if those buses do not get scanned, then it should be enough.

how about just removing pcibios_last_bus probing code?

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 937bcec..3f3c460 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -616,16 +616,6 @@ static void __init __pci_mmcfg_init(int early)
        if (list_empty(&pci_mmcfg_list))
                return;

-       if (pcibios_last_bus < 0) {
-               const struct pci_mmcfg_region *cfg;
-
-               list_for_each_entry(cfg, &pci_mmcfg_list, list) {
-                       if (cfg->segment)
-                               break;
-                       pcibios_last_bus = cfg->end_bus;
-               }
-       }
-
        if (pci_mmcfg_arch_init())
                pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
        else {


also Nehalem *EX* and Westmere *EX* does not support EDAC.

so if the user with Nehalem/Westmere Desktop version, want to use to
EDAC, they could just append
pci=last_bus=255 to get those buses scanned.

Yinghai

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 09/01/2012 12:44 AM, Yinghai Lu wrote:
> On Thu, Aug 30, 2012 at 10:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>> That's true.  I think we should blindly probe for devices only when
>> (1) the system does not have ACPI or (2) the DMI BIOS date is older
>> than some date, e.g., 1 Jan 2013.  For new systems with ACPI, we
>> should only look at the host bridges described in ACPI.
> 
> if those buses do not get scanned, then it should be enough.
> 
> how about just removing pcibios_last_bus probing code?
> 
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index 937bcec..3f3c460 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -616,16 +616,6 @@ static void __init __pci_mmcfg_init(int early)
>         if (list_empty(&pci_mmcfg_list))
>                 return;
> 
> -       if (pcibios_last_bus < 0) {
> -               const struct pci_mmcfg_region *cfg;
> -
> -               list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> -                       if (cfg->segment)
> -                               break;
> -                       pcibios_last_bus = cfg->end_bus;
> -               }
> -       }
> -
>         if (pci_mmcfg_arch_init())
>                 pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
>         else {
> 
> 
> also Nehalem *EX* and Westmere *EX* does not support EDAC.
> 
> so if the user with Nehalem/Westmere Desktop version, want to use to
> EDAC, they could just append
> pci=last_bus=255 to get those buses scanned.
I think this is a better solution than non-ACPI root bus hotplug.
--Gerry

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Taku Izumi]

Hi Bjorn,

  Sorry for late response. 
  I've returned to Japan. It was nice to see you.

On Wed, 29 Aug 2012 23:23:49 -0700
Bjorn Helgaas <bhelgaas@google.com> wrote:

> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> >  Hi all,
> >
> >
> >  I'd like to merge hostbridge hotplug feature.
> >  I looked at Yinghai's branch and found that this branch contains
> >  many work and can be split into some parts.
> >  I believe it is good to merge step by step.
> >
> >  My idea is splitting into the following 4 parts:
> >    1. basic hostbirdge hotplug work
> >    2. acpiphp work
> >    3. /sysfs interface work (logical hotplug?)
> >    4. cleanup
> >
> >  This patchset is 1st step based on the following Yinghai's branch:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
> > for-pci-root-bus-hotplug
> >
> >   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
> >   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
> >   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
> >   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
> >   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
> >   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
> >   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
> 
> I looked at merging these tonight.  But I noticed there are still a
> few unanswered questions I asked earlier.
> 
> 1) For patch [1/7], I pointed out that there is currently no way to
> remove a non-ACPI host bridge, which means the fact that we don't free
> the pci_sysdata is not really a leak.  If you want to add the
> release_fn so that you can add support for removing and adding these
> non-ACPI host bridges in the future, I do not understand that.  It
> just doesn't make sense to me to try to support hotplug for those
> bridges.

  I see. I plan on ACPI-based host bridge hotplug, so I omit this patch now.

> 2) For patch [2/7], I'm still curious about the scenario where this
> patch makes a difference (see my previous response for details).

  I'll omit this patch too according to Jian's comment.

> 3) Patch [5/7], adds mutual exclusion to two different lists, using
> two different mechanisms (mutex and RCU).  It would be easier for
> people to review if this were done in two separate patches.
> 
  OK. I'll sprit into 2 patches.

  And I'll resend new version one soon.

-- 
Best regards,
Taku Izumi <izumi.taku@jp.fujitsu.com>

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-9-3 10:28, Taku Izumi wrote:
> Hi Bjorn,
> 
>   Sorry for late response. 
>   I've returned to Japan. It was nice to see you.
> 
> On Wed, 29 Aug 2012 23:23:49 -0700
> Bjorn Helgaas <bhelgaas@google.com> wrote:
> 
>> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>>  Hi all,
>>>
>>>
>>>  I'd like to merge hostbridge hotplug feature.
>>>  I looked at Yinghai's branch and found that this branch contains
>>>  many work and can be split into some parts.
>>>  I believe it is good to merge step by step.
>>>
>>>  My idea is splitting into the following 4 parts:
>>>    1. basic hostbirdge hotplug work
>>>    2. acpiphp work
>>>    3. /sysfs interface work (logical hotplug?)
>>>    4. cleanup
>>>
>>>  This patchset is 1st step based on the following Yinghai's branch:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>> for-pci-root-bus-hotplug
>>>
>>>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>>>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>>>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>>>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>>>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
>>
>> I looked at merging these tonight.  But I noticed there are still a
>> few unanswered questions I asked earlier.
>>
>> 1) For patch [1/7], I pointed out that there is currently no way to
>> remove a non-ACPI host bridge, which means the fact that we don't free
>> the pci_sysdata is not really a leak.  If you want to add the
>> release_fn so that you can add support for removing and adding these
>> non-ACPI host bridges in the future, I do not understand that.  It
>> just doesn't make sense to me to try to support hotplug for those
>> bridges.
> 
>   I see. I plan on ACPI-based host bridge hotplug, so I omit this patch now.
> 
>> 2) For patch [2/7], I'm still curious about the scenario where this
>> patch makes a difference (see my previous response for details).
> 
>   I'll omit this patch too according to Jian's comment.
> 
>> 3) Patch [5/7], adds mutual exclusion to two different lists, using
>> two different mechanisms (mutex and RCU).  It would be easier for
>> people to review if this were done in two separate patches.
>>
>   OK. I'll sprit into 2 patches.
> 
>   And I'll resend new version one soon.
Hi Taku,
	I have already split it into two patches:)
	--Gerry

> 

Re: [PATCH 0/7][RESEND] acpi, pci: hostbridge hotplug support

[Jiang Liu]

On 2012-9-3 10:28, Taku Izumi wrote:
> Hi Bjorn,
> 
>   Sorry for late response. 
>   I've returned to Japan. It was nice to see you.
> 
> On Wed, 29 Aug 2012 23:23:49 -0700
> Bjorn Helgaas <bhelgaas@google.com> wrote:
> 
>> On Thu, Aug 9, 2012 at 11:09 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>>  Hi all,
>>>
>>>
>>>  I'd like to merge hostbridge hotplug feature.
>>>  I looked at Yinghai's branch and found that this branch contains
>>>  many work and can be split into some parts.
>>>  I believe it is good to merge step by step.
>>>
>>>  My idea is splitting into the following 4 parts:
>>>    1. basic hostbirdge hotplug work
>>>    2. acpiphp work
>>>    3. /sysfs interface work (logical hotplug?)
>>>    4. cleanup
>>>
>>>  This patchset is 1st step based on the following Yinghai's branch:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>> for-pci-root-bus-hotplug
>>>
>>>   * [PATCH 1/7][RESEND] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
>>>   * [PATCH 2/7][RESEND] PCI: Correctly clean up pci root buses in function pci_remove_bus()
>>>   * [PATCH 3/7][RESEND] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>>   * [PATCH 4/7][RESEND] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>>   * [PATCH 5/7][RESEND] ACPI, PCI: Protect global lists in drivers/acpi/pci_root.c
>>>   * [PATCH 6/7][RESEND] ACPI, PCI: add hostbridge removal function
>>>   * [PATCH 7/7][RESEND] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
>>
>> I looked at merging these tonight.  But I noticed there are still a
>> few unanswered questions I asked earlier.
>>
>> 1) For patch [1/7], I pointed out that there is currently no way to
>> remove a non-ACPI host bridge, which means the fact that we don't free
>> the pci_sysdata is not really a leak.  If you want to add the
>> release_fn so that you can add support for removing and adding these
>> non-ACPI host bridges in the future, I do not understand that.  It
>> just doesn't make sense to me to try to support hotplug for those
>> bridges.
> 
>   I see. I plan on ACPI-based host bridge hotplug, so I omit this patch now.
> 
>> 2) For patch [2/7], I'm still curious about the scenario where this
>> patch makes a difference (see my previous response for details).
> 
>   I'll omit this patch too according to Jian's comment.
> 
>> 3) Patch [5/7], adds mutual exclusion to two different lists, using
>> two different mechanisms (mutex and RCU).  It would be easier for
>> people to review if this were done in two separate patches.
>>
>   OK. I'll sprit into 2 patches.
> 
>   And I'll resend new version one soon.
Hi Taku,
	I have already split it into two patches:)
	--Gerry

>