The userspace governor is sending temperature when polling is active
and trip point crossed events. Nothing else.
AFAICT, this governor is used with custom kernels making the userspace
governor co-existing with another governor on the same thermal zone
because there was no notification mechanism, implying a hack in the
framework to support this configuration.
The new netlink thermal notification is able to provide more
information than the userspace governor and give the opportunity to
the users of this governor to replace it by a dedicated notification
framework.
The userspace governor will be removed as its usage is no longer
needed.
Add a warning message to tell the userspace governor is deprecated.
Signed-off-by: Daniel Lezcano <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/gov_user_space.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/thermal/gov_user_space.c b/drivers/thermal/gov_user_space.c
index 82a7198bbe71..f4fe050e1cbc 100644
--- a/drivers/thermal/gov_user_space.c
+++ b/drivers/thermal/gov_user_space.c
@@ -15,6 +15,14 @@
#include "thermal_core.h"
+static int user_space_bind(struct thermal_zone_device *tz)
+{
+ pr_warn("Userspace governor deprecated: use thermal netlink " \
+ "notification instead\n");
+
+ return 0;
+}
+
/**
* notify_user_space - Notifies user space about thermal events
* @tz: thermal_zone_device
@@ -43,5 +51,6 @@ static int notify_user_space(struct thermal_zone_device *tz, int trip)
static struct thermal_governor thermal_gov_user_space = {
.name = "user_space",
.throttle = notify_user_space,
+ .bind_to_tz = user_space_bind,
};
THERMAL_GOVERNOR_DECLARE(thermal_gov_user_space);
--
2.25.1
The cooling devices have their cooling device set_cur_state
read-writable all the time in the sysfs directory, thus allowing the
userspace to act on it.
The thermal framework is wrongly used by userspace as a power capping
framework by acting on the cooling device opaque state. This one then
competes with the in-kernel governor decision.
We have seen in out-of-tree kernels, a big number of devices which are
abusely declaring themselves as cooling device just to act on their
power.
The role of the thermal framework is to protect the junction
temperature of the silicon. Letting the userspace to play with a
cooling device is invalid and potentially dangerous.
The powercap framework is the right framework to do power capping and
moreover it deals with the aggregation via the dev pm qos.
As the userspace governor is marked deprecated and about to be
removed, there is no point to keep this file writable also in the
future.
Emit a warning and deprecate the interface.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_sysfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 1c4aac8464a7..f154bada2906 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -610,6 +610,9 @@ cur_state_store(struct device *dev, struct device_attribute *attr,
unsigned long state;
int result;
+ dev_warn_once(&cdev->device,
+ "Setting cooling device state is deprecated\n");
+
if (sscanf(buf, "%ld\n", &state) != 1)
return -EINVAL;
--
2.25.1
On Tue, Oct 19, 2021 at 6:35 PM Daniel Lezcano
<[email protected]> wrote:
>
> The cooling devices have their cooling device set_cur_state
> read-writable all the time in the sysfs directory, thus allowing the
> userspace to act on it.
>
> The thermal framework is wrongly used by userspace as a power capping
> framework by acting on the cooling device opaque state. This one then
> competes with the in-kernel governor decision.
>
> We have seen in out-of-tree kernels, a big number of devices which are
> abusely declaring themselves as cooling device just to act on their
> power.
>
> The role of the thermal framework is to protect the junction
> temperature of the silicon. Letting the userspace to play with a
> cooling device is invalid and potentially dangerous.
>
> The powercap framework is the right framework to do power capping and
> moreover it deals with the aggregation via the dev pm qos.
>
> As the userspace governor is marked deprecated and about to be
> removed, there is no point to keep this file writable also in the
> future.
>
> Emit a warning and deprecate the interface.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/thermal_sysfs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 1c4aac8464a7..f154bada2906 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -610,6 +610,9 @@ cur_state_store(struct device *dev, struct device_attribute *attr,
> unsigned long state;
> int result;
>
> + dev_warn_once(&cdev->device,
> + "Setting cooling device state is deprecated\n");
> +
> if (sscanf(buf, "%ld\n", &state) != 1)
> return -EINVAL;
>
> --
> 2.25.1
>
Hi Lukasz,
Do you agree with these changes ?
On 19/10/2021 18:35, Daniel Lezcano wrote:
> The userspace governor is sending temperature when polling is active
> and trip point crossed events. Nothing else.
>
> AFAICT, this governor is used with custom kernels making the userspace
> governor co-existing with another governor on the same thermal zone
> because there was no notification mechanism, implying a hack in the
> framework to support this configuration.
>
> The new netlink thermal notification is able to provide more
> information than the userspace governor and give the opportunity to
> the users of this governor to replace it by a dedicated notification
> framework.
>
> The userspace governor will be removed as its usage is no longer
> needed.
>
> Add a warning message to tell the userspace governor is deprecated.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/gov_user_space.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/thermal/gov_user_space.c b/drivers/thermal/gov_user_space.c
> index 82a7198bbe71..f4fe050e1cbc 100644
> --- a/drivers/thermal/gov_user_space.c
> +++ b/drivers/thermal/gov_user_space.c
> @@ -15,6 +15,14 @@
>
> #include "thermal_core.h"
>
> +static int user_space_bind(struct thermal_zone_device *tz)
> +{
> + pr_warn("Userspace governor deprecated: use thermal netlink " \
> + "notification instead\n");
> +
> + return 0;
> +}
> +
> /**
> * notify_user_space - Notifies user space about thermal events
> * @tz: thermal_zone_device
> @@ -43,5 +51,6 @@ static int notify_user_space(struct thermal_zone_device *tz, int trip)
> static struct thermal_governor thermal_gov_user_space = {
> .name = "user_space",
> .throttle = notify_user_space,
> + .bind_to_tz = user_space_bind,
> };
> THERMAL_GOVERNOR_DECLARE(thermal_gov_user_space);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Hi Daniel,
On 10/20/21 11:47 AM, Daniel Lezcano wrote:
>
> Hi Lukasz,
>
> Do you agree with these changes ?
Yes, I agree with these changes.
>
> On 19/10/2021 18:35, Daniel Lezcano wrote:
>> The userspace governor is sending temperature when polling is active
>> and trip point crossed events. Nothing else.
>>
>> AFAICT, this governor is used with custom kernels making the userspace
>> governor co-existing with another governor on the same thermal zone
>> because there was no notification mechanism, implying a hack in the
>> framework to support this configuration.
>>
>> The new netlink thermal notification is able to provide more
>> information than the userspace governor and give the opportunity to
>> the users of this governor to replace it by a dedicated notification
>> framework.
>>
>> The userspace governor will be removed as its usage is no longer
>> needed.
>>
>> Add a warning message to tell the userspace governor is deprecated.
>>
>> Signed-off-by: Daniel Lezcano <[email protected]>
>> Acked-by: Rafael J. Wysocki <[email protected]>
>> ---
>> drivers/thermal/gov_user_space.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/thermal/gov_user_space.c b/drivers/thermal/gov_user_space.c
>> index 82a7198bbe71..f4fe050e1cbc 100644
>> --- a/drivers/thermal/gov_user_space.c
>> +++ b/drivers/thermal/gov_user_space.c
>> @@ -15,6 +15,14 @@
>>
>> #include "thermal_core.h"
>>
>> +static int user_space_bind(struct thermal_zone_device *tz)
>> +{
>> + pr_warn("Userspace governor deprecated: use thermal netlink " \
>> + "notification instead\n");
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * notify_user_space - Notifies user space about thermal events
>> * @tz: thermal_zone_device
>> @@ -43,5 +51,6 @@ static int notify_user_space(struct thermal_zone_device *tz, int trip)
>> static struct thermal_governor thermal_gov_user_space = {
>> .name = "user_space",
>> .throttle = notify_user_space,
>> + .bind_to_tz = user_space_bind,
>> };
>> THERMAL_GOVERNOR_DECLARE(thermal_gov_user_space);
>>
>
>
You can add:
Reviewed-by: Lukasz Luba <[email protected]>
Regards,
Lukasz
On 21/10/2021 17:30, Lukasz Luba wrote:
[ ... ]
>>
>
>
> You can add:
>
> Reviewed-by: Lukasz Luba <[email protected]>
>
Thanks Lukasz
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog