Skip to contentSkip to conversation

ACPICA: add postorder namespace walk

PATCH In review Bjorn Helgaas 2 participants 6 messages Jul 17, 2009 – Jul 31, 2009

Subsystems (2)
ACPI ACPI component architecture (acpica)
lore
Share & tools
lore

Bjorn Helgaas

author
#0 lore raw

cover ACPICA: add postorder namespace walk

AcpiWalkNamespace() only supports preorder traversal of the namespace. This visits a node before any of its children, so it works well for adding new devices. However, when removing devices, we need a postorder traversal so we can remove children before the parents.

These patches add a new AcpiWalkNamespace2() interface that takes two callback arguments: one for preorder traversal and another for postorder. These patches don't change the behavior of AcpiWalkNamespace().

Note that the existing preorder AcpiWalkNamespace() never actually visits the starting node. Therefore, we skip it in postorder also.

---

Bjorn Helgaas (2):
ACPICA: Factor out user callback function from namespace walk
ACPICA: Support both preorder and postorder namespace walks

 drivers/acpi/acpica/acnamesp.h |    9 ++
 drivers/acpi/acpica/nswalk.c   |  234 ++++++++++++++++++++++++++++------------
 drivers/acpi/acpica/nsxfeval.c |   41 +++++--
 include/acpi/acpixf.h          |    8 +
 4 files changed, 208 insertions(+), 84 deletions(-)
Signature · 1 line
Bjorn
author
#1 lore raw

PATCH 1/2 ACPICA: Factor out user callback function from namespace walk

This patch factors out the code to call a user callback function while walking the namespace. This makes the "tree navigation" part of the walk more visible and will make it easier to add a postorder callback.

This patch may be used under either the GPL v2 or the BSD-style license used for the Intel ACPICA.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/acpica/nswalk.c |  130 ++++++++++++++++++++++--------------------
 1 files changed, 68 insertions(+), 62 deletions(-)
drivers/acpi/acpica/nswalk.c +68 -62
index 35539df..572a181 100644
@@ -156,6 +156,60 @@ struct acpi_namespace_node *acpi_ns_get_next_node_typed(acpi_object_type type,
 	return (NULL);
 }
 
+static acpi_status
+acpi_call_user_function(acpi_object_type type,
+			u32 flags,
+			acpi_walk_callback user_function,
+			struct acpi_namespace_node *node,
+			u32 level,
+			void *context,
+			void **return_value)
+{
+	acpi_status status;
+	acpi_status mutex_status;
+
+	/*
+	 * Ignore all temporary namespace nodes (created during control
+	 * method execution) unless told otherwise. These temporary nodes
+	 * can cause a race condition because they can be deleted during
+	 * the execution of the user function (if the namespace is
+	 * unlocked before invocation of the user function.) Only the
+	 * debugger namespace dump will examine the temporary nodes.
+	 */
+	if ((node->flags & ANOBJ_TEMPORARY) &&
+	    !(flags & ACPI_NS_WALK_TEMP_NODES)) {
+		return (AE_CTRL_DEPTH);
+	}
+
+	/* Type must match requested type unless we want ACPI_TYPE_ANY */
+
+	if (node->type != type && type != ACPI_TYPE_ANY) {
+		return (AE_OK);
+	}
+
+	/*
+	 * Found a matching node, invoke the user callback function.
+	 * Unlock the namespace if flag is set.
+	 */
+	if (flags & ACPI_NS_WALK_UNLOCK) {
+		mutex_status = acpi_ut_release_mutex (ACPI_MTX_NAMESPACE);
+		if (ACPI_FAILURE(mutex_status)) {
+			return (mutex_status);
+		}
+	}
+
+	status = user_function(node, level, context, return_value);
+
+	if (flags & ACPI_NS_WALK_UNLOCK) {
+		mutex_status = acpi_ut_acquire_mutex (ACPI_MTX_NAMESPACE);
+		if (ACPI_FAILURE(mutex_status)) {
+			return (mutex_status);
+		}
+	}
+
+	return (status);
+}
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ns_walk_namespace

@@ -195,7 +249,6 @@ acpi_ns_walk_namespace(acpi_object_type type,
 		       void *context, void **return_value)
 {
 	acpi_status status;
-	acpi_status mutex_status;
 	struct acpi_namespace_node *child_node;
 	struct acpi_namespace_node *parent_node;
 	acpi_object_type child_type;

@@ -229,75 +282,28 @@ acpi_ns_walk_namespace(acpi_object_type type,
 		child_node = acpi_ns_get_next_node(parent_node, child_node);
 		if (child_node) {
 
-			/* Found next child, get the type if we are not searching for ANY */
+			status = acpi_call_user_function(type, flags,
+					user_function, child_node, level,
+					context, return_value);
 
-			if (type != ACPI_TYPE_ANY) {
-				child_type = child_node->type;
-			}
+			switch (status) {
+			case AE_OK:
+			case AE_CTRL_DEPTH:
 
-			/*
-			 * Ignore all temporary namespace nodes (created during control
-			 * method execution) unless told otherwise. These temporary nodes
-			 * can cause a race condition because they can be deleted during
-			 * the execution of the user function (if the namespace is
-			 * unlocked before invocation of the user function.) Only the
-			 * debugger namespace dump will examine the temporary nodes.
-			 */
-			if ((child_node->flags & ANOBJ_TEMPORARY) &&
-			    !(flags & ACPI_NS_WALK_TEMP_NODES)) {
-				status = AE_CTRL_DEPTH;
-			}
-
-			/* Type must match requested type */
-
-			else if (child_type == type) {
-				/*
-				 * Found a matching node, invoke the user callback function.
-				 * Unlock the namespace if flag is set.
-				 */
-				if (flags & ACPI_NS_WALK_UNLOCK) {
-					mutex_status =
-					    acpi_ut_release_mutex
-					    (ACPI_MTX_NAMESPACE);
-					if (ACPI_FAILURE(mutex_status)) {
-						return_ACPI_STATUS
-						    (mutex_status);
-					}
-				}
-
-				status =
-				    user_function(child_node, level, context,
-						  return_value);
-
-				if (flags & ACPI_NS_WALK_UNLOCK) {
-					mutex_status =
-					    acpi_ut_acquire_mutex
-					    (ACPI_MTX_NAMESPACE);
-					if (ACPI_FAILURE(mutex_status)) {
-						return_ACPI_STATUS
-						    (mutex_status);
-					}
-				}
-
-				switch (status) {
-				case AE_OK:
-				case AE_CTRL_DEPTH:
+				/* Just keep going */
+				break;
 
-					/* Just keep going */
-					break;
+			case AE_CTRL_TERMINATE:
 
-				case AE_CTRL_TERMINATE:
+				/* Exit now, with OK status */
 
-					/* Exit now, with OK status */
+				return_ACPI_STATUS(AE_OK);
 
-					return_ACPI_STATUS(AE_OK);
+			default:
 
-				default:
+				/* All others are valid exceptions */
 
-					/* All others are valid exceptions */
-
-					return_ACPI_STATUS(status);
-				}
+				return_ACPI_STATUS(status);
 			}
 
 			/*
author
#2 lore raw

PATCH 2/2 ACPICA: Support both preorder and postorder namespace walks

In reply to Bjorn Helgaas

We need to handle device addition in preorder (so we add a new device before any children), but device removal in postorder (so we remove any children before removing the parent).

The callback from AcpiWalkNamespace() is done in preorder. This patch adds a new AcpiWalkNamespace2() that takes both preorder and a postorder callbacks.

This patch may be used under either the GPL v2 or the BSD-style license used for the Intel ACPICA.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/acpica/acnamesp.h |    9 +++
 drivers/acpi/acpica/nswalk.c   |  106 ++++++++++++++++++++++++++++++++++++----
 drivers/acpi/acpica/nsxfeval.c |   41 +++++++++++----
 include/acpi/acpixf.h          |    8 +++
 4 files changed, 141 insertions(+), 23 deletions(-)
drivers/acpi/acpica/acnamesp.h +9 -0
index 94cdc2b..cb3a34f 100644
@@ -99,6 +99,15 @@ acpi_ns_walk_namespace(acpi_object_type type,
 		       acpi_walk_callback user_function,
 		       void *context, void **return_value);
 
+acpi_status
+acpi_ns_walk_namespace2(acpi_object_type type,
+			acpi_handle start_object,
+			u32 max_depth,
+			u32 flags,
+			acpi_walk_callback pre_user_function,
+			acpi_walk_callback post_user_function,
+			void *context, void **return_value);
+
 struct acpi_namespace_node *acpi_ns_get_next_node(struct acpi_namespace_node
 						  *parent,
 						  struct acpi_namespace_node
drivers/acpi/acpica/nswalk.c +95 -11

147 lines not on this page · raw message

drivers/acpi/acpica/nsxfeval.c +29 -12

75 lines not on this page · raw message

include/acpi/acpixf.h +8 -0
index 82ec6a3..8404879 100644
@@ -157,6 +157,14 @@ acpi_walk_namespace(acpi_object_type type,
 		    void *context, void **return_value);
 
 acpi_status
+acpi_walk_namespace2(acpi_object_type type,
+		     acpi_handle start_object,
+		     u32 max_depth,
+		     acpi_walk_callback pre_user_function,
+		     acpi_walk_callback post_user_function,
+		     void *context, void **return_value);
+
+acpi_status
 acpi_get_devices(const char *HID,
 		 acpi_walk_callback user_function,
 		 void *context, void **return_value);
author
#3 lore raw
In reply to Bjorn Helgaas
Quoted 11 lines · Bjorn Helgaas

AcpiWalkNamespace() only supports preorder traversal of the namespace. This visits a node before any of its children, so it works well for adding new devices. However, when removing devices, we need a postorder traversal so we can remove children before the parents.

These patches add a new AcpiWalkNamespace2() interface that takes two callback arguments: one for preorder traversal and another for postorder. These patches don't change the behavior of AcpiWalkNamespace().

Note that the existing preorder AcpiWalkNamespace() never actually visits the starting node. Therefore, we skip it in postorder also.

Hi Bob,

Any feedback on these patches, or on the question of whether AcpiWalkNamespace() should visit the starting node? (I expect that the starting node question will be most easily resolved by a documentation update.)

Bjorn

#4 lore raw

We've been wrapping up ACPI 4.0 changes. I hope to take a look at this next week.

Bob

Quoted 29 lines

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Friday, July 31, 2009 11:17 AM
To: Moore, Robert
Cc: Len Brown; [email protected]
Subject: Re: [PATCH 0/2] ACPICA: add postorder namespace walk

Quoted 11 lines · Bjorn Helgaas

AcpiWalkNamespace() only supports preorder traversal of the namespace. This visits a node before any of its children, so it works well for adding new devices. However, when removing devices, we need a postorder traversal so we can remove children before the parents.

These patches add a new AcpiWalkNamespace2() interface that takes two callback arguments: one for preorder traversal and another for postorder. These patches don't change the behavior of AcpiWalkNamespace().

Note that the existing preorder AcpiWalkNamespace() never actually visits the starting node. Therefore, we skip it in postorder also.

Hi Bob,

Any feedback on these patches, or on the question of whether AcpiWalkNamespace() should visit the starting node? (I expect that the starting node question will be most easily resolved by a documentation update.)

Bjorn

author
#5 lore raw
Quoted 1 line · Robert

We've been wrapping up ACPI 4.0 changes. I hope to take a look at this next week.

Oh, right, I saw mail about that a day or two ago and figured you must be pretty busy :-) I do hope to use the postorder traversal, but there are still some significant Linux issues to work out first, so there's no big rush.

Bjorn

Quoted 31 lines
Quoted 29 lines

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Friday, July 31, 2009 11:17 AM
To: Moore, Robert
Cc: Len Brown; [email protected]
Subject: Re: [PATCH 0/2] ACPICA: add postorder namespace walk

Quoted 11 lines · Bjorn Helgaas

AcpiWalkNamespace() only supports preorder traversal of the namespace. This visits a node before any of its children, so it works well for adding new devices. However, when removing devices, we need a postorder traversal so we can remove children before the parents.

These patches add a new AcpiWalkNamespace2() interface that takes two callback arguments: one for preorder traversal and another for postorder. These patches don't change the behavior of AcpiWalkNamespace().

Note that the existing preorder AcpiWalkNamespace() never actually visits the starting node. Therefore, we skip it in postorder also.

Hi Bob,

Any feedback on these patches, or on the question of whether AcpiWalkNamespace() should visit the starting node? (I expect that the starting node question will be most easily resolved by a documentation update.)

Bjorn