From: Bjorn Helgaas <bhelgaas@google.com>
To: Taku Izumi <izumi.taku@jp.fujitsu.com>
Cc: linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
kaneshige.kenji@jp.fujitsu.com, yinghai@kernel.org,
jiang.liu@huawei.com
Subject: Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function
Date: Mon, 29 Oct 2012 22:02:37 -0600 [thread overview]
Message-ID: <20121030040237.GA10472@google.com> (raw)
In-Reply-To: <20120928184627.0b039d14.izumi.taku@jp.fujitsu.com>
I think I'm missing patches 7/8 and 8/8 from this series. Can
you repost them to make sure I have the latest versions?
Note the comments below:
On Fri, Sep 28, 2012 at 06:46:27PM +0900, Taku Izumi wrote:
>
> Currently there's no PCI-related clean-up code
> in acpi_pci_root_remove() function.
> This patch introduces function for hostbridge removal,
> and brings back pci_stop_bus_devices() function.
>
> diff: rebased against current next
> updated according to Bjorn's comment
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
> drivers/acpi/pci_bind.c | 7 +++++++
> drivers/acpi/pci_root.c | 6 ++++++
> drivers/pci/remove.c | 28 ++++++++++++++++++++++++++++
> include/acpi/acpi_drivers.h | 1 +
> include/linux/pci.h | 2 ++
> 5 files changed, 44 insertions(+)
>
> Index: Bjorn-next-0925-configchange/drivers/pci/remove.c
> ===================================================================
> --- Bjorn-next-0925-configchange.orig/drivers/pci/remove.c
> +++ Bjorn-next-0925-configchange/drivers/pci/remove.c
> @@ -111,3 +111,31 @@ void pci_stop_and_remove_bus_device(stru
> pci_remove_bus_device(dev);
> }
> EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
> +
> +void pci_stop_bus_devices(struct pci_bus *bus)
> +{
> + struct pci_dev *dev, *tmp;
> +
> + list_for_each_entry_safe_reverse(dev, tmp,
> + &bus->devices, bus_list) {
> + pci_stop_bus_device(dev);
> + }
> +
> +}
> +EXPORT_SYMBOL(pci_stop_bus_devices);
I'm hesitant to introduce pci_stop_bus_devices() again, particularly
when it is exported. The stop/remove split introduces the state where
devices are "stopped" but haven't been "removed" yet.
In this state, the driver .remove() method has been called, sysfs has
been cleaned up, and the struct device has been unregistered, but the
struct pci_dev itself still exists. Obviously, this state *must*
exist internally in the PCI core as we remove the PCI device.
The problem is that we have non-core code that *depends* on being
run while in this transitory state. I think this is a design mistake.
The code that depends on this state is basically just the stuff in the
acpi_pci_drivers list, namely, acpi_pci_hp_driver and acpi_pci_slot_driver.
I suspect that the main reason we have the acpi_pci_drivers list and the
whole acpi_pci_register_driver() infrastructure is so that these PCI
host bridge "sub-drivers" can be built as modules.
I don't think there's really any value in having these sub-drivers as
modules, and it leads to a lot of complication in the code. I'm pretty
sure that forcing them to be selected at build-time will let us make
things much simpler.
If we have to have pci_stop_bus_devices() as an interim measure, I can
live with it, but it doesn't need to be exported, does it?
> +void pci_remove_host_bridge(struct pci_host_bridge *bridge)
> +{
> + struct pci_bus *root = bridge->bus;
> + struct pci_dev *dev, *tmp;
> +
> + list_for_each_entry_safe_reverse(dev, tmp,
> + &root->devices, bus_list) {
> + pci_remove_bus_device(dev);
> + }
> +
> + pci_remove_bus(root);
> +
> + device_unregister(&bridge->dev);
> +}
> +EXPORT_SYMBOL(pci_remove_host_bridge);
> Index: Bjorn-next-0925-configchange/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next-0925-configchange.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next-0925-configchange/drivers/acpi/pci_root.c
> @@ -659,8 +659,10 @@ 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);
> @@ -668,6 +670,10 @@ 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_unbind_root(device);
> +
> + pci_remove_host_bridge(bridge);
> +
> list_del(&root->node);
> mutex_unlock(&acpi_pci_root_lock);
> kfree(root);
> Index: Bjorn-next-0925-configchange/include/linux/pci.h
> ===================================================================
> --- Bjorn-next-0925-configchange.orig/include/linux/pci.h
> +++ Bjorn-next-0925-configchange/include/linux/pci.h
> @@ -734,6 +734,8 @@ extern struct pci_dev *pci_dev_get(struc
> extern void pci_dev_put(struct pci_dev *dev);
> extern void pci_remove_bus(struct pci_bus *b);
> 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_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-0925-configchange/drivers/acpi/pci_bind.c
> ===================================================================
> --- Bjorn-next-0925-configchange.orig/drivers/acpi/pci_bind.c
> +++ Bjorn-next-0925-configchange/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-0925-configchange/include/acpi/acpi_drivers.h
> ===================================================================
> --- Bjorn-next-0925-configchange.orig/include/acpi/acpi_drivers.h
> +++ Bjorn-next-0925-configchange/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 */
>
>
next prev parent reply other threads:[~2012-10-30 4:02 UTC|newest]
Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-18 6:12 [PATCH v3 0/6] acpi,pci: hostbridge hotplug support Taku Izumi
2012-09-18 6:19 ` [PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver Taku Izumi
2012-09-18 6:20 ` [PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges Taku Izumi
2012-09-18 6:21 ` [PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection Taku Izumi
2012-09-18 6:22 ` [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface Taku Izumi
2012-09-19 22:46 ` Bjorn Helgaas
2012-09-20 10:15 ` Taku Izumi
2012-09-21 7:03 ` [PATCH] change signature of walk_root_bridge() function Taku Izumi
2012-09-21 13:24 ` Bjorn Helgaas
2012-09-18 6:23 ` [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation Taku Izumi
2012-09-19 22:03 ` Bjorn Helgaas
2012-09-21 7:14 ` Taku Izumi
2012-09-21 17:57 ` Bjorn Helgaas
2012-09-18 6:24 ` [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection Taku Izumi
2012-09-20 19:32 ` Bjorn Helgaas
2012-09-18 6:25 ` [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function Taku Izumi
2012-09-21 20:09 ` Bjorn Helgaas
2012-09-27 16:48 ` Bjorn Helgaas
2012-09-27 17:23 ` Yinghai Lu
2012-09-27 17:59 ` Yinghai Lu
2012-09-27 18:44 ` Bjorn Helgaas
2012-09-27 20:17 ` Yinghai Lu
2012-09-28 16:07 ` Bjorn Helgaas
2012-09-28 16:19 ` Yinghai Lu
2012-09-28 19:44 ` Bjorn Helgaas
2012-09-28 0:15 ` Taku Izumi
2012-09-28 0:23 ` Bjorn Helgaas
2012-09-28 14:16 ` Bjorn Helgaas
2012-09-28 9:46 ` Taku Izumi
2012-10-30 4:02 ` Bjorn Helgaas [this message]
2012-10-30 17:42 ` (unknown), Yinghai Lu
2012-10-30 17:42 ` Yinghai Lu
2012-10-30 17:42 ` [PATCH 1/8] PCI: Separate out pci_assign_unassigned_bus_resources() Yinghai Lu
2012-10-30 17:42 ` [PATCH 2/8] PCI: Move pci_rescan_bus() back to probe.c Yinghai Lu
2012-10-30 17:42 ` [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res Yinghai Lu
2012-11-02 10:02 ` Taku Izumi
2012-11-02 14:56 ` Yinghai Lu
2012-10-30 17:42 ` [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus Yinghai Lu
2012-10-30 17:42 ` [PATCH 5/8] PCI: Add pci_stop/remove_root_bus() Yinghai Lu
2012-10-30 17:42 ` [PATCH 6/8] PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus Yinghai Lu
2012-10-30 17:42 ` [PATCH 7/8] PCI, ACPI: delete root bus prt during hot remove path Yinghai Lu
2012-10-30 17:42 ` [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order Yinghai Lu
2012-11-02 0:17 ` Rafael J. Wysocki
2012-11-05 22:27 ` Re: Bjorn Helgaas
2012-11-05 22:49 ` Re: Yinghai Lu
2012-11-06 5:03 ` Taku Izumi
2012-11-06 5:03 ` RE: Taku Izumi
2012-10-31 8:26 ` [PATCH v3,RESEND 7/8] ACPI, PCI: add hostbridge removal function Taku Izumi
2012-10-31 8:27 ` [PATCH v3,RESEND 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge Taku Izumi
2012-11-04 4:39 ` [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus Yinghai Lu
2012-11-04 4:39 ` [PATCH 1/8] PCI, x86: Separate out pcibios_allocate_bridge_resources() Yinghai Lu
2012-11-04 4:39 ` [PATCH 2/8] PCI, x86: Separate out pcibios_allocate_dev_resources() Yinghai Lu
2012-11-04 4:39 ` [PATCH 3/8] PCI, x86: Let pcibios_allocate_bus_resources() take bus instead Yinghai Lu
2012-11-04 4:39 ` [PATCH 4/8] PCI, x86: Separate out rom resource claim Yinghai Lu
2012-11-04 4:39 ` [PATCH 5/8] PCI, x86: Add pcibios_fw_addr_done Yinghai Lu
2012-11-04 4:39 ` [PATCH 6/8] PCI, x86: Remove __init for hw/fw allocated functions Yinghai Lu
2012-11-04 4:39 ` [PATCH 7/8] PCI, x86: Claim FW allocated resources in hot add path Yinghai Lu
2012-11-04 4:39 ` [PATCH 8/8] PCI, ACPI: reserve fw allocated resource for hot added root bus Yinghai Lu
2012-12-07 7:15 ` [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add " Yinghai Lu
2013-01-07 23:49 ` Bjorn Helgaas
2013-01-08 17:57 ` Bjorn Helgaas
2013-01-08 18:27 ` Yinghai Lu
2013-01-09 17:35 ` Bjorn Helgaas
2013-01-09 17:53 ` Yinghai Lu
2013-01-09 18:39 ` Bjorn Helgaas
2013-01-09 19:01 ` Yinghai Lu
2013-01-09 20:10 ` Rafael J. Wysocki
2013-01-10 0:34 ` Bjorn Helgaas
2013-01-10 13:07 ` Rafael J. Wysocki
2013-01-10 14:49 ` Bjorn Helgaas
2013-01-09 20:59 ` Benjamin Herrenschmidt
2013-07-02 21:31 ` Bjorn Helgaas
2013-07-02 22:55 ` Yinghai Lu
2012-09-18 6:26 ` [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge Taku Izumi
2012-09-20 5:41 ` Yinghai Lu
2012-09-23 22:48 ` Yinghai Lu
2012-09-28 9:48 ` Taku Izumi
2012-09-18 18:41 ` [PATCH v3 0/6] acpi,pci: hostbridge hotplug support Yinghai Lu
2012-09-18 18:57 ` Yinghai Lu
2012-09-19 4:53 ` Taku Izumi
2012-09-20 6:19 ` Yinghai Lu
2012-09-19 0:50 ` Jiang Liu
2012-09-19 0:50 ` Jiang Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121030040237.GA10472@google.com \
--to=bhelgaas@google.com \
--cc=izumi.taku@jp.fujitsu.com \
--cc=jiang.liu@huawei.com \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.