[v4,1/6] thermal/core: Add user thresholds support

The user thresholds mechanism is a way to have the userspace to tell
the thermal framework to send a notification when a temperature limit
is crossed. There is no id, no hysteresis, just the temperature and
the direction of the limit crossing. That means we can be notified
when a threshold is crossed the way up only, or the way down only or
both ways. That allows to create hysteresis values if it is needed.

A threshold can be added, deleted or flushed. The latter means all
thresholds belonging to a thermal zone will be deleted.

When a threshold is added:

 - if the same threshold (temperature and direction) exists, an error
   is returned

 - if a threshold is specified with the same temperature but a
   different direction, the specified direction is added

 - if there is no threshold with the same temperature then it is
   created

When a threshold is deleted:

 - if the same threshold (temperature and direction) exists, it is
   deleted

 - if a threshold is specified with the same temperature but a
   different direction, the specified direction is removed

 - if there is no threshold with the same temperature, then an error
   is returned

When the threshold are flushed:

 - All thresholds related to a thermal zone are deleted

When a threshold is crossed:

 - the userspace does not need to know which threshold(s) have been
   crossed, it will be notified with the current temperature and the
   previous temperature

 - if multiple thresholds have been crossed between two updates only
   one notification will be send to the userspace, it is pointless to
   send a notification per thresholds crossed as the userspace can
   handle that easily when it has the temperature delta information

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/Makefile             |   1 +
 drivers/thermal/thermal_core.h       |   2 +
 drivers/thermal/thermal_thresholds.c | 229 +++++++++++++++++++++++++++
 drivers/thermal/thermal_thresholds.h |  19 +++
 include/linux/thermal.h              |   3 +
 include/uapi/linux/thermal.h         |   2 +
 6 files changed, 256 insertions(+)
 create mode 100644 drivers/thermal/thermal_thresholds.c
 create mode 100644 drivers/thermal/thermal_thresholds.h

On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> The user thresholds mechanism is a way to have the userspace to tell
> the thermal framework to send a notification when a temperature limit
> is crossed. There is no id, no hysteresis, just the temperature and
> the direction of the limit crossing. That means we can be notified
> when a threshold is crossed the way up only, or the way down only or
> both ways. That allows to create hysteresis values if it is needed.
>
> A threshold can be added, deleted or flushed. The latter means all
> thresholds belonging to a thermal zone will be deleted.
>
> When a threshold is added:
>
>  - if the same threshold (temperature and direction) exists, an error
>    is returned
>
>  - if a threshold is specified with the same temperature but a
>    different direction, the specified direction is added
>
>  - if there is no threshold with the same temperature then it is
>    created
>
> When a threshold is deleted:
>
>  - if the same threshold (temperature and direction) exists, it is
>    deleted
>
>  - if a threshold is specified with the same temperature but a
>    different direction, the specified direction is removed
>
>  - if there is no threshold with the same temperature, then an error
>    is returned
>
> When the threshold are flushed:
>
>  - All thresholds related to a thermal zone are deleted
>
> When a threshold is crossed:
>
>  - the userspace does not need to know which threshold(s) have been
>    crossed, it will be notified with the current temperature and the
>    previous temperature
>
>  - if multiple thresholds have been crossed between two updates only
>    one notification will be send to the userspace, it is pointless to
>    send a notification per thresholds crossed as the userspace can
>    handle that easily when it has the temperature delta information
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/Makefile             |   1 +
>  drivers/thermal/thermal_core.h       |   2 +
>  drivers/thermal/thermal_thresholds.c | 229 +++++++++++++++++++++++++++
>  drivers/thermal/thermal_thresholds.h |  19 +++
>  include/linux/thermal.h              |   3 +
>  include/uapi/linux/thermal.h         |   2 +
>  6 files changed, 256 insertions(+)
>  create mode 100644 drivers/thermal/thermal_thresholds.c
>  create mode 100644 drivers/thermal/thermal_thresholds.h
>
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 41c4d56beb40..1e1559bb971e 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,6 +6,7 @@ CFLAGS_thermal_core.o           := -I$(src)
>  obj-$(CONFIG_THERMAL)          += thermal_sys.o
>  thermal_sys-y                  += thermal_core.o thermal_sysfs.o
>  thermal_sys-y                  += thermal_trip.o thermal_helpers.o
> +thermal_sys-y                  += thermal_thresholds.o
>
>  # netlink interface to manage the thermal framework
>  thermal_sys-$(CONFIG_THERMAL_NETLINK)          += thermal_netlink.o
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 50b858aa173a..8f320d17d927 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -13,6 +13,7 @@
>  #include <linux/thermal.h>
>
>  #include "thermal_netlink.h"
> +#include "thermal_thresholds.h"
>  #include "thermal_debugfs.h"
>
>  struct thermal_attr {
> @@ -139,6 +140,7 @@ struct thermal_zone_device {
>  #ifdef CONFIG_THERMAL_DEBUGFS
>         struct thermal_debugfs *debugfs;
>  #endif
> +       struct list_head user_thresholds;
>         struct thermal_trip_desc trips[] __counted_by(num_trips);
>  };
>
> diff --git a/drivers/thermal/thermal_thresholds.c b/drivers/thermal/thermal_thresholds.c
> new file mode 100644
> index 000000000000..f33b6d5474d8
> --- /dev/null
> +++ b/drivers/thermal/thermal_thresholds.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2024 Linaro Limited
> + *
> + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> + *
> + * Thermal thresholds
> + */
> +#include <linux/list.h>
> +#include <linux/list_sort.h>
> +#include <linux/slab.h>
> +
> +#include "thermal_core.h"
> +#include "thermal_thresholds.h"
> +
> +int thermal_thresholds_init(struct thermal_zone_device *tz)
> +{
> +       INIT_LIST_HEAD(&tz->user_thresholds);
> +
> +       return 0;
> +}
> +
> +void thermal_thresholds_flush(struct thermal_zone_device *tz)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *entry, *tmp;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       list_for_each_entry_safe(entry, tmp, thresholds, list_node) {
> +               list_del(&entry->list_node);
> +               kfree(entry);
> +       }
> +
> +       __thermal_zone_device_update(tz, THERMAL_TZ_FLUSH_THRESHOLDS);
> +}
> +
> +void thermal_thresholds_exit(struct thermal_zone_device *tz)
> +{
> +       thermal_thresholds_flush(tz);
> +}
> +
> +static int __thermal_thresholds_cmp(void *data,
> +                                   const struct list_head *l1,
> +                                   const struct list_head *l2)
> +{
> +       struct user_threshold *t1 = container_of(l1, struct user_threshold, list_node);
> +       struct user_threshold *t2 = container_of(l2, struct user_threshold, list_node);
> +
> +       return t1->temperature - t2->temperature;
> +}
> +
> +static struct user_threshold *__thermal_thresholds_find(const struct list_head *thresholds,
> +                                                       int temperature)
> +{
> +       struct user_threshold *t;
> +
> +       list_for_each_entry(t, thresholds, list_node)
> +               if (t->temperature == temperature)
> +                       return t;
> +
> +       return NULL;
> +}
> +
> +static bool __thermal_threshold_is_crossed(struct user_threshold *threshold, int temperature,
> +                                          int last_temperature, int direction,
> +                                          int *low, int *high)
> +{
> +
> +       if (temperature >= threshold->temperature) {
> +               if (threshold->temperature > *low &&
> +                   THERMAL_THRESHOLD_WAY_DOWN & threshold->direction)
> +                       *low = threshold->temperature;
> +
> +               if (last_temperature < threshold->temperature &&
> +                   threshold->direction & direction)
> +                       return true;
> +       } else {
> +               if (threshold->temperature < *high && THERMAL_THRESHOLD_WAY_UP
> +                   & threshold->direction)
> +                       *high = threshold->temperature;
> +
> +               if (last_temperature >= threshold->temperature &&
> +                   threshold->direction & direction)
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
> +static bool thermal_thresholds_handle_raising(struct list_head *thresholds, int temperature,
> +                                             int last_temperature, int *low, int *high)
> +{
> +       struct user_threshold *t;
> +
> +       list_for_each_entry(t, thresholds, list_node) {
> +               if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> +                                                  THERMAL_THRESHOLD_WAY_UP, low, high))
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
> +static bool thermal_thresholds_handle_dropping(struct list_head *thresholds, int temperature,
> +                                              int last_temperature, int *low, int *high)
> +{
> +       struct user_threshold *t;
> +
> +       list_for_each_entry_reverse(t, thresholds, list_node) {
> +               if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> +                                                  THERMAL_THRESHOLD_WAY_DOWN, low, high))
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
> +void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +
> +       int temperature = tz->temperature;
> +       int last_temperature = tz->last_temperature;
> +       bool notify;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       /*
> +        * We need a second update in order to detect a threshold being crossed
> +        */
> +       if (last_temperature == THERMAL_TEMP_INVALID)
> +               return;
> +
> +       /*
> +        * The temperature is stable, so obviously we can not have
> +        * crossed a threshold.
> +        */
> +       if (last_temperature == temperature)
> +               return;
> +
> +       /*
> +        * Since last update the temperature:
> +        * - increased : thresholds are crossed the way up
> +        * - decreased : thresholds are crossed the way down
> +        */
> +       if (temperature > last_temperature)
> +               notify = thermal_thresholds_handle_raising(thresholds, temperature,
> +                                                          last_temperature, low, high);
> +       else
> +               notify = thermal_thresholds_handle_dropping(thresholds, temperature,
> +                                                           last_temperature, low, high);
> +
> +       if (notify)
> +               pr_debug("A threshold has been crossed the way %s, with a temperature=%d, last_temperature=%d\n",
> +                        temperature > last_temperature ? "up" : "down", temperature, last_temperature);
> +}
> +
> +int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *t;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       t = __thermal_thresholds_find(thresholds, temperature);
> +       if (t) {
> +               if (t->direction == direction)
> +                       return -EEXIST;
> +
> +               t->direction |= direction;
> +       } else {
> +
> +               t = kmalloc(sizeof(*t), GFP_KERNEL);
> +               if (!t)
> +                       return -ENOMEM;
> +
> +               INIT_LIST_HEAD(&t->list_node);
> +               t->temperature = temperature;
> +               t->direction = direction;
> +               list_add(&t->list_node, thresholds);
> +               list_sort(NULL, thresholds, __thermal_thresholds_cmp);
> +       }
> +
> +       __thermal_zone_device_update(tz, THERMAL_TZ_ADD_THRESHOLD);
> +
> +       return 0;
> +}
> +
> +int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *t;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       t = __thermal_thresholds_find(thresholds, temperature);
> +       if (!t)
> +               return -ENOENT;
> +
> +       if (t->direction == direction) {
> +               list_del(&t->list_node);
> +               kfree(t);
> +       } else {
> +               t->direction &= ~direction;
> +       }
> +
> +       __thermal_zone_device_update(tz, THERMAL_TZ_DEL_THRESHOLD);
> +
> +       return 0;
> +}
> +
> +int thermal_thresholds_for_each(struct thermal_zone_device *tz,
> +                               int (*cb)(struct user_threshold *, void *arg), void *arg)
> +{
> +       struct list_head *thresholds = &tz->user_thresholds;
> +       struct user_threshold *entry;
> +       int ret;
> +
> +       lockdep_assert_held(&tz->lock);
> +
> +       list_for_each_entry(entry, thresholds, list_node) {
> +               ret = cb(entry, arg);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/thermal/thermal_thresholds.h b/drivers/thermal/thermal_thresholds.h
> new file mode 100644
> index 000000000000..232f4e8089af
> --- /dev/null
> +++ b/drivers/thermal/thermal_thresholds.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __THERMAL_THRESHOLDS_H__
> +#define __THERMAL_THRESHOLDS_H__
> +
> +struct user_threshold {
> +       struct list_head list_node;
> +       int temperature;
> +       int direction;
> +};
> +
> +int thermal_thresholds_init(struct thermal_zone_device *tz);
> +void thermal_thresholds_exit(struct thermal_zone_device *tz);
> +void thermal_thresholds_flush(struct thermal_zone_device *tz);
> +void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high);
> +int thermal_thresholds_add(struct thermal_zone_device *tz, int temperature, int direction);
> +int thermal_thresholds_delete(struct thermal_zone_device *tz, int temperature, int direction);
> +int thermal_thresholds_for_each(struct thermal_zone_device *tz,
> +                               int (*cb)(struct user_threshold *, void *arg), void *arg);
> +#endif
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 25ea8fe2313e..bcaa92732e14 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -56,6 +56,9 @@ enum thermal_notify_event {
>         THERMAL_TZ_UNBIND_CDEV, /* Cooling dev is unbind from the thermal zone */
>         THERMAL_INSTANCE_WEIGHT_CHANGED, /* Thermal instance weight changed */
>         THERMAL_TZ_RESUME, /* Thermal zone is resuming after system sleep */
> +       THERMAL_TZ_ADD_THRESHOLD, /* Threshold added */
> +       THERMAL_TZ_DEL_THRESHOLD, /* Threshold deleted */
> +       THERMAL_TZ_FLUSH_THRESHOLDS, /* All thresholds deleted */
>  };
>
>  /**
> diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
> index fc78bf3aead7..3e7c1c2e71a7 100644
> --- a/include/uapi/linux/thermal.h
> +++ b/include/uapi/linux/thermal.h
> @@ -3,6 +3,8 @@
>  #define _UAPI_LINUX_THERMAL_H
>
>  #define THERMAL_NAME_LENGTH    20
> +#define THERMAL_THRESHOLD_WAY_UP       0x1
> +#define THERMAL_THRESHOLD_WAY_DOWN     0x2

It would be somewhat better to use BIT(0) and BIT(1) here IMO, but
apart from that this patch and patch [2/6] are fine with me (even
though my implementation of threshold crossing detection would be
different).

I still have questions regarding patch [3/6] though, but I'll need to
take a fresh look at it tomorrow.

>
>  enum thermal_device_mode {
>         THERMAL_DEVICE_DISABLED = 0,
> --
> 2.43.0
>

Hi Rafael,

On 02/10/2024 14:22, Rafael J. Wysocki wrote:
> On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:
>>>
>>> The user thresholds mechanism is a way to have the userspace to tell
>>> the thermal framework to send a notification when a temperature limit
>>> is crossed. There is no id, no hysteresis, just the temperature and
>>> the direction of the limit crossing. That means we can be notified
>>> when a threshold is crossed the way up only, or the way down only or
>>> both ways. That allows to create hysteresis values if it is needed.

[ ... ]


> I'm inclined to apply these 2 patches with the change mentioned above,
> so that I can base my 6.13 work on them.

I was expecting you to pick these two patches and do the modifications 
but I don't see them in your tree. Did I misunderstood your comment?

On 09/10/2024 17:59, Rafael J. Wysocki wrote:
> Hi Daniel,
> 
> On Wed, Oct 9, 2024 at 5:40 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>>
>> Hi Rafael,
>>
>> On 02/10/2024 14:22, Rafael J. Wysocki wrote:
>>> On Tue, Oct 1, 2024 at 9:57 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>>
>>>> On Mon, Sep 23, 2024 at 12:00 PM Daniel Lezcano
>>>> <daniel.lezcano@linaro.org> wrote:
>>>>>
>>>>> The user thresholds mechanism is a way to have the userspace to tell
>>>>> the thermal framework to send a notification when a temperature limit
>>>>> is crossed. There is no id, no hysteresis, just the temperature and
>>>>> the direction of the limit crossing. That means we can be notified
>>>>> when a threshold is crossed the way up only, or the way down only or
>>>>> both ways. That allows to create hysteresis values if it is needed.
>>
>> [ ... ]
>>
>>
>>> I'm inclined to apply these 2 patches with the change mentioned above,
>>> so that I can base my 6.13 work on them.
>>
>> I was expecting you to pick these two patches and do the modifications
>> but I don't see them in your tree. Did I misunderstood your comment?
> 
> That's still the plan, I've been waiting for you to respond and confirm.
> 
> I gather that this is OK then.

Yes, it is ;)