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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 30 Jun 2017 10:15:47 +0200
From:   Enric Balletbo Serra <eballetbo@...il.com>
To:     Zhang Rui <rui.zhang@...el.com>
Cc:     Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Guenter Roeck <groeck@...omium.org>,
        Sameer Nanda <snanda@...omium.org>,
        Eduardo Valentin <edubezval@...il.com>,
        Linux PM list <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] thermal: core: Allow to disable polling when
 disabling thermal zone.

Hi Rui,

2017-06-30 7:05 GMT+02:00 Zhang Rui <rui.zhang@...el.com>:
> On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
>> Under each thermal zone there is a optional file called "mode".
>> Writing
>> enabled or disabled to this file allows a given thermal zone to be
>> enabled
>> or disabled, but in current code, the monitoring queue doesn't stops.
>> Add
>> the code to disable polling when disabling thermal zone and enable
>> polling
>> when enabling the thermal zone.
>>
>> This patch is based on the original Sameer Nanda <snanda@...omium.org
>> >
>> patch that implemented this idea for the ACPI thermal driver.
>>
>
> Before these two patches, only platform thermal driver cares about
> "mode", thermal core does nothing but invokes platform .set_mode()
> callback upon sysfs I/F write.
> But after this patch set, "mode" becomes something that we should
> take into account in thermal core as well.
> Thus, IMO, we have a couple of things more to do, like the prototype
> patch attached, which I have not tested yet.
>
> From 8bf51fe65bd386b7e8c3c2ca8b9f7d321c92b22f Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@...el.com>
> Date: Fri, 30 Jun 2017 11:11:45 +0800
> Subject: [RFC PATCH] Thermal: introduce thermal zone device mode control
>
> Thermal "mode" sysfs attribute is introduced to enable/disable a thermal zone,
> and .get_mode()/.set_mode() callback is introduced for platform thermal driver
> to enable/disable the hardware thermal control logic. And thermal core takes
> no action upon thermal zone enable/disable.
>
> Actually, this is not quite right because thermal core still pokes those
> disabled thermal zones occasionally, e.g. upon system resume.
>
> To fix this, a new flag 'mode' is introduced in struct thermal_zone_device
> to represent the thermal zone mode, and several decisions have been made
> based on this flag, including
> 1. check the thermal zone mode right after it's registered.
> 2. skip updating thermal zone if the zone is disabled
> 3. stop the polling timer when the thermal zone is disabled
>
> Note: basically, this patch doesn't affect the existing always-enabled
> thermal zones much, with just one exception -
> thermal zone .get_mode() must be well prepared to reflect the real thermal
> zone status upon the thermal zone registration.
>
> Signed-off-by: Zhang Rui <rui.zhang@...el.com>
> ---
>  drivers/thermal/thermal_core.c  | 37 +++++++++++++++++++++++++++++++++++--
>  drivers/thermal/thermal_sysfs.c | 22 ++++++----------------
>  include/linux/thermal.h         |  3 +++
>  3 files changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 5a51c74..89b2254 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -306,9 +306,9 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
>  {
>         mutex_lock(&tz->lock);
>
> -       if (tz->passive)
> +       if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->passive)

tz->mode

>                 thermal_zone_device_set_polling(tz, tz->passive_delay);
> -       else if (tz->polling_delay)
> +       else if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->polling_delay)

tz->mode

>                 thermal_zone_device_set_polling(tz, tz->polling_delay);
>         else
>                 thermal_zone_device_set_polling(tz, 0);
> @@ -464,11 +464,35 @@ static void thermal_zone_device_reset(struct thermal_zone_device *tz)
>                 pos->initialized = false;
>  }
>
> +int thermal_zone_set_mode(struct thermal_zone_device *tz,
> +                                enum thermal_device_mode mode)
> +{
> +       int result;
> +
> +       if (!tz->ops->set_mode)
> +               return -EPERM;
> +
> +       result = tz->ops->set_mode(tz, mode);
> +       if (result)
> +               return result;
> +
> +       if (tz->mode != mode) {
> +               tz->mode = mode;
> +               monitor_thermal_zone(tz);
> +       }
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_set_mode);
> +
>  void thermal_zone_device_update(struct thermal_zone_device *tz,
>                                 enum thermal_notify_event event)
>  {
>         int count;
>
> +       /* Do nothing if the thermal zone is disabled */
> +       if (tz->mode == THERMAL_DEVICE_DISABLED)
> +               return;
> +
>         if (atomic_read(&in_suspend))
>                 return;
>
> @@ -1287,6 +1311,15 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>         INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check);
>
>         thermal_zone_device_reset(tz);
> +
> +       if (tz->ops->get_mode()) {

remove the ()

> +               enum thermal_device_mode mode;
> +
> +               result = tz->ops->get_mode(tz, &mode);
> +               tz->mode = result ? THERMAL_DEVICE_ENABLED : mode;
> +       } else
> +               tz->mode = THERMAL_DEVICE_ENABLED;
> +
>         /* Update the new thermal zone and mark it as already updated. */
>         if (atomic_cmpxchg(&tz->need_update, 1, 0))
>                 thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index a694de9..95d2587 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -51,17 +51,8 @@ static ssize_t
>  mode_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
> -       enum thermal_device_mode mode;
> -       int result;
> -
> -       if (!tz->ops->get_mode)
> -               return -EPERM;
>
> -       result = tz->ops->get_mode(tz, &mode);
> -       if (result)
> -               return result;
> -
> -       return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED ? "enabled"
> +       return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ? "enabled"
>                        : "disabled");
>  }
>
> @@ -70,18 +61,17 @@ mode_store(struct device *dev, struct device_attribute *attr,
>            const char *buf, size_t count)
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
> +       enum thermal_device_mode mode;
>         int result;
>
> -       if (!tz->ops->set_mode)
> -               return -EPERM;
> -
>         if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
> -               result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
> +               mode = THERMAL_DEVICE_ENABLED;
>         else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
> -               result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
> +               mode = THERMAL_DEVICE_DISABLED;
>         else
> -               result = -EINVAL;
> +               return -EINVAL;
>
> +       result = thermal_zone_set_mode(tz, mode)
>         if (result)
>                 return result;
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index dab11f9..2f427de 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -210,6 +210,7 @@ struct thermal_zone_device {
>         struct thermal_attr *trip_type_attrs;
>         struct thermal_attr *trip_hyst_attrs;
>         void *devdata;
> +       enum thermal_device_mode mode;
>         int trips;
>         unsigned long trips_disabled;   /* bitmap for disabled trips */
>         int passive_delay;
> @@ -465,6 +466,8 @@ struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>  int thermal_zone_get_slope(struct thermal_zone_device *tz);
>  int thermal_zone_get_offset(struct thermal_zone_device *tz);
> +int thermal_zone_set_mode(struct thermal_zone_device *tz,
> +                         enum thermal_device_mode mode);
>
>  int get_tz_trend(struct thermal_zone_device *, int);
>  struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> --
> 2.7.4
>

There are some build issues but once fixed the patch works as
expected, many thanks.

Tested-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>

Rui, do you want I send a fixed version of this patch? Or do you want
to fix yourself?

Best regards,
  Enric

Powered by blists - more mailing lists