From patchwork Thu Jul 11 23:40:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 258647 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 6B1C62C0335 for ; Fri, 12 Jul 2013 10:01:27 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756883Ab3GLABI (ORCPT ); Thu, 11 Jul 2013 20:01:08 -0400 Received: from hydra.sisk.pl ([212.160.235.94]:39082 "EHLO hydra.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756813Ab3GKX7I (ORCPT ); Thu, 11 Jul 2013 19:59:08 -0400 Received: from vostro.rjw.lan (afcc173.neoplus.adsl.tpnet.pl [95.49.54.173]) by hydra.sisk.pl (Postfix) with ESMTPSA id 79031E3DC1; Fri, 12 Jul 2013 01:54:56 +0200 (CEST) From: "Rafael J. Wysocki" To: ACPI Devel Maling List Cc: Bjorn Helgaas , LKML , Linux PCI , Yinghai Lu , Jiang Liu , Mika Westerberg Subject: [RFC][PATCH 5/30] ACPI / hotplug / PCI: Unified notify handler for hotplug events Date: Fri, 12 Jul 2013 01:40:57 +0200 Message-ID: <5248420.dTt3AhzoPf@vostro.rjw.lan> User-Agent: KMail/4.9.5 (Linux/3.10.0+; KDE/4.9.5; x86_64; ; ) In-Reply-To: <3718119.FLASu5DBx8@vostro.rjw.lan> References: <26431283.HJCKsss0rt@vostro.rjw.lan> <3718119.FLASu5DBx8@vostro.rjw.lan> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org From: Rafael J. Wysocki Using the hotplug context objects introduced previously rework the ACPI-based PCI hotplug (ACPIPHP) core code so that all notifications for ACPI device objects corresponding to the hotplug PCI devices are handled by one function, handle_hotplug_event(), which recognizes whether it has to handle a bridge or a function. In addition to code size reduction it allows some ugly pieces of code where notify handlers have to be uninstalled and installed again to go away. Moreover, it fixes a theoretically possible race between handle_hotplug_event() and free_bridge() tearing down data structures for the same handle. Signed-off-by: Rafael J. Wysocki --- drivers/pci/hotplug/acpiphp.h | 1 drivers/pci/hotplug/acpiphp_glue.c | 133 ++++++++++++++----------------------- 2 files changed, 54 insertions(+), 80 deletions(-) -- 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 Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c =================================================================== --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c @@ -59,11 +59,10 @@ static DEFINE_MUTEX(acpiphp_context_lock #define MY_NAME "acpiphp_glue" -static void handle_hotplug_event_bridge (acpi_handle, u32, void *); +static void handle_hotplug_event(acpi_handle handle, u32 type, void *data); static void acpiphp_sanitize_bus(struct pci_bus *bus); static void acpiphp_set_hpp_values(struct pci_bus *bus); static void hotplug_event_func(acpi_handle handle, u32 type, void *context); -static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context); static void free_bridge(struct kref *kref); /* callback routine to check for the existence of a pci dock device */ @@ -179,13 +178,13 @@ static void free_bridge(struct kref *kre kfree(slot); } + context = bridge->context; /* Release the reference acquired by acpiphp_enumerate_slots(). */ - if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) + if (context->handler_for_func) put_bridge(bridge->func->slot->bridge); put_device(&bridge->pci_bus->dev); pci_dev_put(bridge->pci_dev); - context = bridge->context; context->bridge = NULL; acpiphp_put_context(context); kfree(bridge); @@ -424,12 +423,12 @@ static acpi_status register_slot(acpi_ha /* install notify handler */ if (!(newfunc->flags & FUNC_HAS_DCK)) { - status = acpi_install_notify_handler(handle, - ACPI_SYSTEM_NOTIFY, - handle_hotplug_event_func, - newfunc); - - if (ACPI_FAILURE(status)) + status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, + handle_hotplug_event, + context); + if (ACPI_SUCCESS(status)) + context->handler_for_func = true; + else err("failed to register interrupt notify handler\n"); } @@ -486,23 +485,13 @@ static void cleanup_bridge(struct acpiph acpi_status status; acpi_handle handle = bridge->handle; - if (!pci_is_root_bus(bridge->pci_bus)) { - status = acpi_remove_notify_handler(handle, - ACPI_SYSTEM_NOTIFY, - handle_hotplug_event_bridge); + if (!bridge->context->handler_for_func) { + status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY, + handle_hotplug_event); if (ACPI_FAILURE(status)) err("failed to remove notify handler\n"); } - if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) { - status = acpi_install_notify_handler(bridge->func->handle, - ACPI_SYSTEM_NOTIFY, - handle_hotplug_event_func, - bridge->func); - if (ACPI_FAILURE(status)) - err("failed to install interrupt notify handler\n"); - } - list_for_each_entry(slot, &bridge->slots, node) { list_for_each_entry(func, &slot->funcs, sibling) { if (is_dock_device(func->handle)) { @@ -510,9 +499,10 @@ static void cleanup_bridge(struct acpiph unregister_dock_notifier(&func->nb); } if (!(func->flags & FUNC_HAS_DCK)) { + func->context->handler_for_func = false; status = acpi_remove_notify_handler(func->handle, - ACPI_SYSTEM_NOTIFY, - handle_hotplug_event_func); + ACPI_SYSTEM_NOTIFY, + handle_hotplug_event); if (ACPI_FAILURE(status)) err("failed to remove notify handler\n"); } @@ -1093,31 +1083,6 @@ static void _handle_hotplug_event_bridge put_bridge(bridge); } -/** - * handle_hotplug_event_bridge - handle ACPI event on bridges - * @handle: Notify()'ed acpi_handle - * @type: Notify code - * @context: pointer to acpiphp_bridge structure - * - * Handles ACPI event notification on {host,p2p} bridges. - */ -static void handle_hotplug_event_bridge(acpi_handle handle, u32 type, - void *context) -{ - struct acpiphp_bridge *bridge = context; - - /* - * Currently the code adds all hotplug events to the kacpid_wq - * queue when it should add hotplug events to the kacpi_hotplug_wq. - * The proper way to fix this is to reorganize the code so that - * drivers (dock, etc.) do not call acpi_os_execute(), etc. - * For now just re-add this work to the kacpi_hotplug_wq so we - * don't deadlock on hotplug actions. - */ - get_bridge(bridge); - alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_bridge); -} - static void hotplug_event_func(acpi_handle handle, u32 type, void *context) { struct acpiphp_func *func = context; @@ -1175,17 +1140,33 @@ static void _handle_hotplug_event_func(s } /** - * handle_hotplug_event_func - handle ACPI event on functions (i.e. slots) + * handle_hotplug_event - handle ACPI hotplug event * @handle: Notify()'ed acpi_handle * @type: Notify code - * @context: pointer to acpiphp_func structure + * @data: pointer to acpiphp_context structure * * Handles ACPI event notification on slots. */ -static void handle_hotplug_event_func(acpi_handle handle, u32 type, - void *context) +static void handle_hotplug_event(acpi_handle handle, u32 type, void *data) { - struct acpiphp_func *func = context; + struct acpiphp_context *context; + void (*work_func)(struct work_struct *work) = NULL; + + mutex_lock(&acpiphp_context_lock); + context = acpiphp_get_context(handle); + if (context) { + if (context->bridge) { + get_bridge(context->bridge); + data = context->bridge; + work_func = _handle_hotplug_event_bridge; + } else if (context->func) { + get_bridge(context->func->slot->bridge); + data = context->func; + work_func = _handle_hotplug_event_func; + } + acpiphp_put_context(context); + } + mutex_unlock(&acpiphp_context_lock); /* * Currently the code adds all hotplug events to the kacpid_wq @@ -1195,8 +1176,8 @@ static void handle_hotplug_event_func(ac * For now just re-add this work to the kacpi_hotplug_wq so we * don't deadlock on hotplug actions. */ - get_bridge(func->slot->bridge); - alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_func); + if (work_func) + alloc_acpi_hp_work(handle, type, data, work_func); } /* @@ -1267,33 +1248,25 @@ void acpiphp_enumerate_slots(struct pci_ if (pci_is_root_bus(bridge->pci_bus)) return; + status = acpi_get_handle(bridge->handle, "_EJ0", &handle); + if (ACPI_SUCCESS(status)) { + dbg("found ejectable p2p bridge\n"); + bridge->flags |= BRIDGE_HAS_EJ0; + } + if (context->handler_for_func) { + /* Notify handler already installed. */ + bridge->func = context->func; + get_bridge(context->func->slot->bridge); + return; + } + /* install notify handler for P2P bridges */ status = acpi_install_notify_handler(bridge->handle, ACPI_SYSTEM_NOTIFY, - handle_hotplug_event_bridge, - bridge); - if (ACPI_FAILURE(status)) { - acpi_handle_err(bridge->handle, - "failed to register notify handler\n"); - goto err; - } - status = acpi_get_handle(bridge->handle, "_EJ0", &handle); - if (ACPI_FAILURE(status)) + handle_hotplug_event, NULL); + if (ACPI_SUCCESS(status)) return; - dbg("found ejectable p2p bridge\n"); - bridge->flags |= BRIDGE_HAS_EJ0; - if (context->func) { - get_bridge(context->func->slot->bridge); - bridge->func = context->func; - handle = context->handle; - WARN_ON(bridge->handle != handle); - status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY, - handle_hotplug_event_func); - if (ACPI_FAILURE(status)) - acpi_handle_err(handle, - "failed to remove notify handler\n"); - } - return; + acpi_handle_err(bridge->handle, "failed to register notify handler\n"); err: cleanup_bridge(bridge); Index: linux-pm/drivers/pci/hotplug/acpiphp.h =================================================================== --- linux-pm.orig/drivers/pci/hotplug/acpiphp.h +++ linux-pm/drivers/pci/hotplug/acpiphp.h @@ -137,6 +137,7 @@ struct acpiphp_context { struct acpiphp_func *func; struct acpiphp_bridge *bridge; unsigned int refcount; + bool handler_for_func; }; /*