From 2e57d10a6591560724b80a628235559571f4cb8d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 27 Sep 2023 13:17:25 -0700 Subject: [PATCH] ACPI: utils: Dynamically determine acpi_handle_list size Address a long-standing "TBD" comment in the ACPI headers regarding the number of handles in struct acpi_handle_list. The number 10, which along with the comment dates back to 2.4.23, seems like it may have been arbitrarily chosen and isn't sufficient in all cases [1]. Finally change the code to dynamically determine the size of the handles table in struct acpi_handle_list and allocate it accordingly. Update the users of to struct acpi_handle_list to take the additional dynamic allocation into account. Link: https://lore.kernel.org/linux-acpi/20230809094451.15473-1-ivan.hu@canonical.com # [1] Co-developed-by: Vicki Pfau Signed-off-by: Vicki Pfau Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_lpss.c | 10 ++- drivers/acpi/scan.c | 1 + drivers/acpi/thermal.c | 28 ++++++--- drivers/acpi/utils.c | 61 ++++++++++++++++++- .../platform/surface/surface_acpi_notify.c | 10 ++- include/acpi/acpi_bus.h | 9 ++- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index 539e700de4d2..3e756b9b5aa6 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -578,6 +578,7 @@ static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) { struct acpi_handle_list dep_devices; acpi_status status; + bool ret = false; int i; if (!acpi_has_method(adev->handle, "_DEP")) @@ -591,11 +592,14 @@ static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) } for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == handle) - return true; + if (dep_devices.handles[i] == handle) { + ret = true; + break; + } } - return false; + acpi_handle_list_free(&dep_devices); + return ret; } static void acpi_lpss_link_consumer(struct device *dev1, diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 691d4b7686ee..7cae369ca33b 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2032,6 +2032,7 @@ static u32 acpi_scan_check_dep(acpi_handle handle, bool check_dep) mutex_unlock(&acpi_dep_list_lock); } + acpi_handle_list_free(&dep_devices); return count; } diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index c303688ead71..42a3df9d625d 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -208,7 +208,7 @@ static bool update_trip_devices(struct acpi_thermal *tz, struct acpi_thermal_trip *acpi_trip, int index, bool compare) { - struct acpi_handle_list devices; + struct acpi_handle_list devices = { 0 }; char method[] = "_PSL"; acpi_status status; @@ -218,18 +218,21 @@ static bool update_trip_devices(struct acpi_thermal *tz, method[3] = '0' + index; } - memset(&devices, 0, sizeof(devices)); - status = acpi_evaluate_reference(tz->device->handle, method, NULL, &devices); if (ACPI_FAILURE(status)) { acpi_handle_info(tz->device->handle, "%s evaluation failure\n", method); return false; } - if (compare && memcmp(&acpi_trip->devices, &devices, sizeof(devices))) + if (acpi_handle_list_equal(&acpi_trip->devices, &devices)) { + acpi_handle_list_free(&devices); + return true; + } + + if (compare) ACPI_THERMAL_TRIPS_EXCEPTION(tz, "device"); - memcpy(&acpi_trip->devices, &devices, sizeof(devices)); + acpi_handle_list_replace(&acpi_trip->devices, &devices); return true; } @@ -842,6 +845,17 @@ static void acpi_thermal_check_fn(struct work_struct *work) mutex_unlock(&tz->thermal_check_lock); } +static void acpi_thermal_free_thermal_zone(struct acpi_thermal *tz) +{ + int i; + + acpi_handle_list_free(&tz->trips.passive.trip.devices); + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) + acpi_handle_list_free(&tz->trips.active[i].trip.devices); + + kfree(tz); +} + static int acpi_thermal_add(struct acpi_device *device) { struct acpi_thermal_trip *acpi_trip; @@ -968,7 +982,7 @@ static int acpi_thermal_add(struct acpi_device *device) free_trips: kfree(tz->trip_table); free_memory: - kfree(tz); + acpi_thermal_free_thermal_zone(tz); return result; } @@ -988,7 +1002,7 @@ static void acpi_thermal_remove(struct acpi_device *device) flush_workqueue(acpi_thermal_pm_queue); acpi_thermal_unregister_thermal_zone(tz); - kfree(tz); + acpi_thermal_free_thermal_zone(tz); } #ifdef CONFIG_PM_SLEEP diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 2ea14648a661..c6f83c21bb2a 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -370,7 +370,8 @@ acpi_evaluate_reference(acpi_handle handle, goto end; } - if (package->package.count > ACPI_MAX_HANDLES) { + list->handles = kcalloc(package->package.count, sizeof(*list->handles), GFP_KERNEL); + if (!list->handles) { kfree(package); return AE_NO_MEMORY; } @@ -402,7 +403,8 @@ acpi_evaluate_reference(acpi_handle handle, end: if (ACPI_FAILURE(status)) { list->count = 0; - //kfree(list->handles); + kfree(list->handles); + list->handles = NULL; } kfree(buffer.pointer); @@ -412,6 +414,61 @@ acpi_evaluate_reference(acpi_handle handle, EXPORT_SYMBOL(acpi_evaluate_reference); +/** + * acpi_handle_list_equal - Check if two ACPI handle lists are the same + * @list1: First list to compare. + * @list2: Second list to compare. + * + * Return true if the given ACPI handle lists are of the same size and + * contain the same ACPI handles in the same order. Otherwise, return false. + */ +bool acpi_handle_list_equal(struct acpi_handle_list *list1, + struct acpi_handle_list *list2) +{ + return list1->count == list2->count && + !memcmp(list1->handles, list2->handles, + list1->count * sizeof(acpi_handle)); +} +EXPORT_SYMBOL_GPL(acpi_handle_list_equal); + +/** + * acpi_handle_list_replace - Replace one ACPI handle list with another + * @dst: ACPI handle list to replace. + * @src: Source ACPI handle list. + * + * Free the handles table in @dst, move the handles table from @src to @dst, + * copy count from @src to @dst and clear @src. + */ +void acpi_handle_list_replace(struct acpi_handle_list *dst, + struct acpi_handle_list *src) +{ + if (dst->count) + kfree(dst->handles); + + dst->count = src->count; + dst->handles = src->handles; + + src->handles = NULL; + src->count = 0; +} +EXPORT_SYMBOL_GPL(acpi_handle_list_replace); + +/** + * acpi_handle_list_free - Free the handles table in an ACPI handle list + * @list: ACPI handle list to free. + * + * Free the handles table in @list and clear its count field. + */ +void acpi_handle_list_free(struct acpi_handle_list *list) +{ + if (!list->count) + return; + + kfree(list->handles); + list->count = 0; +} +EXPORT_SYMBOL_GPL(acpi_handle_list_free); + acpi_status acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) { diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c index 897cdd9c3aae..0412a644fece 100644 --- a/drivers/platform/surface/surface_acpi_notify.c +++ b/drivers/platform/surface/surface_acpi_notify.c @@ -741,6 +741,7 @@ static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle) struct acpi_handle_list dep_devices; acpi_handle supplier = ACPI_HANDLE(&pdev->dev); acpi_status status; + bool ret = false; int i; if (!acpi_has_method(handle, "_DEP")) @@ -753,11 +754,14 @@ static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle) } for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == supplier) - return true; + if (dep_devices.handles[i] == supplier) { + ret = true; + break; + } } - return false; + acpi_handle_list_free(&dep_devices); + return ret; } static acpi_status san_consumer_setup(acpi_handle handle, u32 lvl, diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 254685085c82..9920611a6faa 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -12,11 +12,9 @@ #include #include -/* TBD: Make dynamic */ -#define ACPI_MAX_HANDLES 10 struct acpi_handle_list { u32 count; - acpi_handle handles[ACPI_MAX_HANDLES]; + acpi_handle* handles; }; /* acpi_utils.h */ @@ -32,6 +30,11 @@ acpi_evaluate_reference(acpi_handle handle, acpi_string pathname, struct acpi_object_list *arguments, struct acpi_handle_list *list); +bool acpi_handle_list_equal(struct acpi_handle_list *list1, + struct acpi_handle_list *list2); +void acpi_handle_list_replace(struct acpi_handle_list *dst, + struct acpi_handle_list *src); +void acpi_handle_list_free(struct acpi_handle_list *list); acpi_status acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, struct acpi_buffer *status_buf);