Skip to contentSkip to conversation

ACPICA: Events: Fix notify handler atomicity

RFC PATCH New Lv Zheng 1 participant 4 messages Sep 11, 2015

Subsystems (2)
lore
Share & tools
lore
author
#1 lore raw

RFC PATCH 1/3 ACPICA: Utilities: Add deferred lock free contexts for deleting operand objects

In reply to Lv Zheng

When acpi_ut_delete_internal_obj() is invoked, reference count should be zero which means this context is the only context remained referencing this object. Normally, we may just unlock namespace mutex (ACPI_MTX_NAMESPACE) before invoking this function, but since this function is invoked from different environment, doing this have side effects:
1. Other ACPI internal mutexes may be held orderly when this function is invoked. Without unlocking such locks, we still cannot achieve a lock free environment.
2. For some environment, it might not be safe to unlock the namespace mutex right in that context.
So we introduces a deferred unlocked environment for further cleanups.

There are two choices to convert this function into a lock free style:
1. Deferred work queue: this may not be performance friendly as in order to __delete__ the object, we __create__ a new OS object (work queue item) to achieve each deletion, which may lead to a failure when the memory resource is heavily used.
2. Garbage collection: the deleted objects are linked together and at each exit of such a deletion, a garbage cleaner function is invoked where this function is invoked in a lock free style.

This patch implements both choices:
1. OSL_GC_HANDLER: This is a deferred work item, it is used in case there link point available to link the object into the garbage;
2. OSL_GC_THREAD: The linked deleted objects are destructed in a thread. Note that in the slow path, we still need to __create__ a new OS object in order to __delete__ the operand object. Lv Zheng.

Cc Lee, Chun-Yi <[email protected]>
Cc Hanjun Guo <[email protected]>
---
 drivers/acpi/acpica/acglobal.h |    5 ++
 drivers/acpi/acpica/utdelete.c |  155 +++++++++++++++++++++++++++++++++++++---
 include/acpi/acpiosxf.h        |    2 +
 3 files changed, 153 insertions(+), 9 deletions(-)
drivers/acpi/acpica/acglobal.h +5 -0
index 09f37b5..e7f0f82 100644
@@ -161,6 +161,11 @@ ACPI_GLOBAL(u32, acpi_gbl_owner_id_mask[ACPI_NUM_OWNERID_MASKS]);
 ACPI_GLOBAL(u8, acpi_gbl_last_owner_id_index);
 ACPI_GLOBAL(u8, acpi_gbl_next_owner_id_offset);
 
+/* Garbage collection support */
+
+ACPI_INIT_GLOBAL(union acpi_operand_object *, acpi_gbl_garbage_operands, NULL);
+ACPI_INIT_GLOBAL(u8, acpi_gbl_gc_thread_created, FALSE);
+
 /* Initialization sequencing */
 
 ACPI_GLOBAL(u8, acpi_gbl_reg_methods_executed);
drivers/acpi/acpica/utdelete.c +146 -9

212 lines not on this page · raw message

