[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e5c96b65-8efe-4209-ad32-1d99c7174794@arm.com>
Date: Thu, 24 Oct 2024 11:14:55 +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>,
Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: Re: [PATCH v1 01/10] thermal: core: Build sorted lists instead of
sorting them later
On 10/16/24 12:21, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Since it is not expected that multiple trip points will be crossed
> in one go very often (if this happens, there are too many trip points
> in the given thermal zone or they are checked too rarely), quite likely
> it is more efficient to build a sorted list of crossed trip points than
> to put them on an unsorted list and sort it later.
>
> Moreover, trip points are often sorted in ascending temperature order
> during thermal zone registration. so building a sorted list out of
nit: small s
> them is quite straightforward and relatively inexpensive.
>
> Accordingly, make handle_thermal_trip() maintain list ordering when
> adding trip points to the lists and get rid of separate list sorting
> in __thermal_zone_device_update().
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/thermal/thermal_core.c | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 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,7 +15,6 @@
> #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>
> @@ -409,6 +408,21 @@ static void handle_critical_trips(struct
> tz->ops.hot(tz);
> }
>
> +static void add_trip_to_sorted_list(struct thermal_trip_desc *td,
> + struct list_head *list)
> +{
> + struct thermal_trip_desc *entry;
> +
> + /* Assume that the new entry is likely to be the last one. */
> + list_for_each_entry_reverse(entry, list, notify_list_node) {
> + if (entry->notify_temp <= td->notify_temp) {
> + list_add(&td->notify_list_node, &entry->notify_list_node);
> + return;
> + }
> + }
> + list_add(&td->notify_list_node, list);
> +}
> +
> static void handle_thermal_trip(struct thermal_zone_device *tz,
> struct thermal_trip_desc *td,
> struct list_head *way_up_list,
> @@ -438,8 +452,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) {
> - list_add(&td->notify_list_node, way_down_list);
> td->notify_temp = trip->temperature - trip->hysteresis;
> + add_trip_to_sorted_list(td, way_down_list);
>
> if (trip->type == THERMAL_TRIP_PASSIVE) {
> tz->passive--;
> @@ -454,8 +468,9 @@ 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.
> */
> - list_add_tail(&td->notify_list_node, way_up_list);
> td->notify_temp = trip->temperature;
> + add_trip_to_sorted_list(td, way_up_list);
> +
> td->threshold -= trip->hysteresis;
>
> if (trip->type == THERMAL_TRIP_PASSIVE)
> @@ -519,16 +534,6 @@ static void thermal_trip_crossed(struct
> thermal_governor_trip_crossed(governor, tz, trip, crossed_up);
> }
>
> -static int thermal_trip_notify_cmp(void *not_used, 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);
> - return tda->notify_temp - tdb->notify_temp;
> -}
> -
> void __thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> @@ -581,11 +586,9 @@ void __thermal_zone_device_update(struct
>
> thermal_zone_set_trips(tz, low, high);
>
> - list_sort(NULL, &way_up_list, thermal_trip_notify_cmp);
> list_for_each_entry(td, &way_up_list, notify_list_node)
> thermal_trip_crossed(tz, &td->trip, governor, true);
>
> - list_sort(NULL, &way_down_list, thermal_trip_notify_cmp);
> list_for_each_entry_reverse(td, &way_down_list, notify_list_node)
> thermal_trip_crossed(tz, &td->trip, governor, false);
>
>
>
>
Make sense, LGTM
Reviewed-by: Lukasz Luba <lukasz.luba@....com>
Powered by blists - more mailing lists