[PATCH] ACPI: Rework acpi_get_child() to be more efficient

[PATCH] ACPI: Rework acpi_get_child() to be more efficient

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Observe that acpi_get_child() doesn't need to use the helper
struct acpi_find_child structure and change it to work without it.

Moreover, acpi_get_child() doesn't need to loop any more once it has
found a matching handle, so make it stop in that case.  To prevent
the results from changing, make it use do_acpi_find_child() as a
post-order callback.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c |   36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

Index: linux/drivers/acpi/glue.c
===================================================================
--- linux.orig/drivers/acpi/glue.c
+++ linux/drivers/acpi/glue.c
@@ -90,38 +90,34 @@ static int acpi_find_bridge_device(struc
 	return ret;
 }
 
-/* Get device's handler per its address under its parent */
-struct acpi_find_child {
-	acpi_handle handle;
-	u64 address;
-};
-
-static acpi_status
-do_acpi_find_child(acpi_handle handle, u32 lvl, void *context, void **rv)
+static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
+				      void *addr_p, void **ret_p)
 {
-	acpi_status status;
 	struct acpi_device_info *info;
-	struct acpi_find_child *find = context;
 
-	status = acpi_get_object_info(handle, &info);
-	if (ACPI_SUCCESS(status)) {
-		if ((info->address == find->address)
-			&& (info->valid & ACPI_VALID_ADR))
-			find->handle = handle;
-		kfree(info);
+	if (ACPI_SUCCESS(acpi_get_object_info(handle, &info))) {
+		bool found = info->address == *((u64 *)addr_p)
+				&& (info->valid & ACPI_VALID_ADR);
+
+		ACPI_FREE(info);
+		if (found) {
+			*ret_p = handle;
+			return AE_CTRL_TERMINATE;
+		}
 	}
 	return AE_OK;
 }
 
 acpi_handle acpi_get_child(acpi_handle parent, u64 address)
 {
-	struct acpi_find_child find = { NULL, address };
+	void *ret = NULL;
 
 	if (!parent)
 		return NULL;
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, parent,
-			    1, do_acpi_find_child, NULL, &find, NULL);
-	return find.handle;
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, 1, NULL,
+			    do_acpi_find_child, &address, &ret);
+	return (acpi_handle)ret;
 }
 
 EXPORT_SYMBOL(acpi_get_child);

[Update][PATCH] ACPI: Rework acpi_get_child() to be more efficient

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: ACPI: Rework acpi_get_child() to be more efficient

Observe that acpi_get_child() doesn't need to use the helper
struct acpi_find_child structure and change it to work without it.
Also, using acpi_get_object_info() to get the output of _ADR for the
given device is overkill, because that function does much more than
just evaluating _ADR (let alone the additional memory allocation
done by it).

Moreover, acpi_get_child() doesn't need to loop any more once it has
found a matching handle, so make it stop in that case.  To prevent
the results from changing, make it use do_acpi_find_child() as
a post-order callback.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

The difference from the previous version is the replacement of
acpi_get_object_info() with the direct execution of _ADR, which is
more straightforward.

Thanks,
Rafael

---
 drivers/acpi/glue.c |   33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

Index: linux/drivers/acpi/glue.c
===================================================================
--- linux.orig/drivers/acpi/glue.c
+++ linux/drivers/acpi/glue.c
@@ -93,40 +93,31 @@ static int acpi_find_bridge_device(struc
 	return ret;
 }
 
