| Message ID | 1358525267-14268-6-git-send-email-jiang.liu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Saturday, January 19, 2013 12:07:43 AM Jiang Liu wrote: > Currently the pci_slot driver doesn't update PCI slot information > when PCI device hotplug event happens, which may cause memory leak > and returning stale information to user. > > By hooking PCI bus notifications, this patch unifies the way to > create/destroy PCI slots when: > 1) create PCI hierarchy at boot time > 2) create PCI hierarchy for PCI root bridge hot-adding > 3) create PCI hierarchy for PCI device hotplug hot-adding > 4) destroy PCI hierarchy for PCI root bridge hot-removal > 4) destroy PCI hierarchy for PCI device hot-removal This patch also removes some code which apparently is not necessary any more. It would be good to write in the changelog _why_ that code isn't necessary. Apart from this, looks good. Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > Signed-off-by: Yijing Wang <wangyijing@huawei.com> > --- > drivers/acpi/pci_slot.c | 197 +++++++++++++---------------------------------- > 1 file changed, 54 insertions(+), 143 deletions(-) > > diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c > index a7d7e77..2b38f4d 100644 > --- a/drivers/acpi/pci_slot.c > +++ b/drivers/acpi/pci_slot.c > @@ -9,6 +9,9 @@ > * Copyright (C) 2007-2008 Hewlett-Packard Development Company, L.P. > * Alex Chiang <achiang@hp.com> > * > + * Copyright (C) 2013 Huawei Tech. Co., Ltd. > + * Jiang Liu <jiang.liu@huawei.com> > + * > * This program is free software; you can redistribute it and/or modify it > * under the terms and conditions of the GNU General Public License, > * version 2, as published by the Free Software Foundation. > @@ -28,10 +31,9 @@ > #include <linux/init.h> > #include <linux/slab.h> > #include <linux/types.h> > +#include <linux/list.h> > #include <linux/pci.h> > #include <linux/acpi.h> > -#include <acpi/acpi_bus.h> > -#include <acpi/acpi_drivers.h> > #include <linux/dmi.h> > > static bool debug; > @@ -62,20 +64,12 @@ ACPI_MODULE_NAME("pci_slot"); > #define SLOT_NAME_SIZE 21 /* Inspired by #define in acpiphp.h */ > > struct acpi_pci_slot { > - acpi_handle root_handle; /* handle of the root bridge */ > struct pci_slot *pci_slot; /* corresponding pci_slot */ > struct list_head list; /* node in the list of slots */ > }; > > -static int acpi_pci_slot_add(struct acpi_pci_root *root); > -static void acpi_pci_slot_remove(struct acpi_pci_root *root); > - > static LIST_HEAD(slot_list); > static DEFINE_MUTEX(slot_list_lock); > -static struct acpi_pci_driver acpi_pci_slot_driver = { > - .add = acpi_pci_slot_add, > - .remove = acpi_pci_slot_remove, > -}; > > static int > check_slot(acpi_handle handle, unsigned long long *sun) > @@ -114,21 +108,8 @@ out: > return device; > } > > -struct callback_args { > - acpi_walk_callback user_function; /* only for walk_p2p_bridge */ > - struct pci_bus *pci_bus; > - acpi_handle root_handle; > -}; > - > /* > - * register_slot > - * > - * Called once for each SxFy object in the namespace. Don't worry about > - * calling pci_create_slot multiple times for the same pci_bus:device, > - * since each subsequent call simply bumps the refcount on the pci_slot. > - * > - * The number of calls to pci_destroy_slot from unregister_slot is > - * symmetrical. > + * Check whether handle has an associated slot and create PCI slot if it has. > */ > static acpi_status > register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) > @@ -138,13 +119,23 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) > char name[SLOT_NAME_SIZE]; > struct acpi_pci_slot *slot; > struct pci_slot *pci_slot; > - struct callback_args *parent_context = context; > - struct pci_bus *pci_bus = parent_context->pci_bus; > + struct pci_bus *pci_bus = context; > > device = check_slot(handle, &sun); > if (device < 0) > return AE_OK; > > + /* > + * There may be multiple PCI functions associated with the same slot. > + * Check whether PCI slot has already been created for this PCI device. > + */ > + list_for_each_entry(slot, &slot_list, list) { > + pci_slot = slot->pci_slot; > + if (pci_slot->bus == pci_bus && pci_slot->number == device) { > + return AE_OK; > + } > + } > + > slot = kmalloc(sizeof(*slot), GFP_KERNEL); > if (!slot) { > err("%s: cannot allocate memory\n", __func__); > @@ -159,12 +150,9 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) > return AE_OK; > } > > - slot->root_handle = parent_context->root_handle; > slot->pci_slot = pci_slot; > INIT_LIST_HEAD(&slot->list); > - mutex_lock(&slot_list_lock); > list_add(&slot->list, &slot_list); > - mutex_unlock(&slot_list_lock); > > get_device(&pci_bus->dev); > > @@ -174,137 +162,60 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) > return AE_OK; > } > > -/* > - * walk_p2p_bridge - discover and walk p2p bridges > - * @handle: points to an acpi_pci_root > - * @context: p2p_bridge_context pointer > - * > - * Note that when we call ourselves recursively, we pass a different > - * value of pci_bus in the child_context. > - */ > -static acpi_status > -walk_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv) > -{ > - int device, function; > - unsigned long long adr; > - acpi_status status; > - acpi_handle dummy_handle; > - acpi_walk_callback user_function; > - > - struct pci_dev *dev; > - struct pci_bus *pci_bus; > - struct callback_args child_context; > - struct callback_args *parent_context = context; > - > - pci_bus = parent_context->pci_bus; > - user_function = parent_context->user_function; > - > - status = acpi_get_handle(handle, "_ADR", &dummy_handle); > - if (ACPI_FAILURE(status)) > - return AE_OK; > - > - status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr); > - if (ACPI_FAILURE(status)) > - return AE_OK; > - > - device = (adr >> 16) & 0xffff; > - function = adr & 0xffff; > - > - dev = pci_get_slot(pci_bus, PCI_DEVFN(device, function)); > - if (!dev || !dev->subordinate) > - goto out; > - > - child_context.pci_bus = dev->subordinate; > - child_context.user_function = user_function; > - child_context.root_handle = parent_context->root_handle; > - > - dbg("p2p bridge walk, pci_bus = %x\n", dev->subordinate->number); > - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, > - user_function, NULL, &child_context, NULL); > - if (ACPI_FAILURE(status)) > - goto out; > - > - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, > - walk_p2p_bridge, NULL, &child_context, NULL); > -out: > - pci_dev_put(dev); > - return AE_OK; > -} > - > -/* > - * walk_root_bridge - generic root bridge walker > - * @root: poiner of an acpi_pci_root > - * @user_function: user callback for slot objects > - * > - * Call user_function for all objects underneath this root bridge. > - * Walk p2p bridges underneath us and call user_function on those too. > - */ > -static int > -walk_root_bridge(struct acpi_pci_root *root, acpi_walk_callback user_function) > +static void acpi_pci_slot_notify_add(struct pci_bus *bus) > { > - acpi_status status; > - acpi_handle handle = root->device->handle; > - struct pci_bus *pci_bus = root->bus; > - struct callback_args context; > - > - context.pci_bus = pci_bus; > - context.user_function = user_function; > - context.root_handle = handle; > - > - dbg("root bridge walk, pci_bus = %x\n", pci_bus->number); > - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, > - user_function, NULL, &context, NULL); > - if (ACPI_FAILURE(status)) > - return status; > - > - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, > - walk_p2p_bridge, NULL, &context, NULL); > - if (ACPI_FAILURE(status)) > - err("%s: walk_p2p_bridge failure - %d\n", __func__, status); > - > - return status; > -} > + acpi_handle handle = NULL; > > -/* > - * acpi_pci_slot_add > - * @handle: points to an acpi_pci_root > - */ > -static int > -acpi_pci_slot_add(struct acpi_pci_root *root) > -{ > - acpi_status status; > - > - status = walk_root_bridge(root, register_slot); > - if (ACPI_FAILURE(status)) > - err("%s: register_slot failure - %d\n", __func__, status); > + if (bus->bridge) > + handle = DEVICE_ACPI_HANDLE(bus->bridge); > + if (!handle) > + return; > > - return status; > + mutex_lock(&slot_list_lock); > + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1, > + register_slot, NULL, bus, NULL); > + mutex_unlock(&slot_list_lock); > } > > -/* > - * acpi_pci_slot_remove > - * @handle: points to an acpi_pci_root > - */ > -static void > -acpi_pci_slot_remove(struct acpi_pci_root *root) > +static void acpi_pci_slot_notify_del(struct pci_bus *bus) > { > struct acpi_pci_slot *slot, *tmp; > - struct pci_bus *pbus; > - acpi_handle handle = root->device->handle; > > mutex_lock(&slot_list_lock); > list_for_each_entry_safe(slot, tmp, &slot_list, list) { > - if (slot->root_handle == handle) { > + if (slot->pci_slot->bus == bus) { > list_del(&slot->list); > - pbus = slot->pci_slot->bus; > pci_destroy_slot(slot->pci_slot); > - put_device(&pbus->dev); > + put_device(&bus->dev); > kfree(slot); > } > } > mutex_unlock(&slot_list_lock); > } > > +static int acpi_pci_slot_notify_fn(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + struct pci_bus *bus = data; > + > + switch (event) { > + case PCI_NOTIFY_POST_BUS_ADD: > + acpi_pci_slot_notify_add(bus); > + break; > + case PCI_NOTIFY_PRE_BUS_DEL: > + acpi_pci_slot_notify_del(bus); > + break; > + default: > + return NOTIFY_DONE; > + } > + > + return NOTIFY_OK; > +} > + > +static struct notifier_block acpi_pci_slot_notifier = { > + .notifier_call = &acpi_pci_slot_notify_fn, > +}; > + > static int do_sta_before_sun(const struct dmi_system_id *d) > { > info("%s detected: will evaluate _STA before calling _SUN\n", d->ident); > @@ -333,5 +244,5 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { > void __init acpi_pci_slot_init(void) > { > dmi_check_system(acpi_pci_slot_dmi_table); > - acpi_pci_register_driver(&acpi_pci_slot_driver); > + pci_bus_register_notifier(&acpi_pci_slot_notifier); > } >
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c index a7d7e77..2b38f4d 100644 --- a/drivers/acpi/pci_slot.c +++ b/drivers/acpi/pci_slot.c @@ -9,6 +9,9 @@ * Copyright (C) 2007-2008 Hewlett-Packard Development Company, L.P. * Alex Chiang <achiang@hp.com> * + * Copyright (C) 2013 Huawei Tech. Co., Ltd. + * Jiang Liu <jiang.liu@huawei.com> + * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, * version 2, as published by the Free Software Foundation. @@ -28,10 +31,9 @@ #include <linux/init.h> #include <linux/slab.h> #include <linux/types.h> +#include <linux/list.h> #include <linux/pci.h> #include <linux/acpi.h> -#include <acpi/acpi_bus.h> -#include <acpi/acpi_drivers.h> #include <linux/dmi.h> static bool debug; @@ -62,20 +64,12 @@ ACPI_MODULE_NAME("pci_slot"); #define SLOT_NAME_SIZE 21 /* Inspired by #define in acpiphp.h */ struct acpi_pci_slot { - acpi_handle root_handle; /* handle of the root bridge */ struct pci_slot *pci_slot; /* corresponding pci_slot */ struct list_head list; /* node in the list of slots */ }; -static int acpi_pci_slot_add(struct acpi_pci_root *root); -static void acpi_pci_slot_remove(struct acpi_pci_root *root); - static LIST_HEAD(slot_list); static DEFINE_MUTEX(slot_list_lock); -static struct acpi_pci_driver acpi_pci_slot_driver = { - .add = acpi_pci_slot_add, - .remove = acpi_pci_slot_remove, -}; static int check_slot(acpi_handle handle, unsigned long long *sun) @@ -114,21 +108,8 @@ out: return device; } -struct callback_args { - acpi_walk_callback user_function; /* only for walk_p2p_bridge */ - struct pci_bus *pci_bus; - acpi_handle root_handle; -}; - /* - * register_slot - * - * Called once for each SxFy object in the namespace. Don't worry about - * calling pci_create_slot multiple times for the same pci_bus:device, - * since each subsequent call simply bumps the refcount on the pci_slot. - * - * The number of calls to pci_destroy_slot from unregister_slot is - * symmetrical. + * Check whether handle has an associated slot and create PCI slot if it has. */ static acpi_status register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) @@ -138,13 +119,23 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) char name[SLOT_NAME_SIZE]; struct acpi_pci_slot *slot; struct pci_slot *pci_slot; - struct callback_args *parent_context = context; - struct pci_bus *pci_bus = parent_context->pci_bus; + struct pci_bus *pci_bus = context; device = check_slot(handle, &sun); if (device < 0) return AE_OK; + /* + * There may be multiple PCI functions associated with the same slot. + * Check whether PCI slot has already been created for this PCI device. + */ + list_for_each_entry(slot, &slot_list, list) { + pci_slot = slot->pci_slot; + if (pci_slot->bus == pci_bus && pci_slot->number == device) { + return AE_OK; + } + } + slot = kmalloc(sizeof(*slot), GFP_KERNEL); if (!slot) { err("%s: cannot allocate memory\n", __func__); @@ -159,12 +150,9 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) return AE_OK; } - slot->root_handle = parent_context->root_handle; slot->pci_slot = pci_slot; INIT_LIST_HEAD(&slot->list); - mutex_lock(&slot_list_lock); list_add(&slot->list, &slot_list); - mutex_unlock(&slot_list_lock); get_device(&pci_bus->dev); @@ -174,137 +162,60 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) return AE_OK; } -/* - * walk_p2p_bridge - discover and walk p2p bridges - * @handle: points to an acpi_pci_root - * @context: p2p_bridge_context pointer - * - * Note that when we call ourselves recursively, we pass a different - * value of pci_bus in the child_context. - */ -static acpi_status -walk_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv) -{ - int device, function; - unsigned long long adr; - acpi_status status; - acpi_handle dummy_handle; - acpi_walk_callback user_function; - - struct pci_dev *dev; - struct pci_bus *pci_bus; - struct callback_args child_context; - struct callback_args *parent_context = context; - - pci_bus = parent_context->pci_bus; - user_function = parent_context->user_function; - - status = acpi_get_handle(handle, "_ADR", &dummy_handle); - if (ACPI_FAILURE(status)) - return AE_OK; - - status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr); - if (ACPI_FAILURE(status)) - return AE_OK; - - device = (adr >> 16) & 0xffff; - function = adr & 0xffff; - - dev = pci_get_slot(pci_bus, PCI_DEVFN(device, function)); - if (!dev || !dev->subordinate) - goto out; - - child_context.pci_bus = dev->subordinate; - child_context.user_function = user_function; - child_context.root_handle = parent_context->root_handle; - - dbg("p2p bridge walk, pci_bus = %x\n", dev->subordinate->number); - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, - user_function, NULL, &child_context, NULL); - if (ACPI_FAILURE(status)) - goto out; - - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, - walk_p2p_bridge, NULL, &child_context, NULL); -out: - pci_dev_put(dev); - return AE_OK; -} - -/* - * walk_root_bridge - generic root bridge walker - * @root: poiner of an acpi_pci_root - * @user_function: user callback for slot objects - * - * Call user_function for all objects underneath this root bridge. - * Walk p2p bridges underneath us and call user_function on those too. - */ -static int -walk_root_bridge(struct acpi_pci_root *root, acpi_walk_callback user_function) +static void acpi_pci_slot_notify_add(struct pci_bus *bus) { - acpi_status status; - acpi_handle handle = root->device->handle; - struct pci_bus *pci_bus = root->bus; - struct callback_args context; - - context.pci_bus = pci_bus; - context.user_function = user_function; - context.root_handle = handle; - - dbg("root bridge walk, pci_bus = %x\n", pci_bus->number); - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, - user_function, NULL, &context, NULL); - if (ACPI_FAILURE(status)) - return status; - - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, - walk_p2p_bridge, NULL, &context, NULL); - if (ACPI_FAILURE(status)) - err("%s: walk_p2p_bridge failure - %d\n", __func__, status); - - return status; -} + acpi_handle handle = NULL; -/* - * acpi_pci_slot_add - * @handle: points to an acpi_pci_root - */ -static int -acpi_pci_slot_add(struct acpi_pci_root *root) -{ - acpi_status status; - - status = walk_root_bridge(root, register_slot); - if (ACPI_FAILURE(status)) - err("%s: register_slot failure - %d\n", __func__, status); + if (bus->bridge) + handle = DEVICE_ACPI_HANDLE(bus->bridge); + if (!handle) + return; - return status; + mutex_lock(&slot_list_lock); + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1, + register_slot, NULL, bus, NULL); + mutex_unlock(&slot_list_lock); } -/* - * acpi_pci_slot_remove - * @handle: points to an acpi_pci_root - */ -static void -acpi_pci_slot_remove(struct acpi_pci_root *root) +static void acpi_pci_slot_notify_del(struct pci_bus *bus) { struct acpi_pci_slot *slot, *tmp; - struct pci_bus *pbus; - acpi_handle handle = root->device->handle; mutex_lock(&slot_list_lock); list_for_each_entry_safe(slot, tmp, &slot_list, list) { - if (slot->root_handle == handle) { + if (slot->pci_slot->bus == bus) { list_del(&slot->list); - pbus = slot->pci_slot->bus; pci_destroy_slot(slot->pci_slot); - put_device(&pbus->dev); + put_device(&bus->dev); kfree(slot); } } mutex_unlock(&slot_list_lock); } +static int acpi_pci_slot_notify_fn(struct notifier_block *nb, + unsigned long event, void *data) +{ + struct pci_bus *bus = data; + + switch (event) { + case PCI_NOTIFY_POST_BUS_ADD: + acpi_pci_slot_notify_add(bus); + break; + case PCI_NOTIFY_PRE_BUS_DEL: + acpi_pci_slot_notify_del(bus); + break; + default: + return NOTIFY_DONE; + } + + return NOTIFY_OK; +} + +static struct notifier_block acpi_pci_slot_notifier = { + .notifier_call = &acpi_pci_slot_notify_fn, +}; + static int do_sta_before_sun(const struct dmi_system_id *d) { info("%s detected: will evaluate _STA before calling _SUN\n", d->ident); @@ -333,5 +244,5 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = { void __init acpi_pci_slot_init(void) { dmi_check_system(acpi_pci_slot_dmi_table); - acpi_pci_register_driver(&acpi_pci_slot_driver); + pci_bus_register_notifier(&acpi_pci_slot_notifier); }