Commit 69c841b6 authored by Lv Zheng's avatar Lv Zheng Committed by Rafael J. Wysocki

ACPICA: Update use of acpi_os_wait_events_complete interface.

This patch cleans up all of the acpi_os_wait_events_complete() invocations to
make it to be invoked inside of ACPICA in the way to accommodate Linux's
work queue implementation.

According to the report, current Linux kernel code is facing a boot time
race issue in the acpi_remove_notify_handler(). This is because:
Linux is using work queues to implement a deferred handler call environment
while ACPICA expects OSPM to implement acpi_os_wait_events_complete() using
wait queues.  The position to invoke a "waiter" is not suitable for a
"flusher" as new invocations can be scheduled after this position and
before the deletion of the handler from its management container.

Since the following commit has deleted acpi_os_wait_events_complete()
parameters, it thus might not be possible for OSPM to achieve a safe
removal using wait queues.  This requires ACPICA to be changed accordingly
to "flush" handlers rather than "wait" them to be drain up:

  Commit: 5ff986a2a9db11858247b71fe242fe17617229aa
  Date: Wed, 16 May 2012 13:36:07 -0700
  Subject: Introduce acpi_os_wait_events_complete interface.

  This interface will block until asynchronous events like notifies
  and GPEs are complete. Within ACPICA, it is called before a notify or GPE
  handler is removed. ACPICA BZ 868.

This patch fixes this issue by invoking acpi_os_wait_events_complete() in the
way to "flush" things - it thus should be put to the position after handler
is removed from its management container but before it is destructed.

The technical concerns are:
1. MTX_NAMESPACE is used to protect things that acpi_os_wait_events_complete()
   might be waiting for, thus MTX_NAMESPACE must be unlocked before
   invoking acpi_os_wait_events_complete().
2. MTX_NAMESPACE is also used to implement the serialization of
   acpi_install_notify_handler() and acpi_remove_notify_handler(). This patch
   changes this logic, thus if there are many
   acpi_install/remove_notify_handler() invoked in parallel, the
   acpi_os_wait_events_complete() might face the races which could cause it
   never running to an end.  Normally this will require additional code to
   implement a separate locking facility which is not implemented due to 3.
3. Given ACPICA users will always invoke acpi_install_notify_handler() once
   during Linux module/device initialization and invoke
   acpi_remove_notify_handler() once during module/device finalization,
   problem stated in 2 will not happen in Linux environment due to the
   mutual exclusive module/device existence, this fix thus is sufficient.
Same concerns can apply to acpi_install/remove_gpe_handler(). Reported and
tested: Ronald Vink.  Fixed: Lv Zheng.

