[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 4 Apr 2024 23:53:53 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v3 5/6] thermal: core: Sort trip point crossing
notifications by temperature
On 4/2/24 20:03, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> If multiple trip points are crossed in one go and the trips table in
> the thermal zone device object is not sorted, the corresponding trip
> point crossing notifications sent to user space will not be ordered
> either.
>
> Moreover, if the trips table is sorted by trip temperature in ascending
> order, the trip crossing notifications on the way up will be sent in that
> order too, but the trip crossing notifications on the way down will be
> sent in the reverse order.
>
> This is generally confusing and it is better to make the kernel send the
> notifications in the order of growing (on the way up) or falling (on the
> way down) trip temperature.
>
> To achieve that, instead of sending a trip crossing notification and
> recording a trip crossing event in the statistics right away from
> handle_thermal_trip(), put the trip in question on a list that will be
> sorted by __thermal_zone_device_update() after processing all of the trips
> and before sending the notifications and recording trip crossing events.
>
> Link: https://lore.kernel.org/linux-pm/20240306085428.88011-1-daniel.lezcano@linaro.org/
> Reported-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>
> v2 -> v3: Rebase, update changelog, add notify_temp to struct thermal_trip_desc
>
> v1 -> v2: New patch
>
> ---
> drivers/thermal/thermal_core.c | 41 +++++++++++++++++++++++++++++++++++------
> drivers/thermal/thermal_core.h | 2 ++
> 2 files changed, 37 insertions(+), 6 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/kdev_t.h>
> #include <linux/idr.h>
> +#include <linux/list_sort.h>
> #include <linux/thermal.h>
> #include <linux/reboot.h>
> #include <linux/string.h>
> @@ -361,7 +362,9 @@ static void handle_critical_trips(struct
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz,
> - struct thermal_trip_desc *td)
> + struct thermal_trip_desc *td,
> + struct list_head *way_up_list,
> + struct list_head *way_down_list)
> {
> const struct thermal_trip *trip = &td->trip;
> int old_threshold;
> @@ -387,8 +390,8 @@ static void handle_thermal_trip(struct t
> * In that case, the trip temperature becomes the new threshold.
> */
> if (tz->temperature < trip->temperature - trip->hysteresis) {
> - thermal_notify_tz_trip_down(tz, trip);
> - thermal_debug_tz_trip_down(tz, trip);
> + list_add(&td->notify_list_node, way_down_list);
> + td->notify_temp = trip->temperature - trip->hysteresis;
> } else {
> td->threshold -= trip->hysteresis;
> }
> @@ -398,8 +401,8 @@ static void handle_thermal_trip(struct t
> * if the zone temperature exceeds the trip one. The new
> * threshold is then set to the low temperature of the trip.
> */
> - thermal_notify_tz_trip_up(tz, trip);
> - thermal_debug_tz_trip_up(tz, trip);
> + list_add_tail(&td->notify_list_node, way_up_list);
> + td->notify_temp = trip->temperature;
> td->threshold -= trip->hysteresis;
> }
>
> @@ -452,10 +455,24 @@ static void thermal_zone_device_init(str
> pos->initialized = false;
> }
>
> +static int thermal_trip_notify_cmp(void *ascending, const struct list_head *a,
> + const struct list_head *b)
> +{
> + struct thermal_trip_desc *tda = container_of(a, struct thermal_trip_desc,
> + notify_list_node);
> + struct thermal_trip_desc *tdb = container_of(b, struct thermal_trip_desc,
> + notify_list_node);
> + int ret = tdb->notify_temp - tda->notify_temp;
> +
> + return ascending ? ret : -ret;
> +}
> +
> void __thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> struct thermal_trip_desc *td;
> + LIST_HEAD(way_down_list);
> + LIST_HEAD(way_up_list);
>
> if (tz->suspended)
> return;
> @@ -470,7 +487,19 @@ void __thermal_zone_device_update(struct
> tz->notify_event = event;
>
> for_each_trip_desc(tz, td)
> - handle_thermal_trip(tz, td);
> + handle_thermal_trip(tz, td, &way_up_list, &way_down_list);
> +
> + list_sort(&way_up_list, &way_up_list, thermal_trip_notify_cmp);
> + list_for_each_entry(td, &way_up_list, notify_list_node) {
> + thermal_notify_tz_trip_up(tz, &td->trip);
> + thermal_debug_tz_trip_up(tz, &td->trip);
> + }
> +
> + list_sort(NULL, &way_down_list, thermal_trip_notify_cmp);
> + list_for_each_entry(td, &way_down_list, notify_list_node) {
> + thermal_notify_tz_trip_down(tz, &td->trip);
> + thermal_debug_tz_trip_down(tz, &td->trip);
> + }
>
> monitor_thermal_zone(tz);
> }
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -17,6 +17,8 @@
>
> struct thermal_trip_desc {
> struct thermal_trip trip;
> + struct list_head notify_list_node;
> + int notify_temp;
> int threshold;
> };
>
>
>
>
Reviewed-by: Lukasz Luba <lukasz.luba@....com>
Powered by blists - more mailing lists