From c0d127b56937c3e72c2b1819161d2f6718eee877 Mon Sep 17 00:00:00 2001 From: Alexey Starikovskiy Date: Thu, 15 Feb 2007 16:12:23 -0500 Subject: ACPICA: fix AML mutex re-entrancy ACPI AML supports "serialized" methods which are protected by an implicit mutex. The mutex is re-entrant for that AML thread to allow recursion. However, Linux implements notify() by creating a new AML thread. So for systems where notify() re-enters a serialized method, deadlock results. The fix is to use the Linux thread_id as the key to allowing re-entrancy, not the AML thread pointer. http://bugzilla.kernel.org/show_bug.cgi?id=5534 Signed-off-by: Alexey Starikovskiy Signed-off-by: Len Brown --- drivers/acpi/dispatcher/dsmethod.c | 12 +++++------- drivers/acpi/executer/exdump.c | 2 +- drivers/acpi/executer/exmutex.c | 36 ++++++++++++++++-------------------- drivers/acpi/utilities/utdelete.c | 1 - 4 files changed, 22 insertions(+), 29 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/dispatcher/dsmethod.c b/drivers/acpi/dispatcher/dsmethod.c index 1cbe619..1683e5c 100644 --- a/drivers/acpi/dispatcher/dsmethod.c +++ b/drivers/acpi/dispatcher/dsmethod.c @@ -231,10 +231,8 @@ acpi_ds_begin_method_execution(struct acpi_namespace_node *method_node, * Obtain the method mutex if necessary. Do not acquire mutex for a * recursive call. */ - if (!walk_state || - !obj_desc->method.mutex->mutex.owner_thread || - (walk_state->thread != - obj_desc->method.mutex->mutex.owner_thread)) { + if (acpi_os_get_thread_id() != + obj_desc->method.mutex->mutex.owner_thread_id) { /* * Acquire the method mutex. This releases the interpreter if we * block (and reacquires it before it returns) @@ -248,14 +246,14 @@ acpi_ds_begin_method_execution(struct acpi_namespace_node *method_node, } /* Update the mutex and walk info and save the original sync_level */ + obj_desc->method.mutex->mutex.owner_thread_id = + acpi_os_get_thread_id(); if (walk_state) { obj_desc->method.mutex->mutex. original_sync_level = walk_state->thread->current_sync_level; - obj_desc->method.mutex->mutex.owner_thread = - walk_state->thread; walk_state->thread->current_sync_level = obj_desc->method.sync_level; } else { @@ -569,7 +567,7 @@ acpi_ds_terminate_control_method(union acpi_operand_object *method_desc, acpi_os_release_mutex(method_desc->method.mutex->mutex. os_mutex); - method_desc->method.mutex->mutex.owner_thread = NULL; + method_desc->method.mutex->mutex.owner_thread_id = ACPI_MUTEX_NOT_ACQUIRED; } } diff --git a/drivers/acpi/executer/exdump.c b/drivers/acpi/executer/exdump.c index 68d283f..1a73c14 100644 --- a/drivers/acpi/executer/exdump.c +++ b/drivers/acpi/executer/exdump.c @@ -134,7 +134,7 @@ static struct acpi_exdump_info acpi_ex_dump_method[8] = { static struct acpi_exdump_info acpi_ex_dump_mutex[5] = { {ACPI_EXD_INIT, ACPI_EXD_TABLE_SIZE(acpi_ex_dump_mutex), NULL}, {ACPI_EXD_UINT8, ACPI_EXD_OFFSET(mutex.sync_level), "Sync Level"}, - {ACPI_EXD_POINTER, ACPI_EXD_OFFSET(mutex.owner_thread), "Owner Thread"}, + {ACPI_EXD_POINTER, ACPI_EXD_OFFSET(mutex.owner_thread_id), "Owner Thread"}, {ACPI_EXD_UINT16, ACPI_EXD_OFFSET(mutex.acquisition_depth), "Acquire Depth"}, {ACPI_EXD_POINTER, ACPI_EXD_OFFSET(mutex.os_mutex), "OsMutex"} diff --git a/drivers/acpi/executer/exmutex.c b/drivers/acpi/executer/exmutex.c index 5101bad..4eb883b 100644 --- a/drivers/acpi/executer/exmutex.c +++ b/drivers/acpi/executer/exmutex.c @@ -66,10 +66,9 @@ acpi_ex_link_mutex(union acpi_operand_object *obj_desc, * ******************************************************************************/ -void acpi_ex_unlink_mutex(union acpi_operand_object *obj_desc) +void acpi_ex_unlink_mutex(union acpi_operand_object *obj_desc, + struct acpi_thread_state *thread) { - struct acpi_thread_state *thread = obj_desc->mutex.owner_thread; - if (!thread) { return; } @@ -174,16 +173,13 @@ acpi_ex_acquire_mutex(union acpi_operand_object *time_desc, /* Support for multiple acquires by the owning thread */ - if (obj_desc->mutex.owner_thread) { - if (obj_desc->mutex.owner_thread->thread_id == - walk_state->thread->thread_id) { - /* - * The mutex is already owned by this thread, just increment the - * acquisition depth - */ - obj_desc->mutex.acquisition_depth++; - return_ACPI_STATUS(AE_OK); - } + if (obj_desc->mutex.owner_thread_id == acpi_os_get_thread_id()) { + /* + * The mutex is already owned by this thread, just increment the + * acquisition depth + */ + obj_desc->mutex.acquisition_depth++; + return_ACPI_STATUS(AE_OK); } /* Acquire the mutex, wait if necessary. Special case for Global Lock */ @@ -206,7 +202,7 @@ acpi_ex_acquire_mutex(union acpi_operand_object *time_desc, /* Have the mutex: update mutex and walk info and save the sync_level */ - obj_desc->mutex.owner_thread = walk_state->thread; + obj_desc->mutex.owner_thread_id = acpi_os_get_thread_id(); obj_desc->mutex.acquisition_depth = 1; obj_desc->mutex.original_sync_level = walk_state->thread->current_sync_level; @@ -246,7 +242,7 @@ acpi_ex_release_mutex(union acpi_operand_object *obj_desc, /* The mutex must have been previously acquired in order to release it */ - if (!obj_desc->mutex.owner_thread) { + if (!obj_desc->mutex.owner_thread_id) { ACPI_ERROR((AE_INFO, "Cannot release Mutex [%4.4s], not acquired", acpi_ut_get_node_name(obj_desc->mutex.node))); @@ -266,14 +262,14 @@ acpi_ex_release_mutex(union acpi_operand_object *obj_desc, * The Mutex is owned, but this thread must be the owner. * Special case for Global Lock, any thread can release */ - if ((obj_desc->mutex.owner_thread->thread_id != + if ((obj_desc->mutex.owner_thread_id != walk_state->thread->thread_id) && (obj_desc->mutex.os_mutex != acpi_gbl_global_lock_mutex)) { ACPI_ERROR((AE_INFO, "Thread %lX cannot release Mutex [%4.4s] acquired by thread %lX", (unsigned long)walk_state->thread->thread_id, acpi_ut_get_node_name(obj_desc->mutex.node), - (unsigned long)obj_desc->mutex.owner_thread->thread_id)); + (unsigned long)obj_desc->mutex.owner_thread_id)); return_ACPI_STATUS(AE_AML_NOT_OWNER); } @@ -300,7 +296,7 @@ acpi_ex_release_mutex(union acpi_operand_object *obj_desc, /* Unlink the mutex from the owner's list */ - acpi_ex_unlink_mutex(obj_desc); + acpi_ex_unlink_mutex(obj_desc, walk_state->thread); /* Release the mutex, special case for Global Lock */ @@ -312,7 +308,7 @@ acpi_ex_release_mutex(union acpi_operand_object *obj_desc, /* Update the mutex and restore sync_level */ - obj_desc->mutex.owner_thread = NULL; + obj_desc->mutex.owner_thread_id = ACPI_MUTEX_NOT_ACQUIRED; walk_state->thread->current_sync_level = obj_desc->mutex.original_sync_level; @@ -367,7 +363,7 @@ void acpi_ex_release_all_mutexes(struct acpi_thread_state *thread) /* Mark mutex unowned */ - obj_desc->mutex.owner_thread = NULL; + obj_desc->mutex.owner_thread_id = ACPI_MUTEX_NOT_ACQUIRED; /* Update Thread sync_level (Last mutex is the important one) */ diff --git a/drivers/acpi/utilities/utdelete.c b/drivers/acpi/utilities/utdelete.c index f777ceb..673a0ca 100644 --- a/drivers/acpi/utilities/utdelete.c +++ b/drivers/acpi/utilities/utdelete.c @@ -170,7 +170,6 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object) acpi_os_delete_mutex(object->mutex.os_mutex); acpi_gbl_global_lock_mutex = NULL; } else { - acpi_ex_unlink_mutex(object); acpi_os_delete_mutex(object->mutex.os_mutex); } break; -- cgit v1.1 From 5f7748cf91558a5026ded5be93c5bf6c1ac34edf Mon Sep 17 00:00:00 2001 From: Alexey Starikovskiy Date: Thu, 15 Feb 2007 16:13:51 -0500 Subject: Execute AML Notify() requests on stack. HP nx6125/nx6325/... machines have a _GPE handler with an infinite loop sending Notify() events to different ACPI subsystems. The notify handler in the ACPI thermal driver is a C-routine, which may invoke the ACPI interpreter again to get access to some ACPI variables such as temperature. (acpi_evaluate_xxx) On these HP machines such an evaluation changes state of an ASL variable and lets the loop above break. In the current ACPI implementation, Notify requests are being deferred to the same kacpid workqueue on which the above GPE handler with infinite loop is executing. Thus we have a deadlock -- loop will continue to spin, sending notify events, and at the same time preventing these notify events from being run on a workqueue. All notify events are deferred, thus we see explosion in memory consumption. Also as GPE handling is blocked, machines overheat because ACPI-based fan control is stalled. Eventually by external poll of the same acpi_evaluate, kacpid is released and all the queued notify events are free to run, thus 100% CPU utilization by kacpid for several seconds or more. To prevent this failure, Linux must not send notify events to the kacpid workqueue -- either executing them immediately or putting them on some other thread. The first attempt to create a new thread was done by Peter Wainwright He created a bunch of threads, which were stealing work from a kacpid workqueue. This patch appeared in 2.6.15-based kernel shipped with Ubuntu 6.06 LTS. Second attempt was done by Alexey Starikovskiy, who created a new thread for each Notify event. This worked OK on HP nx machines, but broke Linus' Compaq n620c, by producing threads with a speed what they stopped the machine completely. Thus this patch was reverted from 2.6.18-rc2. Alexey re-made the patch to create second workqueue just for notify events, thus hopping it will not break Linus' machine. Patch was tested on the same HP nx machines in #5534 and #7122, but this broke Linus' machine also and was reverted from 2.6.19-rc with much fanfair. The 4th patch inserted schedule_timeout(1) into deferred execution of kacpid, if we had any notify requests pending, but Linus decided that it was too complex (involved either changes to workqueue to see if it's empty or atomic inc/dec). Then a 5th attempt did a yield() to every GPE execution. Finally, this 6th generation patch simply executes the notify handler on the stack. Previous attempts to do this simple solution failed because of issues in AML mutex re-entrancy which are now fixed by the previous patch in this series. http://bugzilla.kernel.org/show_bug.cgi?id=5534 Signed-off-by: Alexey Starikovskiy Signed-off-by: Len Brown --- drivers/acpi/events/evmisc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/events/evmisc.c b/drivers/acpi/events/evmisc.c index 1b784ffe..d572700 100644 --- a/drivers/acpi/events/evmisc.c +++ b/drivers/acpi/events/evmisc.c @@ -196,12 +196,11 @@ acpi_ev_queue_notify_request(struct acpi_namespace_node * node, notify_info->notify.value = (u16) notify_value; notify_info->notify.handler_obj = handler_obj; - status = - acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_ev_notify_dispatch, - notify_info); - if (ACPI_FAILURE(status)) { - acpi_ut_delete_generic_state(notify_info); - } + acpi_ex_relinquish_interpreter(); + + acpi_ev_notify_dispatch(notify_info); + + acpi_ex_reacquire_interpreter(); } if (!handler_obj) { -- cgit v1.1