References: https://bugzilla.kernel.org/show_bug.cgi?id=60583Signed-off-by: default avatarLv Zheng <lv.zheng@intel.com>
Reported-and-Tested-by: default avatarRonald Vink <ronald.vink@boskalis.com>
Signed-off-by: default avatarBob Moore <robert.moore@intel.com>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
parent 06a63e33
...@@ -239,7 +239,7 @@ acpi_remove_notify_handler(acpi_handle device, ...@@ -239,7 +239,7 @@ acpi_remove_notify_handler(acpi_handle device,
union acpi_operand_object *obj_desc; union acpi_operand_object *obj_desc;
union acpi_operand_object *handler_obj; union acpi_operand_object *handler_obj;
union acpi_operand_object *previous_handler_obj; union acpi_operand_object *previous_handler_obj;
acpi_status status; acpi_status status = AE_OK;
u32 i; u32 i;
ACPI_FUNCTION_TRACE(acpi_remove_notify_handler); ACPI_FUNCTION_TRACE(acpi_remove_notify_handler);
...@@ -251,20 +251,17 @@ acpi_remove_notify_handler(acpi_handle device, ...@@ -251,20 +251,17 @@ acpi_remove_notify_handler(acpi_handle device,
return_ACPI_STATUS(AE_BAD_PARAMETER); return_ACPI_STATUS(AE_BAD_PARAMETER);
} }
/* Make sure all deferred notify tasks are completed */
acpi_os_wait_events_complete();
status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status);
}
/* Root Object. Global handlers are removed here */ /* Root Object. Global handlers are removed here */
if (device == ACPI_ROOT_OBJECT) { if (device == ACPI_ROOT_OBJECT) {
for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) { for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) {
if (handler_type & (i + 1)) { if (handler_type & (i + 1)) {
status =
acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status);
}
if (!acpi_gbl_global_notify[i].handler || if (!acpi_gbl_global_notify[i].handler ||
(acpi_gbl_global_notify[i].handler != (acpi_gbl_global_notify[i].handler !=
handler)) { handler)) {
...@@ -277,31 +274,40 @@ acpi_remove_notify_handler(acpi_handle device, ...@@ -277,31 +274,40 @@ acpi_remove_notify_handler(acpi_handle device,
acpi_gbl_global_notify[i].handler = NULL; acpi_gbl_global_notify[i].handler = NULL;
acpi_gbl_global_notify[i].context = NULL; acpi_gbl_global_notify[i].context = NULL;
(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
/* Make sure all deferred notify tasks are completed */
acpi_os_wait_events_complete();
} }
} }
goto unlock_and_exit; return_ACPI_STATUS(AE_OK);
} }
/* All other objects: Are Notifies allowed on this object? */ /* All other objects: Are Notifies allowed on this object? */
if (!acpi_ev_is_notify_object(node)) { if (!acpi_ev_is_notify_object(node)) {
status = AE_TYPE; return_ACPI_STATUS(AE_TYPE);
goto unlock_and_exit;
} }
/* Must have an existing internal object */ /* Must have an existing internal object */
obj_desc = acpi_ns_get_attached_object(node); obj_desc = acpi_ns_get_attached_object(node);
if (!obj_desc) { if (!obj_desc) {
status = AE_NOT_EXIST; return_ACPI_STATUS(AE_NOT_EXIST);
goto unlock_and_exit;
} }
/* Internal object exists. Find the handler and remove it */ /* Internal object exists. Find the handler and remove it */
for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) { for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) {
if (handler_type & (i + 1)) { if (handler_type & (i + 1)) {
status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status);
}
handler_obj = obj_desc->common_notify.notify_list[i]; handler_obj = obj_desc->common_notify.notify_list[i];
previous_handler_obj = NULL; previous_handler_obj = NULL;
...@@ -329,10 +335,17 @@ acpi_remove_notify_handler(acpi_handle device, ...@@ -329,10 +335,17 @@ acpi_remove_notify_handler(acpi_handle device,
handler_obj->notify.next[i]; handler_obj->notify.next[i];
} }
(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
/* Make sure all deferred notify tasks are completed */
acpi_os_wait_events_complete();
acpi_ut_remove_reference(handler_obj); acpi_ut_remove_reference(handler_obj);
} }
} }
return_ACPI_STATUS(status);
unlock_and_exit: unlock_and_exit:
(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
return_ACPI_STATUS(status); return_ACPI_STATUS(status);
...@@ -842,10 +855,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_device, ...@@ -842,10 +855,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
return_ACPI_STATUS(AE_BAD_PARAMETER); return_ACPI_STATUS(AE_BAD_PARAMETER);
} }
/* Make sure all deferred GPE tasks are completed */
acpi_os_wait_events_complete();
status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS); status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS);
if (ACPI_FAILURE(status)) { if (ACPI_FAILURE(status)) {
return_ACPI_STATUS(status); return_ACPI_STATUS(status);
...@@ -897,9 +906,17 @@ acpi_remove_gpe_handler(acpi_handle gpe_device, ...@@ -897,9 +906,17 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
(void)acpi_ev_add_gpe_reference(gpe_event_info); (void)acpi_ev_add_gpe_reference(gpe_event_info);
} }
acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
/* Make sure all deferred GPE tasks are completed */
acpi_os_wait_events_complete();
/* Now we can free the handler object */ /* Now we can free the handler object */
ACPI_FREE(handler); ACPI_FREE(handler);
return_ACPI_STATUS(status);
unlock_and_exit: unlock_and_exit:
acpi_os_release_lock(acpi_gbl_gpe_lock, flags); acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment