[PATCH 0/3] Thermal governors improvements and a fix

[PATCH 0/3] Thermal governors improvements and a fix

[Lukasz Luba]

Hi all,

The patch set introduces one fix (patch 1/3) and two improvements, which
are possible thanks to the new helper function [1].
The patch 1/3 with a fix for fair share thermal governor is also sent
CC'ed stable, but it's hard to point a particular commit, which back
then was for fair_share.c.

The patch set should apply on top of [1].

Regards,
Lukasz

[1] https://lore.kernel.org/linux-pm/20210422114308.29684-2-lukasz.luba@arm.com/

Lukasz Luba (3):
  thermal: fair share: lock thermal zone while looping over instances
  thermal: fair share: use __thermal_cdev_update()
  thermal: power allocator: use __thermal_cdev_update()

 drivers/thermal/gov_fair_share.c      | 11 +++++++----
 drivers/thermal/gov_power_allocator.c |  3 +--
 2 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH 1/3] thermal: fair share: lock thermal zone while looping over instances

[Lukasz Luba]

The tz->lock must be hold during the looping over the instances in that
thermal zone. This lock was missing in the governor code since the
beginning, so it's hard to point into a particular commit.

CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/thermal/gov_fair_share.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index aaa07180ab48..645432ce6365 100644
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -82,6 +82,8 @@ static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 	int total_instance = 0;
 	int cur_trip_level = get_trip_level(tz);
 
+	mutex_lock(&tz->lock);
+
 	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
 		if (instance->trip != trip)
 			continue;
@@ -110,6 +112,8 @@ static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 		mutex_unlock(&instance->cdev->lock);
 		thermal_cdev_update(cdev);
 	}
+
+	mutex_unlock(&tz->lock);
 	return 0;
 }
 
-- 
2.17.1

[PATCH 2/3] thermal: fair share: use __thermal_cdev_update()

[Lukasz Luba]

Use the new helper function and avoid unnecessery second lock/unlock,
which was present in old approach with thermal_cdev_update().

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/thermal/gov_fair_share.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index 645432ce6365..1e5abf4822be 100644
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -107,10 +107,9 @@ static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 		instance->target = get_target_state(tz, cdev, percentage,
 						    cur_trip_level);
 
-		mutex_lock(&instance->cdev->lock);
-		instance->cdev->updated = false;
-		mutex_unlock(&instance->cdev->lock);
-		thermal_cdev_update(cdev);
+		mutex_lock(&cdev->lock);
+		__thermal_cdev_update(cdev);
+		mutex_unlock(&cdev->lock);
 	}
 
 	mutex_unlock(&tz->lock);
-- 
2.17.1

[PATCH 3/3] thermal: power allocator: use __thermal_cdev_update()

[Lukasz Luba]

Use the new helper function and avoid unnecessery second lock/unlock,
which was present in old approach with thermal_cdev_update().

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/thermal/gov_power_allocator.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index a6cdb2e892da..13e375751d22 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -301,9 +301,8 @@ power_actor_set_power(struct thermal_cooling_device *cdev,
 
 	instance->target = clamp_val(state, instance->lower, instance->upper);
 	mutex_lock(&cdev->lock);
-	cdev->updated = false;
+	__thermal_cdev_update(cdev);
 	mutex_unlock(&cdev->lock);
-	thermal_cdev_update(cdev);
 
 	return 0;
 }
-- 
2.17.1

Re: [PATCH 0/3] Thermal governors improvements and a fix

[Daniel Lezcano]

On 22/04/2021 17:36, Lukasz Luba wrote:
> Hi all,
> 
> The patch set introduces one fix (patch 1/3) and two improvements, which
> are possible thanks to the new helper function [1].
> The patch 1/3 with a fix for fair share thermal governor is also sent
> CC'ed stable, but it's hard to point a particular commit, which back
> then was for fair_share.c.
> 
> The patch set should apply on top of [1].
> 
> Regards,
> Lukasz
> 
> [1] https://lore.kernel.org/linux-pm/20210422114308.29684-2-lukasz.luba@arm.com/
> 
> Lukasz Luba (3):
>   thermal: fair share: lock thermal zone while looping over instances
>   thermal: fair share: use __thermal_cdev_update()
>   thermal: power allocator: use __thermal_cdev_update()
> 
>  drivers/thermal/gov_fair_share.c      | 11 +++++++----
>  drivers/thermal/gov_power_allocator.c |  3 +--
>  2 files changed, 8 insertions(+), 6 deletions(-)

Applied, thanks

Two users left of thermal_cdev_update ;)


-- 
<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 0/3] Thermal governors improvements and a fix

[Lukasz Luba]

On 4/22/21 6:31 PM, Daniel Lezcano wrote:
> On 22/04/2021 17:36, Lukasz Luba wrote:
>> Hi all,
>>
>> The patch set introduces one fix (patch 1/3) and two improvements, which
>> are possible thanks to the new helper function [1].
>> The patch 1/3 with a fix for fair share thermal governor is also sent
>> CC'ed stable, but it's hard to point a particular commit, which back
>> then was for fair_share.c.
>>
>> The patch set should apply on top of [1].
>>
>> Regards,
>> Lukasz
>>
>> [1] https://lore.kernel.org/linux-pm/20210422114308.29684-2-lukasz.luba@arm.com/
>>
>> Lukasz Luba (3):
>>    thermal: fair share: lock thermal zone while looping over instances
>>    thermal: fair share: use __thermal_cdev_update()
>>    thermal: power allocator: use __thermal_cdev_update()
>>
>>   drivers/thermal/gov_fair_share.c      | 11 +++++++----
>>   drivers/thermal/gov_power_allocator.c |  3 +--
>>   2 files changed, 8 insertions(+), 6 deletions(-)
> 
> Applied, thanks

thanks!

> 
> Two users left of thermal_cdev_update ;)
> 
> 

True, I didn't dare to touch them, since that would require more
work and understandings :)

Re: [PATCH 0/3] Thermal governors improvements and a fix

[Daniel Lezcano]

On 22/04/2021 19:33, Lukasz Luba wrote:
> 
> 
> On 4/22/21 6:31 PM, Daniel Lezcano wrote:
>> On 22/04/2021 17:36, Lukasz Luba wrote:
>>> Hi all,
>>>
>>> The patch set introduces one fix (patch 1/3) and two improvements, which
>>> are possible thanks to the new helper function [1].
>>> The patch 1/3 with a fix for fair share thermal governor is also sent
>>> CC'ed stable, but it's hard to point a particular commit, which back
>>> then was for fair_share.c.
>>>
>>> The patch set should apply on top of [1].
>>>
>>> Regards,
>>> Lukasz
>>>
>>> [1]
>>> https://lore.kernel.org/linux-pm/20210422114308.29684-2-lukasz.luba@arm.com/
>>>
>>>
>>> Lukasz Luba (3):
>>>    thermal: fair share: lock thermal zone while looping over instances
>>>    thermal: fair share: use __thermal_cdev_update()
>>>    thermal: power allocator: use __thermal_cdev_update()
>>>
>>>   drivers/thermal/gov_fair_share.c      | 11 +++++++----
>>>   drivers/thermal/gov_power_allocator.c |  3 +--
>>>   2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> Applied, thanks
> 
> thanks!
> 
>>
>> Two users left of thermal_cdev_update ;)
>>
>>
> 
> True, I didn't dare to touch them, since that would require more
> work and understandings :)

Yeah I agree. The changes are no trivial ...




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