[SRU][M][PATCH v5 1/2] ACPI: utils: Dynamically determine acpi_handle_list size
ivanhu
ivan.hu at canonical.com
Tue Mar 5 04:01:24 UTC 2024
sure, have sent out v6
Cheers,
Ivan
On 3/4/24 20:54, Andrei Gherzan wrote:
> On 24/03/01 03:51PM, Ivan Hu wrote:
>> From: "Rafael J. Wysocki" <rafael.j.wysocki at intel.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/2049733
>>
>> 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 <vi at endrift.com>
>> Signed-off-by: Vicki Pfau <vi at endrift.com>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
>> (backported from commit 2e57d10a6591560724b80a628235559571f4cb8d)
>> [Ivan Hu: ignore irrelevant moving PSL patches and rearrange the memset and compare]
>
> Is the backport issue you've hit here this one?
> https://lore.kernel.org/lkml/2924343.e9J7NaK4W3@kreacher/
>
> I propose to include that as a 3rd patch too (as it is a fix of the
> initial one).
>
>> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
>> ---
>> drivers/acpi/acpi_lpss.c | 10 ++-
>> drivers/acpi/scan.c | 1 +
>> drivers/acpi/thermal.c | 29 ++++++---
>> drivers/acpi/utils.c | 61 ++++++++++++++++++-
>> .../platform/surface/surface_acpi_notify.c | 10 ++-
>> include/acpi/acpi_bus.h | 9 ++-
>> 6 files changed, 100 insertions(+), 20 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 c1cb6fcd7930..71b302c16a87 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -2034,6 +2034,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 3163a40f02e3..91aeea3ad766 100644
>> --- a/drivers/acpi/thermal.c
>> +++ b/drivers/acpi/thermal.c
>> @@ -190,7 +190,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
>> {
>> acpi_status status;
>> unsigned long long tmp;
>> - struct acpi_handle_list devices;
>> + struct acpi_handle_list devices = { 0 };
>> bool valid = false;
>> int i;
>>
>> @@ -294,7 +294,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
>> }
>> }
>> if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.valid) {
>> - memset(&devices, 0, sizeof(struct acpi_handle_list));
>> +
>> status = acpi_evaluate_reference(tz->device->handle, "_PSL",
>> NULL, &devices);
>> if (ACPI_FAILURE(status)) {
>> @@ -305,12 +305,12 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
>> tz->trips.passive.valid = true;
>> }
>>
>> - if (memcmp(&tz->trips.passive.devices, &devices,
>> - sizeof(struct acpi_handle_list))) {
>> - memcpy(&tz->trips.passive.devices, &devices,
>> - sizeof(struct acpi_handle_list));
>> - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
>> + if (acpi_handle_list_equal(&tz->trips.passive.devices, &devices)) {
>> + acpi_handle_list_free(&devices);
>> + return 0;
>> }
>> + ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
>> + acpi_handle_list_replace(&tz->trips.passive.devices, &devices);
>> }
>> if ((flag & ACPI_TRIPS_PASSIVE) || (flag & ACPI_TRIPS_DEVICES)) {
>> if (valid != tz->trips.passive.valid)
>> @@ -959,6 +959,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.devices);
>> + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
>> + acpi_handle_list_free(&tz->trips.active[i].devices);
>> +
>> + kfree(tz);
>> +}
>> +
>> static int acpi_thermal_add(struct acpi_device *device)
>> {
>> struct acpi_thermal *tz;
>> @@ -996,7 +1007,7 @@ static int acpi_thermal_add(struct acpi_device *device)
>> goto end;
>>
>> free_memory:
>> - kfree(tz);
>> + acpi_thermal_free_thermal_zone(tz);
>> end:
>> return result;
>> }
>> @@ -1012,7 +1023,7 @@ static void acpi_thermal_remove(struct acpi_device *device)
>> tz = acpi_driver_data(device);
>>
>> 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 db7514033a96..d5de93e1ac7c 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -12,11 +12,9 @@
>> #include <linux/device.h>
>> #include <linux/property.h>
>>
>> -/* 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);
>> --
>> 2.34.1
>>
>>
>> --
>> kernel-team mailing list
>> kernel-team at lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
More information about the kernel-team
mailing list