[v1,2/6] thermal: ACPI: Discard trip table after zone registration
Commit Message
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because the thermal core creates and uses its own copy of the trips
table passed to thermal_zone_device_register_with_trips(), it is not
necessary to hold on to a local copy of it any more after the given
thermal zone has been registered.
Accordingly, modify the ACPI thermal driver to store the trips table
passed to thermal_zone_device_register_with_trips() in a local variable
which is automatically discarded after the zone registration.
Also make some additional code simplifications unlocked by the above
change.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/thermal.c | 57 +++++++++++++++++--------------------------------
1 file changed, 20 insertions(+), 37 deletions(-)
Comments
On Mon, Feb 05, 2024 at 10:15:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Because the thermal core creates and uses its own copy of the trips
> table passed to thermal_zone_device_register_with_trips(), it is not
> necessary to hold on to a local copy of it any more after the given
> thermal zone has been registered.
>
> Accordingly, modify the ACPI thermal driver to store the trips table
> passed to thermal_zone_device_register_with_trips() in a local variable
> which is automatically discarded after the zone registration.
>
> Also make some additional code simplifications unlocked by the above
> change.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/acpi/thermal.c | 57 +++++++++++++++++--------------------------------
> 1 file changed, 20 insertions(+), 37 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -47,6 +47,8 @@
>
> #define ACPI_THERMAL_TRIP_PASSIVE (-1)
>
> +#define ACPI_THERMAL_MAX_NR_TRIPS (ACPI_THERMAL_MAX_ACTIVE + 3)
> +
> /*
> * This exception is thrown out in two cases:
> * 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
> @@ -112,7 +114,6 @@ struct acpi_thermal {
> unsigned long polling_frequency;
> volatile u8 zombie;
> struct acpi_thermal_trips trips;
> - struct thermal_trip *trip_table;
> struct thermal_zone_device *thermal_zone;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> @@ -451,26 +452,19 @@ fail:
> return false;
> }
>
> -static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
> +static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
> {
> - unsigned int count = 0;
> int i;
>
> - if (acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE))
> - count++;
> + acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
>
> for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> - if (acpi_thermal_init_trip(tz, i))
> - count++;
> - else
> + if (!acpi_thermal_init_trip(tz, i))
> break;
> -
> }
>
> while (++i < ACPI_THERMAL_MAX_ACTIVE)
> tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
> -
> - return count;
> }
>
> /* sys I/F for generic thermal sysfs support */
> @@ -662,13 +656,14 @@ static void acpi_thermal_zone_sysfs_remo
> }
>
> static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
> + const struct thermal_trip *trip_table,
> unsigned int trip_count,
> int passive_delay)
> {
> int result;
>
> tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
> - tz->trip_table,
> + trip_table,
> trip_count,
> 0, tz,
> &acpi_thermal_zone_ops,
> @@ -823,10 +818,10 @@ static void acpi_thermal_free_thermal_zo
>
> static int acpi_thermal_add(struct acpi_device *device)
> {
> + struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
> struct acpi_thermal_trip *acpi_trip;
> struct thermal_trip *trip;
> struct acpi_thermal *tz;
> - unsigned int trip_count;
> int crit_temp, hot_temp;
> int passive_delay = 0;
> int result;
> @@ -848,21 +843,10 @@ static int acpi_thermal_add(struct acpi_
> acpi_thermal_aml_dependency_fix(tz);
>
> /* Get trip points [_CRT, _PSV, etc.] (required). */
> - trip_count = acpi_thermal_get_trip_points(tz);
> + acpi_thermal_get_trip_points(tz);
>
> crit_temp = acpi_thermal_get_critical_trip(tz);
> - if (crit_temp != THERMAL_TEMP_INVALID)
> - trip_count++;
> -
> hot_temp = acpi_thermal_get_hot_trip(tz);
> - if (hot_temp != THERMAL_TEMP_INVALID)
> - trip_count++;
> -
> - if (!trip_count) {
> - pr_warn(FW_BUG "No valid trip points!\n");
> - result = -ENODEV;
> - goto free_memory;
> - }
>
> /* Get temperature [_TMP] (required). */
> result = acpi_thermal_get_temperature(tz);
> @@ -881,13 +865,7 @@ static int acpi_thermal_add(struct acpi_
>
> acpi_thermal_guess_offset(tz, crit_temp);
>
> - trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> - if (!trip) {
> - result = -ENOMEM;
> - goto free_memory;
> - }
> -
> - tz->trip_table = trip;
> + trip = trip_table;
>
> if (crit_temp != THERMAL_TEMP_INVALID) {
> trip->type = THERMAL_TRIP_CRITICAL;
> @@ -923,9 +901,17 @@ static int acpi_thermal_add(struct acpi_
> trip++;
> }
>
> - result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
> + if (trip == trip_table) {
> + pr_warn(FW_BUG "No valid trip points!\n");
> + result = -ENODEV;
> + goto free_memory;
> + }
> +
> + result = acpi_thermal_register_thermal_zone(tz, trip_table,
> + trip - trip_table,
> + passive_delay);
> if (result)
> - goto free_trips;
> + goto free_memory;
>
> refcount_set(&tz->thermal_check_count, 3);
> mutex_init(&tz->thermal_check_lock);
> @@ -944,8 +930,6 @@ static int acpi_thermal_add(struct acpi_
> flush_wq:
> flush_workqueue(acpi_thermal_pm_queue);
> acpi_thermal_unregister_thermal_zone(tz);
> -free_trips:
> - kfree(tz->trip_table);
> free_memory:
> acpi_thermal_free_thermal_zone(tz);
>
> @@ -966,7 +950,6 @@ static void acpi_thermal_remove(struct a
>
> flush_workqueue(acpi_thermal_pm_queue);
> acpi_thermal_unregister_thermal_zone(tz);
> - kfree(tz->trip_table);
> acpi_thermal_free_thermal_zone(tz);
> }
>
>
>
>
===================================================================
@@ -47,6 +47,8 @@
#define ACPI_THERMAL_TRIP_PASSIVE (-1)
+#define ACPI_THERMAL_MAX_NR_TRIPS (ACPI_THERMAL_MAX_ACTIVE + 3)
+
/*
* This exception is thrown out in two cases:
* 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
@@ -112,7 +114,6 @@ struct acpi_thermal {
unsigned long polling_frequency;
volatile u8 zombie;
struct acpi_thermal_trips trips;
- struct thermal_trip *trip_table;
struct thermal_zone_device *thermal_zone;
int kelvin_offset; /* in millidegrees */
struct work_struct thermal_check_work;
@@ -451,26 +452,19 @@ fail:
return false;
}
-static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
+static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
{
- unsigned int count = 0;
int i;
- if (acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE))
- count++;
+ acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
- if (acpi_thermal_init_trip(tz, i))
- count++;
- else
+ if (!acpi_thermal_init_trip(tz, i))
break;
-
}
while (++i < ACPI_THERMAL_MAX_ACTIVE)
tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
-
- return count;
}
/* sys I/F for generic thermal sysfs support */
@@ -662,13 +656,14 @@ static void acpi_thermal_zone_sysfs_remo
}
static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
+ const struct thermal_trip *trip_table,
unsigned int trip_count,
int passive_delay)
{
int result;
tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
- tz->trip_table,
+ trip_table,
trip_count,
0, tz,
&acpi_thermal_zone_ops,
@@ -823,10 +818,10 @@ static void acpi_thermal_free_thermal_zo
static int acpi_thermal_add(struct acpi_device *device)
{
+ struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
struct acpi_thermal_trip *acpi_trip;
struct thermal_trip *trip;
struct acpi_thermal *tz;
- unsigned int trip_count;
int crit_temp, hot_temp;
int passive_delay = 0;
int result;
@@ -848,21 +843,10 @@ static int acpi_thermal_add(struct acpi_
acpi_thermal_aml_dependency_fix(tz);
/* Get trip points [_CRT, _PSV, etc.] (required). */
- trip_count = acpi_thermal_get_trip_points(tz);
+ acpi_thermal_get_trip_points(tz);
crit_temp = acpi_thermal_get_critical_trip(tz);
- if (crit_temp != THERMAL_TEMP_INVALID)
- trip_count++;
-
hot_temp = acpi_thermal_get_hot_trip(tz);
- if (hot_temp != THERMAL_TEMP_INVALID)
- trip_count++;
-
- if (!trip_count) {
- pr_warn(FW_BUG "No valid trip points!\n");
- result = -ENODEV;
- goto free_memory;
- }
/* Get temperature [_TMP] (required). */
result = acpi_thermal_get_temperature(tz);
@@ -881,13 +865,7 @@ static int acpi_thermal_add(struct acpi_
acpi_thermal_guess_offset(tz, crit_temp);
- trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
- if (!trip) {
- result = -ENOMEM;
- goto free_memory;
- }
-
- tz->trip_table = trip;
+ trip = trip_table;
if (crit_temp != THERMAL_TEMP_INVALID) {
trip->type = THERMAL_TRIP_CRITICAL;
@@ -923,9 +901,17 @@ static int acpi_thermal_add(struct acpi_
trip++;
}
- result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
+ if (trip == trip_table) {
+ pr_warn(FW_BUG "No valid trip points!\n");
+ result = -ENODEV;
+ goto free_memory;
+ }
+
+ result = acpi_thermal_register_thermal_zone(tz, trip_table,
+ trip - trip_table,
+ passive_delay);
if (result)
- goto free_trips;
+ goto free_memory;
refcount_set(&tz->thermal_check_count, 3);
mutex_init(&tz->thermal_check_lock);
@@ -944,8 +930,6 @@ static int acpi_thermal_add(struct acpi_
flush_wq:
flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
-free_trips:
- kfree(tz->trip_table);
free_memory:
acpi_thermal_free_thermal_zone(tz);
@@ -966,7 +950,6 @@ static void acpi_thermal_remove(struct a
flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
- kfree(tz->trip_table);
acpi_thermal_free_thermal_zone(tz);
}