-/* Get device's handler per its address under its parent */
-struct acpi_find_child {
-	acpi_handle handle;
-	u64 address;
-};
-
-static acpi_status
-do_acpi_find_child(acpi_handle handle, u32 lvl, void *context, void **rv)
+static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
+				      void *addr_p, void **ret_p)
 {
+	unsigned long long addr;
 	acpi_status status;
-	struct acpi_device_info *info;
-	struct acpi_find_child *find = context;
 
-	status = acpi_get_object_info(handle, &info);
-	if (ACPI_SUCCESS(status)) {
-		if ((info->address == find->address)
-			&& (info->valid & ACPI_VALID_ADR))
-			find->handle = handle;
-		kfree(info);
+	status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
+	if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+		*ret_p = handle;
+		return AE_CTRL_TERMINATE;
 	}
 	return AE_OK;
 }
 
 acpi_handle acpi_get_child(acpi_handle parent, u64 address)
 {
-	struct acpi_find_child find = { NULL, address };
+	void *ret = NULL;
 
 	if (!parent)
 		return NULL;
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, parent,
-			    1, do_acpi_find_child, NULL, &find, NULL);
-	return find.handle;
-}
 
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, 1, NULL,
+			    do_acpi_find_child, &address, &ret);
+	return (acpi_handle)ret;
+}
 EXPORT_SYMBOL(acpi_get_child);
 
 static int acpi_bind_one(struct device *dev, acpi_handle handle)

Re: [Update][PATCH] ACPI: Rework acpi_get_child() to be more efficient

[Rafael J. Wysocki]

On Wednesday, December 26, 2012 11:42:49 PM Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: ACPI: Rework acpi_get_child() to be more efficient
> 
> Observe that acpi_get_child() doesn't need to use the helper
> struct acpi_find_child structure and change it to work without it.
> Also, using acpi_get_object_info() to get the output of _ADR for the
> given device is overkill, because that function does much more than
> just evaluating _ADR (let alone the additional memory allocation
> done by it).
> 
> Moreover, acpi_get_child() doesn't need to loop any more once it has
> found a matching handle, so make it stop in that case.  To prevent
> the results from changing, make it use do_acpi_find_child() as
> a post-order callback.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> The difference from the previous version is the replacement of
> acpi_get_object_info() with the direct execution of _ADR, which is
> more straightforward.

There seem to be no objections, most likely due to the holiday period
that has just ended, but if there are still none, I'm going to push this
for v3.9.

Thanks,
Rafael


> ---
>  drivers/acpi/glue.c |   33 ++++++++++++---------------------
>  1 file changed, 12 insertions(+), 21 deletions(-)
> 
> Index: linux/drivers/acpi/glue.c
> ===================================================================
> --- linux.orig/drivers/acpi/glue.c
> +++ linux/drivers/acpi/glue.c
> @@ -93,40 +93,31 @@ static int acpi_find_bridge_device(struc
>  	return ret;
>  }
>  
> -/* Get device's handler per its address under its parent */
> -struct acpi_find_child {
> -	acpi_handle handle;
> -	u64 address;
> -};
> -
> -static acpi_status
> -do_acpi_find_child(acpi_handle handle, u32 lvl, void *context, void **rv)
> +static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
> +				      void *addr_p, void **ret_p)
>  {
> +	unsigned long long addr;
>  	acpi_status status;
> -	struct acpi_device_info *info;
> -	struct acpi_find_child *find = context;
>  
> -	status = acpi_get_object_info(handle, &info);
> -	if (ACPI_SUCCESS(status)) {
> -		if ((info->address == find->address)
> -			&& (info->valid & ACPI_VALID_ADR))
> -			find->handle = handle;
> -		kfree(info);
> +	status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
> +	if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
> +		*ret_p = handle;
> +		return AE_CTRL_TERMINATE;
>  	}
>  	return AE_OK;
>  }
>  
>  acpi_handle acpi_get_child(acpi_handle parent, u64 address)
>  {
> -	struct acpi_find_child find = { NULL, address };
> +	void *ret = NULL;
>  
>  	if (!parent)
>  		return NULL;
> -	acpi_walk_namespace(ACPI_TYPE_DEVICE, parent,
> -			    1, do_acpi_find_child, NULL, &find, NULL);
> -	return find.handle;
> -}
>  
> +	acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, 1, NULL,
> +			    do_acpi_find_child, &address, &ret);
> +	return (acpi_handle)ret;
> +}
>  EXPORT_SYMBOL(acpi_get_child);
>  
>  static int acpi_bind_one(struct device *dev, acpi_handle handle)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.