include/acpi/acpiosxf.h +2 -0
index a54ad1c..6a8f6c3 100644
@@ -55,6 +55,8 @@ typedef enum {
 	OSL_GLOBAL_LOCK_HANDLER,
 	OSL_NOTIFY_HANDLER,
 	OSL_GPE_HANDLER,
+	OSL_GC_THREAD,
+	OSL_GC_HANDLER,
 	OSL_DEBUGGER_THREAD,
 	OSL_EC_POLL_HANDLER,
 	OSL_EC_BURST_HANDLER
Signature · 1 line
1.7.10

Lv Zheng

author
#0 lore raw

cover ACPICA: Events: Fix notify handler atomicity

This patchset tries to fix atomicity issue in the notify handler.

Lv Zheng (3):
ACPICA: Utilities: Add deferred lock free contexts for deleting
operand objects
ACPI / osl: Add OSL_GC_HANDLER/OSL_GC_THREAD support
ACPICA: Events: Fix reference counting code for per-object notify
handlers

 drivers/acpi/acpica/acevents.h |    4 +
 drivers/acpi/acpica/acglobal.h |    5 ++
 drivers/acpi/acpica/acobject.h |    2 +-
 drivers/acpi/acpica/evmisc.c   |  134 +++++++++++++++++++++++++++-
 drivers/acpi/acpica/evxface.c  |   66 ++------------
 drivers/acpi/acpica/utdelete.c |  191 +++++++++++++++++++++++++++++++++-------
 drivers/acpi/acpica/utstate.c  |   22 ++++-
 drivers/acpi/bus.c             |    4 +-
 drivers/acpi/osl.c             |   52 ++++++++++-
 include/acpi/acpiosxf.h        |    2 +
 10 files changed, 381 insertions(+), 101 deletions(-)
Signature · 1 line
1.7.10
author
#2 lore raw

RFC PATCH 2/3 ACPI / osl: Add OSL_GC_HANDLER/OSL_GC_THREAD support

This patch enables Linux capability of supporting ACPICA operand object GC.

Note that, when the operand object is deferred freed in the early stage, the GC work queue and the GC thread cannot be used for the execution, acpi_os_initialized can be used to indicate such periods and the operand object can either be freed in ACPICA non deferred path (work queue) or linked together for later try.

Cc Lee, Chun-Yi <[email protected]>
Cc Hanjun Guo <[email protected]>
---
 drivers/acpi/bus.c |    4 ++--
 drivers/acpi/osl.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 6 deletions(-)
drivers/acpi/bus.c +2 -2
index 46506e7..7455f69 100644
@@ -982,8 +982,6 @@ static int __init acpi_bus_init(void)
 	int result;
 	acpi_status status;
 
-	acpi_os_initialize1();
-
 	status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
 	if (ACPI_FAILURE(status)) {
 		printk(KERN_ERR PREFIX
@@ -1085,6 +1083,8 @@ static int __init acpi_init(void)
 		acpi_kobj = NULL;
 	}
 
+	acpi_os_initialize1();
+
 	init_acpi_device_notify();
 	result = acpi_bus_init();
 	if (result) {
drivers/acpi/osl.c +48 -4

126 lines not on this page · raw message

Signature · 1 line
1.7.10
author
#3 lore raw

RFC PATCH 3/3 ACPICA: Events: Fix reference counting code for per-object notify handlers

In reply to Lv Zheng

The reference counting code of Notify handler object is abnormal. It doesn't follow the coding rule that the reference increment should/must only happen to a pointer reference and the reference decrement should /must only happen to a pointer dereference.

This patch fixes per-object notify handler reference counting. The global notify handlers are not covered by this patch. Lv Zheng.

Reference: https://bugs.acpica.org/show_bug.cgi?id=1115

---
 drivers/acpi/acpica/acevents.h |    4 ++
 drivers/acpi/acpica/acobject.h |    2 +-
 drivers/acpi/acpica/evmisc.c   |  134 +++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/acpica/evxface.c  |   66 +++-----------------
 drivers/acpi/acpica/utdelete.c |   36 +++++------
 drivers/acpi/acpica/utstate.c  |   22 ++++++-
 6 files changed, 178 insertions(+), 86 deletions(-)
drivers/acpi/acpica/acevents.h +4 -0
index 228704b7..dc0b19e 100644
@@ -64,6 +64,10 @@ acpi_status
 acpi_ev_queue_notify_request(struct acpi_namespace_node *node,
 			     u32 notify_value);
 
+acpi_status
+acpi_ev_delete_notify_handlers(union acpi_operand_object *object,
+			       u32 handler_type, acpi_notify_handler handler);
+
 /*
  * evglock - Global Lock support
  */
drivers/acpi/acpica/acobject.h +1 -1
index 0bd02c4..35625ec 100644
@@ -207,7 +207,7 @@ struct acpi_object_method {
  * Common fields for objects that support ASL notifications
  */
 #define ACPI_COMMON_NOTIFY_INFO \
-	union acpi_operand_object       *notify_list[2];    /* Handlers for system/device notifies */\
+	union acpi_operand_object       *notify_list[ACPI_NUM_NOTIFY_TYPES]; /* Handlers for system/device notifies */\
 	union acpi_operand_object       *handler;	/* Handler for Address space */
 
 struct acpi_object_notify_common {	/* COMMON NOTIFY for POWER, PROCESSOR, DEVICE, and THERMAL */
drivers/acpi/acpica/evmisc.c +132 -2

165 lines not on this page · raw message

drivers/acpi/acpica/evxface.c +7 -59

94 lines not on this page · raw message

drivers/acpi/acpica/utdelete.c +14 -22
index ea737f4..619ab54 100644
@@ -190,6 +190,20 @@ static void ACPI_SYSTEM_XFACE acpi_ut_delete_object_deferred(void *context)
 			acpi_ut_remove_reference(handler_desc);
 			handler_desc = next_desc;
 		}
+
+		/*lint -fallthrough */
+
+	case ACPI_TYPE_POWER:
+		/*
+		 * Update the notify objects for these types (if present)
+		 * Two lists, system and device notify handlers.
+		 */
+		(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+		acpi_ev_delete_notify_handlers(object, ACPI_ALL_NOTIFY, NULL);
+		status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
+		if (ACPI_FAILURE(status)) {
+			return_VOID;
+		}
 		break;
 
 	case ACPI_TYPE_MUTEX:
@@ -635,7 +649,6 @@ acpi_ut_update_object_reference(union acpi_operand_object *object, u16 action)
 	acpi_status status = AE_OK;
 	union acpi_generic_state *state_list = NULL;
 	union acpi_operand_object *next_object = NULL;
-	union acpi_operand_object *prev_object;
 	union acpi_generic_state *state;
 	u32 i;
 
@@ -656,27 +669,6 @@ acpi_ut_update_object_reference(union acpi_operand_object *object, u16 action)
 		 * Different object types have different subobjects.
 		 */
 		switch (object->common.type) {
-		case ACPI_TYPE_DEVICE:
-		case ACPI_TYPE_PROCESSOR:
-		case ACPI_TYPE_POWER:
-		case ACPI_TYPE_THERMAL:
-			/*
-			 * Update the notify objects for these types (if present)
-			 * Two lists, system and device notify handlers.
-			 */
-			for (i = 0; i < ACPI_NUM_NOTIFY_TYPES; i++) {
-				prev_object =
-				    object->common_notify.notify_list[i];
-				while (prev_object) {
-					next_object =
-					    prev_object->notify.next[i];
-					acpi_ut_update_ref_count(prev_object,
-								 action);
-					prev_object = next_object;
-				}
-			}
-			break;
-
 		case ACPI_TYPE_PACKAGE:
 			/*
 			 * We must update all the sub-objects of the package,
drivers/acpi/acpica/utstate.c +20 -2
index f201171..a959b3f 100644
@@ -297,12 +297,30 @@ union acpi_generic_state *acpi_ut_create_control_state(void)
 
 void acpi_ut_delete_generic_state(union acpi_generic_state *state)
 {
+	union acpi_operand_object *handler_list_head;
+
 	ACPI_FUNCTION_ENTRY();
 
 	/* Ignore null state */
 
-	if (state) {
-		(void)acpi_os_release_object(acpi_gbl_state_cache, state);
+	if (!state) {
+		return;
+	}
+
+	switch (state->common.descriptor_type) {
+	case ACPI_DESC_TYPE_STATE_NOTIFY:
+
+		if (state->notify.handler_list_head) {
+			handler_list_head = state->notify.handler_list_head;
+			state->notify.handler_list_head = NULL;
+			acpi_ut_remove_reference(handler_list_head);
+		}
+		break;
+
+	default:
+
+		break;
 	}
+	(void)acpi_os_release_object(acpi_gbl_state_cache, state);
 	return;
 }
Signature · 1 line
1.7.10