| 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: <6d08939a167870ff7c1c83bb254fda5939f1d648.camel@intel.com>
Date: Mon, 18 Jul 2022 13:28:55 +0800
From: Zhang Rui <rui.zhang@...el.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>, rafael@...nel.org
Cc: quic_manafm@...cinc.com, amitk@...nel.org, lukasz.luba@....com,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] thermal/core: Build ascending ordered indexes
for the trip points
On Fri, 2022-07-15 at 23:09 +0200, Daniel Lezcano 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
> thermal events. 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.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> drivers/thermal/thermal_core.c | 62 +++++++++++++++++++++++++++-----
> --
> include/linux/thermal.h | 2 ++
> 2 files changed, 52 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index f66036b3daae..f02f38b66445 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;
> @@ -1165,6 +1166,46 @@ static void bind_tz(struct thermal_zone_device
> *tz)
> mutex_unlock(&thermal_list_lock);
> }
>
> +static void sort_trips_indexes(struct thermal_zone_device *tz)
> +{
> + int i, j;
> +
> + for (i = 0; i < tz->trips; i++)
> + tz->trips_indexes[i] = i;
> +
> + for (i = 0; i < tz->trips; i++) {
> + for (j = i + 1; j < tz->trips; j++) {
> + int t1, t2;
> +
> + tz->ops->get_trip_temp(tz, tz-
> >trips_indexes[i], &t1);
This line can be moved to the upper loop.
> + tz->ops->get_trip_temp(tz, tz-
> >trips_indexes[j], &t2);
> +
what about the disabled trip points?
we should ignore those trip points and check the return value to make
sure we're comparing the valid trip_temp values.
thanks,
rui
> + 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->trips * sizeof(int),
> GFP_KERNEL);
> + if (!tz->trips_indexes)
> + return -ENOMEM;
> +
> + for (i = 0; i < tz->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() - register a new thermal zone
> device
> * @type: the thermal zone device type
> @@ -1196,11 +1237,8 @@ thermal_zone_device_register(const char *type,
> int trips, int mask,
> 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) {
> @@ -1272,12 +1310,9 @@ thermal_zone_device_register(const char *type,
> int trips, int mask,
> if (result)
> goto release_device;
>
> - for (count = 0; count < 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);
> @@ -1290,7 +1325,7 @@ thermal_zone_device_register(const char *type,
> int trips, int mask,
> result = thermal_set_governor(tz, governor);
> if (result) {
> mutex_unlock(&thermal_governor_lock);
> - goto unregister;
> + goto kfree_indexes;
> }
>
> mutex_unlock(&thermal_governor_lock);
> @@ -1298,7 +1333,7 @@ thermal_zone_device_register(const char *type,
> int trips, int mask,
> 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);
> @@ -1319,6 +1354,8 @@ thermal_zone_device_register(const char *type,
> int trips, int mask,
>
> return tz;
>
> +kfree_indexes:
> + kfree(tz->trips_indexes);
> unregister:
> device_del(&tz->device);
> release_device:
> @@ -1387,6 +1424,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 231bac2768fb..4c3b72536772 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -112,6 +112,7 @@ struct thermal_cooling_device {
> * @mode: current mode of this thermal zone
> * @devdata: private pointer for device private data
> * @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.
> @@ -152,6 +153,7 @@ struct thermal_zone_device {
> enum thermal_device_mode mode;
> void *devdata;
> int trips;
> + int *trips_indexes;
> unsigned long trips_disabled; /* bitmap for disabled trips
> */
> unsigned long passive_delay_jiffies;
> unsigned long polling_delay_jiffies;
Powered by blists - more mailing lists