[PATCH v3 1/2] thermal/core: Make the userspace governor deprecated

[PATCH v3 1/2] thermal/core: Make the userspace governor deprecated

[Daniel Lezcano]

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 <daniel.lezcano@linaro.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 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

[PATCH v3 2/2] thermal/core: Deprecate changing cooling device state from userspace

[Daniel Lezcano]

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 <daniel.lezcano@linaro.org>
---
 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

Re: [PATCH v3 2/2] thermal/core: Deprecate changing cooling device state from userspace

[Rafael J. Wysocki]

On Tue, Oct 19, 2021 at 6:35 PM Daniel Lezcano
<daniel.lezcano@linaro.org> 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 <daniel.lezcano@linaro.org>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  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
>

Re: [PATCH v3 1/2] thermal/core: Make the userspace governor deprecated

[Daniel Lezcano]

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 <daniel.lezcano@linaro.org>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  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

Re: [PATCH v3 1/2] thermal/core: Make the userspace governor deprecated

[Lukasz Luba]

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 <daniel.lezcano@linaro.org>
>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> ---
>>   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 <lukasz.luba@arm.com>

Regards,
Lukasz

Re: [PATCH v3 1/2] thermal/core: Make the userspace governor deprecated

[Daniel Lezcano]

On 21/10/2021 17:30, Lukasz Luba wrote:


[ ... ]

>>
> 
> 
> You can add:
> 
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
> 

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

[thermal: thermal/next] thermal/core: Deprecate changing cooling device state from userspace

[thermal-bot for Daniel Lezcano]

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     a67a46af4ad6342378e332b7420c1d1a2818c53f
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//a67a46af4ad6342378e332b7420c1d1a2818c53f
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 19 Oct 2021 18:35:06 +02:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Thu, 21 Oct 2021 17:35:11 +02:00

thermal/core: Deprecate changing cooling device state from userspace

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 <daniel.lezcano@linaro.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Link: https://lore.kernel.org/r/20211019163506.2831454-2-daniel.lezcano@linaro.org
---
 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 1c4aac8..f154bad 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;
 

[thermal: thermal/next] thermal/core: Make the userspace governor deprecated

[thermal-bot for Daniel Lezcano]

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     0275c9fb0eff3f77d437ab8a95b16e6f8f80b7f2
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//0275c9fb0eff3f77d437ab8a95b16e6f8f80b7f2
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 19 Oct 2021 18:35:05 +02:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Thu, 21 Oct 2021 17:35:05 +02:00

thermal/core: Make the userspace governor deprecated

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 <daniel.lezcano@linaro.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Link: https://lore.kernel.org/r/20211019163506.2831454-1-daniel.lezcano@linaro.org
---
 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 82a7198..f4fe050 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);