aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLv Zheng <lv.zheng@intel.com>2016-07-05 01:53:12 -0400
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>2016-07-05 16:48:44 -0400
commit45209046c47b93fadf26dc59a9da724f387b9cf2 (patch)
tree0eaaaeee0a25b9bb2c86260f6a7ab0fb3fab9099
parent2f38b1b16d9280689e5cfa47a4c50956bf437f0d (diff)
ACPICA: Namespace: Fix namespace/interpreter lock ordering
There is a lock order issue in acpi_load_tables(). The namespace lock is held before holding the interpreter lock. With ACPI_MUTEX_DEBUG enabled in the kernel, this is printed to the log during boot: [ 0.885699] ACPI Error: Invalid acquire order: Thread 405884224 owns [ACPI_MTX_Namespace], wants [ACPI_MTX_Interpreter] (20160422/utmutex-263) [ 0.885881] ACPI Error: Could not acquire AML Interpreter mutex (20160422/exutils-95) [ 0.893846] ACPI Error: Mutex [0x0] is not acquired, cannot release (20160422/utmutex-326) [ 0.894019] ACPI Error: Could not release AML Interpreter mutex (20160422/exutils-133) The issue has been introduced by the following commit: Commit: 2f38b1b16d9280689e5cfa47a4c50956bf437f0d ACPICA Commit: bfe03ffcde8ed56a7eae38ea0b188aeb12f9c52e Subject: ACPICA: Namespace: Fix a regression that MLC support triggers dead lock in dynamic table loading Which fixed a deadlock issue for acpi_ns_load_table() in acpi_ex_add_table() but didn't take care of the lock order in acpi_ns_load_table() correctly. Originally (before the above commit), ACPICA used the namespace/interpreter locks in the following 2 key code paths: 1. Table loading: acpi_ns_load_table L(Namespace) acpi_ns_parse_table acpi_ns_one_complete_parse U(Namespace) 2. Object evaluation: acpi_ns_evaluate L(Interpreter) acpi_ps_execute_method U(Interpreter) acpi_ns_load_table L(Namespace) U(Namespace) acpi_ev_initialize_region L(Namespace) U(Namespace) address_space.setup L(Namespace) U(Namespace) address_space.handler L(Namespace) U(Namespace) acpi_os_wait_semaphore acpi_os_acquire_mutex acpi_os_sleep L(Interpreter) U(Interpreter) During runtime, while acpi_ns_evaluate is called, the lock order is always Interpreter -> Namespace. In turn, the problematic commit acquires the locks in the following order: 3. Table loading: acpi_ns_load_table L(Namespace) acpi_ns_parse_table L(Interpreter) acpi_ns_one_complete_parse U(Interpreter) U(Namespace) To fix the lock order issue, move the interpreter lock to acpi_ns_load_table() to ensure the lock order correctness: 4. Table loading: acpi_ns_load_table L(Interpreter) L(Namespace) acpi_ns_parse_table acpi_ns_one_complete_parse U(Namespace) U(Interpreter) However, this doesn't fix the current design issues related to the namespace lock. For example, we can notice that in acpi_ns_evaluate(), outside of acpi_ns_load_table(), the namespace objects may be created by the named object creation control methods. And the creation of the method-owned namespace objects are not locked by the namespace lock. This patch doesn't try to fix such kind of existing issues. Fixes: 2f38b1b16d92 (ACPICA: Namespace: Fix a regression that MLC support triggers dead lock in dynamic table loading) Signed-off-by: Lv Zheng <lv.zheng@intel.com> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
-rw-r--r--drivers/acpi/acpica/nsload.c7
-rw-r--r--drivers/acpi/acpica/nsparse.c9
2 files changed, 8 insertions, 8 deletions
diff --git a/drivers/acpi/acpica/nsload.c b/drivers/acpi/acpica/nsload.c
index b5e2b0ada0ab..297f6aacd7d4 100644
--- a/drivers/acpi/acpica/nsload.c
+++ b/drivers/acpi/acpica/nsload.c
@@ -46,6 +46,7 @@
46#include "acnamesp.h" 46#include "acnamesp.h"
47#include "acdispat.h" 47#include "acdispat.h"
48#include "actables.h" 48#include "actables.h"
49#include "acinterp.h"
49 50
50#define _COMPONENT ACPI_NAMESPACE 51#define _COMPONENT ACPI_NAMESPACE
51ACPI_MODULE_NAME("nsload") 52ACPI_MODULE_NAME("nsload")
@@ -78,6 +79,8 @@ acpi_ns_load_table(u32 table_index, struct acpi_namespace_node *node)
78 79
79 ACPI_FUNCTION_TRACE(ns_load_table); 80 ACPI_FUNCTION_TRACE(ns_load_table);
80 81
82 acpi_ex_enter_interpreter();
83
81 /* 84 /*
82 * Parse the table and load the namespace with all named 85 * Parse the table and load the namespace with all named
83 * objects found within. Control methods are NOT parsed 86 * objects found within. Control methods are NOT parsed
@@ -89,7 +92,7 @@ acpi_ns_load_table(u32 table_index, struct acpi_namespace_node *node)
89 */ 92 */
90 status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); 93 status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
91 if (ACPI_FAILURE(status)) { 94 if (ACPI_FAILURE(status)) {
92 return_ACPI_STATUS(status); 95 goto unlock_interp;
93 } 96 }
94 97
95 /* If table already loaded into namespace, just return */ 98 /* If table already loaded into namespace, just return */
@@ -130,6 +133,8 @@ acpi_ns_load_table(u32 table_index, struct acpi_namespace_node *node)
130 133
131unlock: 134unlock:
132 (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); 135 (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
136unlock_interp:
137 (void)acpi_ex_exit_interpreter();
133 138
134 if (ACPI_FAILURE(status)) { 139 if (ACPI_FAILURE(status)) {
135 return_ACPI_STATUS(status); 140 return_ACPI_STATUS(status);
diff --git a/drivers/acpi/acpica/nsparse.c b/drivers/acpi/acpica/nsparse.c
index 1783cd7e1446..f631a47724f0 100644
--- a/drivers/acpi/acpica/nsparse.c
+++ b/drivers/acpi/acpica/nsparse.c
@@ -47,7 +47,6 @@
47#include "acparser.h" 47#include "acparser.h"
48#include "acdispat.h" 48#include "acdispat.h"
49#include "actables.h" 49#include "actables.h"
50#include "acinterp.h"
51 50
52#define _COMPONENT ACPI_NAMESPACE 51#define _COMPONENT ACPI_NAMESPACE
53ACPI_MODULE_NAME("nsparse") 52ACPI_MODULE_NAME("nsparse")
@@ -171,8 +170,6 @@ acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node *start_node)
171 170
172 ACPI_FUNCTION_TRACE(ns_parse_table); 171 ACPI_FUNCTION_TRACE(ns_parse_table);
173 172
174 acpi_ex_enter_interpreter();
175
176 /* 173 /*
177 * AML Parse, pass 1 174 * AML Parse, pass 1
178 * 175 *
@@ -188,7 +185,7 @@ acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node *start_node)
188 status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS1, 185 status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS1,
189 table_index, start_node); 186 table_index, start_node);
190 if (ACPI_FAILURE(status)) { 187 if (ACPI_FAILURE(status)) {
191 goto error_exit; 188 return_ACPI_STATUS(status);
192 } 189 }
193 190
194 /* 191 /*
@@ -204,10 +201,8 @@ acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node *start_node)
204 status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS2, 201 status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS2,
205 table_index, start_node); 202 table_index, start_node);
206 if (ACPI_FAILURE(status)) { 203 if (ACPI_FAILURE(status)) {
207 goto error_exit; 204 return_ACPI_STATUS(status);
208 } 205 }
209 206
210error_exit:
211 acpi_ex_exit_interpreter();
212 return_ACPI_STATUS(status); 207 return_ACPI_STATUS(status);
213} 208}