diff options
author | Lv Zheng <lv.zheng@intel.com> | 2016-07-05 01:53:12 -0400 |
---|---|---|
committer | Rafael J. Wysocki <rafael.j.wysocki@intel.com> | 2016-07-05 16:48:44 -0400 |
commit | 45209046c47b93fadf26dc59a9da724f387b9cf2 (patch) | |
tree | 0eaaaeee0a25b9bb2c86260f6a7ab0fb3fab9099 | |
parent | 2f38b1b16d9280689e5cfa47a4c50956bf437f0d (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.c | 7 | ||||
-rw-r--r-- | drivers/acpi/acpica/nsparse.c | 9 |
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 |
51 | ACPI_MODULE_NAME("nsload") | 52 | ACPI_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 | ||
131 | unlock: | 134 | unlock: |
132 | (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); | 135 | (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); |
136 | unlock_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 |
53 | ACPI_MODULE_NAME("nsparse") | 52 | ACPI_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 | ||
210 | error_exit: | ||
211 | acpi_ex_exit_interpreter(); | ||
212 | return_ACPI_STATUS(status); | 207 | return_ACPI_STATUS(status); |
213 | } | 208 | } |