From patchwork Sun Feb 21 00:51:01 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 80992 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter.kernel.org (8.14.3/8.14.3) with ESMTP id o1L0okep006441 for ; Sun, 21 Feb 2010 00:50:47 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754806Ab0BUAua (ORCPT ); Sat, 20 Feb 2010 19:50:30 -0500 Received: from ogre.sisk.pl ([217.79.144.158]:56743 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754452Ab0BUAua (ORCPT ); Sat, 20 Feb 2010 19:50:30 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by ogre.sisk.pl (Postfix) with ESMTP id DF4A1178A1C; Sun, 21 Feb 2010 01:29:53 +0100 (CET) Received: from ogre.sisk.pl ([127.0.0.1]) by localhost (ogre.sisk.pl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 12569-05; Sun, 21 Feb 2010 01:29:46 +0100 (CET) Received: from tosh.localnet (220-bem-13.acn.waw.pl [82.210.184.220]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ogre.sisk.pl (Postfix) with ESMTP id 5E166178A41; Sun, 21 Feb 2010 01:29:46 +0100 (CET) From: "Rafael J. Wysocki" To: ACPI Devel Maling List Subject: [RFC][PATCH] ACPI: Eliminate race conditions related to removing event handlers Date: Sun, 21 Feb 2010 01:51:01 +0100 User-Agent: KMail/1.12.4 (Linux/2.6.33-rc8-rjw; KDE/4.3.5; x86_64; ; ) Cc: Len Brown , "Moore, Robert" , LKML , Matthew Garrett MIME-Version: 1.0 Message-Id: <201002210151.01159.rjw@sisk.pl> X-Virus-Scanned: amavisd-new at ogre.sisk.pl using MkS_Vir for Linux Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter.kernel.org [140.211.167.41]); Sun, 21 Feb 2010 00:50:47 +0000 (UTC) Index: linux-2.6/drivers/acpi/osl.c =================================================================== --- linux-2.6.orig/drivers/acpi/osl.c +++ linux-2.6/drivers/acpi/osl.c @@ -61,6 +61,12 @@ struct acpi_os_dpc { int wait; }; +struct acpi_os_barrier_work { + struct completion run; + struct completion exit; + struct work_struct work; +}; + #ifdef CONFIG_ACPI_CUSTOM_DSDT #include CONFIG_ACPI_CUSTOM_DSDT_FILE #endif @@ -700,28 +706,15 @@ static void acpi_os_execute_deferred(str { struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work); - if (dpc->wait) - acpi_os_wait_events_complete(NULL); + if (dpc->wait) { + flush_workqueue(kacpid_wq); + flush_workqueue(kacpi_notify_wq); + } dpc->function(dpc->context); kfree(dpc); } -/******************************************************************************* - * - * FUNCTION: acpi_os_execute - * - * PARAMETERS: Type - Type of the callback - * Function - Function to be executed - * Context - Function parameters - * - * RETURN: Status - * - * DESCRIPTION: Depending on type, either queues function for deferred execution or - * immediately executes function on a separate thread. - * - ******************************************************************************/ - static acpi_status __acpi_os_execute(acpi_execute_type type, acpi_osd_exec_callback function, void *context, int hp) { @@ -770,6 +763,20 @@ static acpi_status __acpi_os_execute(acp return status; } +/******************************************************************************* + * + * FUNCTION: acpi_os_execute + * + * PARAMETERS: Type - Type of the callback + * Function - Function to be executed + * Context - Function parameters + * + * RETURN: Status + * + * DESCRIPTION: Depending on type, either queues function for deferred execution or + * immediately executes function on a separate thread. + * + ******************************************************************************/ acpi_status acpi_os_execute(acpi_execute_type type, acpi_osd_exec_callback function, void *context) { @@ -783,13 +790,77 @@ acpi_status acpi_os_hotplug_execute(acpi return __acpi_os_execute(0, function, context, 1); } -void acpi_os_wait_events_complete(void *context) +static void acpi_os_barrier_fn(struct work_struct *work) { - flush_workqueue(kacpid_wq); - flush_workqueue(kacpi_notify_wq); + struct acpi_os_barrier_work *barrier; + + barrier = container_of(work, struct acpi_os_barrier_work, work); + complete(&barrier->run); + wait_for_completion(&barrier->exit); + kfree(barrier); +} + +/******************************************************************************* + * + * FUNCTION: acpi_os_add_events_barrier + * + * PARAMETERS: type - Type of events to add the barrier for + * + * RETURN: Context to be passed to acpi_os_remove_events_barrier() or NULL + * on failure. + * + * DESCRIPTION: Ensure that all of the events of given type either have been + * handled before we continue running or will not be handled until + * acpi_os_remove_events_barrier() is run. + * + ******************************************************************************/ +void *acpi_os_add_events_barrier(acpi_execute_type type) +{ + struct acpi_os_barrier_work *barrier; + struct workqueue_struct *queue; + int ret; + + barrier = kmalloc(sizeof(*barrier), GFP_KERNEL); + if (!barrier) + return NULL; + + init_completion(&barrier->run); + init_completion(&barrier->exit); + INIT_WORK(&barrier->work, acpi_os_barrier_fn); + queue = (type == OSL_NOTIFY_HANDLER) ? kacpi_notify_wq : kacpid_wq; + ret = queue_work(queue, &barrier->work); + if (!ret) { + printk(KERN_ERR PREFIX "queue_work() failed!\n"); + kfree(barrier); + return NULL; + } + + wait_for_completion(&barrier->run); + + return barrier; } +EXPORT_SYMBOL(acpi_os_add_events_barrier); + +/******************************************************************************* + * + * FUNCTION: acpi_os_remove_events_barrier + * + * PARAMETERS: context - Information needed to remove the barrier + * + * RETURN: None + * + * DESCRIPTION: Using the context information remove an events barrier added by + * acpi_os_add_events_barrier(). + * + ******************************************************************************/ +void acpi_os_remove_events_barrier(void *context) +{ + struct acpi_os_barrier_work *barrier = context; -EXPORT_SYMBOL(acpi_os_wait_events_complete); + if (barrier) + complete(&barrier->exit); +} +EXPORT_SYMBOL(acpi_os_remove_events_barrier); /* * Allocate the memory for a spinlock and initialize it. Index: linux-2.6/include/acpi/acpiosxf.h =================================================================== --- linux-2.6.orig/include/acpi/acpiosxf.h +++ linux-2.6/include/acpi/acpiosxf.h @@ -194,7 +194,8 @@ acpi_os_execute(acpi_execute_type type, acpi_status acpi_os_hotplug_execute(acpi_osd_exec_callback function, void *context); -void acpi_os_wait_events_complete(void *context); +void *acpi_os_add_events_barrier(acpi_execute_type type); +void acpi_os_remove_events_barrier(void *context); void acpi_os_sleep(acpi_integer milliseconds); Index: linux-2.6/drivers/acpi/acpica/evxface.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxface.c +++ linux-2.6/drivers/acpi/acpica/evxface.c @@ -497,6 +497,7 @@ acpi_remove_notify_handler(acpi_handle d union acpi_operand_object *notify_obj; union acpi_operand_object *obj_desc; struct acpi_namespace_node *node; + void *events_barrier; acpi_status status; ACPI_FUNCTION_TRACE(acpi_remove_notify_handler); @@ -511,11 +512,16 @@ acpi_remove_notify_handler(acpi_handle d /* Make sure all deferred tasks are completed */ - acpi_os_wait_events_complete(NULL); + + events_barrier = acpi_os_add_events_barrier(OSL_NOTIFY_HANDLER); + if (!events_barrier) { + status = AE_ERROR; + goto exit; + } status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); if (ACPI_FAILURE(status)) { - goto exit; + goto release_events; } /* Convert and validate the device handle */ @@ -653,6 +659,8 @@ acpi_remove_notify_handler(acpi_handle d unlock_and_exit: (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); + release_events: + acpi_os_remove_events_barrier(events_barrier); exit: if (ACPI_FAILURE(status)) ACPI_EXCEPTION((AE_INFO, status, "Removing notify handler")); @@ -774,6 +782,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_ { struct acpi_gpe_event_info *gpe_event_info; struct acpi_handler_info *handler; + void *events_barrier; acpi_status status; acpi_cpu_flags flags; @@ -785,9 +794,16 @@ acpi_remove_gpe_handler(acpi_handle gpe_ return_ACPI_STATUS(AE_BAD_PARAMETER); } + /* Make sure all deferred tasks are completed */ + + events_barrier = acpi_os_add_events_barrier(OSL_GPE_HANDLER); + if (!events_barrier) { + return_ACPI_STATUS(AE_ERROR); + } + status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS); if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); + goto release_events; } /* Ensure that we have a valid GPE number */ @@ -813,15 +829,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_ goto unlock_and_exit; } - /* Make sure all deferred tasks are completed */ - - (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); - acpi_os_wait_events_complete(NULL); - status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS); - if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); - } - /* Remove the handler */ flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); @@ -842,6 +849,8 @@ acpi_remove_gpe_handler(acpi_handle gpe_ unlock_and_exit: (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); + release_events: + acpi_os_remove_events_barrier(events_barrier); return_ACPI_STATUS(status); }