| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0hj0kMRNBqO_0SqsAAY8Rb8h2NrWOYogDLgGZnCtiTEwg@mail.gmail.com>
Date: Tue, 19 Jul 2022 20:56:26 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Manaf Meethalavalappu Pallikunhi <quic_manafm@...cinc.com>,
"Zhang, Rui" <rui.zhang@...el.com>,
Amit Kucheria <amitk@...nel.org>,
Lukasz Luba <lukasz.luba@....com>,
Linux PM <linux-pm@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/4] thermal/core: Build ascending ordered indexes for
the trip points
On Mon, Jul 18, 2022 at 4:50 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> By convention the trips points are declared in the ascending
> temperature order. However, no specification for the device tree, ACPI
> or documentation tells the trip points must be ordered this way.
>
> In the other hand, we need those to be ordered to browse them at the
s/In/On/
> thermal events.
What if they are all inspected every time?
> But if we assume they are ordered and change the code
> based on this assumption, any platform with shuffled trip points
> description will be broken (if they exist).
>
> Instead of taking the risk of breaking the existing platforms, use an
> array of temperature ordered trip identifiers and make it available
> for the code needing to browse the trip points in an ordered way.
Well, having ops->get_trip_temp() suggests that the trip temperatures
can be dynamic. Is the ordering guaranteed to be preserved in that
case?
Anyway, if they need to be sorted, why don't we just sort them
properly instead of adding this extra array?
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
>
> V4:
> - Fix conflicts due to tz->trips renamed to tz->num_trips
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> drivers/thermal/thermal_core.c | 63 +++++++++++++++++++++++++++-------
> include/linux/thermal.h | 2 ++
> 2 files changed, 53 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index fc6ccc5edbfb..f274dc7d9c48 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -355,7 +355,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> }
>
> static void handle_thermal_trip_crossed(struct thermal_zone_device *tz, int trip,
> - int trip_temp, int trip_hyst, enum thermal_trip_type trip_type)
> + int trip_temp, int trip_hyst,
> + enum thermal_trip_type trip_type)
> {
> if (tz->last_temperature == THERMAL_TEMP_INVALID)
> return;
> @@ -1171,6 +1172,47 @@ static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms
> if (delay_ms > 1000)
> *delay_jiffies = round_jiffies(*delay_jiffies);
> }
> +
> +static void sort_trips_indexes(struct thermal_zone_device *tz)
> +{
> + int i, j;
> +
> + for (i = 0; i < tz->num_trips; i++)
> + tz->trips_indexes[i] = i;
> +
> + for (i = 0; i < tz->num_trips; i++) {
> +
> + for (j = i + 1; j < tz->num_trips; j++) {
> + int t1, t2;
> +
> + tz->ops->get_trip_temp(tz, tz->trips_indexes[i], &t1);
> + tz->ops->get_trip_temp(tz, tz->trips_indexes[j], &t2);
> +
> + if (t1 > t2)
> + swap(tz->trips_indexes[i], tz->trips_indexes[j]);
> + }
> + }
> +}
> +
> +static int thermal_zone_device_trip_init(struct thermal_zone_device *tz)
> +{
> + enum thermal_trip_type trip_type;
> + int trip_temp, i;
> +
> + tz->trips_indexes = kzalloc(tz->num_trips * sizeof(int), GFP_KERNEL);
> + if (!tz->trips_indexes)
> + return -ENOMEM;
> +
> + for (i = 0; i < tz->num_trips; i++) {
> + if (tz->ops->get_trip_type(tz, i, &trip_type) ||
> + tz->ops->get_trip_temp(tz, i, &trip_temp) || !trip_temp)
> + set_bit(i, &tz->trips_disabled);
> + }
> +
> + sort_trips_indexes(tz);
> +
> + return 0;
> +}
>
> /**
> * thermal_zone_device_register_with_trips() - register a new thermal zone device
> @@ -1204,11 +1246,8 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
> int polling_delay)
> {
> struct thermal_zone_device *tz;
> - enum thermal_trip_type trip_type;
> - int trip_temp;
> int id;
> int result;
> - int count;
> struct thermal_governor *governor;
>
> if (!type || strlen(type) == 0) {
> @@ -1281,12 +1320,9 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
> if (result)
> goto release_device;
>
> - for (count = 0; count < num_trips; count++) {
> - if (tz->ops->get_trip_type(tz, count, &trip_type) ||
> - tz->ops->get_trip_temp(tz, count, &trip_temp) ||
> - !trip_temp)
> - set_bit(count, &tz->trips_disabled);
> - }
> + result = thermal_zone_device_trip_init(tz);
> + if (result)
> + goto unregister;
>
> /* Update 'this' zone's governor information */
> mutex_lock(&thermal_governor_lock);
> @@ -1299,7 +1335,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
> result = thermal_set_governor(tz, governor);
> if (result) {
> mutex_unlock(&thermal_governor_lock);
> - goto unregister;
> + goto kfree_indexes;
> }
>
> mutex_unlock(&thermal_governor_lock);
> @@ -1307,7 +1343,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
> if (!tz->tzp || !tz->tzp->no_hwmon) {
> result = thermal_add_hwmon_sysfs(tz);
> if (result)
> - goto unregister;
> + goto kfree_indexes;
> }
>
> mutex_lock(&thermal_list_lock);
> @@ -1328,6 +1364,8 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>
> return tz;
>
> +kfree_indexes:
> + kfree(tz->trips_indexes);
> unregister:
> device_del(&tz->device);
> release_device:
> @@ -1406,6 +1444,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> thermal_remove_hwmon_sysfs(tz);
> ida_simple_remove(&thermal_tz_ida, tz->id);
> ida_destroy(&tz->ida);
> + kfree(tz->trips_indexes);
> mutex_destroy(&tz->lock);
> device_unregister(&tz->device);
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 1386c713885d..4e576184df49 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -125,6 +125,7 @@ struct thermal_cooling_device {
> * @devdata: private pointer for device private data
> * @trips: an array of struct thermal_trip
> * @num_trips: number of trip points the thermal zone supports
> + * @trips_indexes: an array of sorted trip points indexes
> * @trips_disabled; bitmap for disabled trips
> * @passive_delay_jiffies: number of jiffies to wait between polls when
> * performing passive cooling.
> @@ -166,6 +167,7 @@ struct thermal_zone_device {
> void *devdata;
> struct thermal_trip *trips;
> int num_trips;
> + int *trips_indexes;
> unsigned long trips_disabled; /* bitmap for disabled trips */
> unsigned long passive_delay_jiffies;
> unsigned long polling_delay_jiffies;
> --
> 2.25.1
>
Powered by blists - more mailing lists