Re: [PATCH V3 3/3] thermal/cpu-cooling: Update thermal pressure in case of a maximum frequency capping

[Thara Gopinath <thara.gopinath@linaro.org>]

On 04/18/2019 05:48 AM, Quentin Perret wrote:
> On Tuesday 16 Apr 2019 at 15:38:41 (-0400), Thara Gopinath wrote:
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> @@ -177,6 +178,9 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
>>  
>>  		if (policy->max > clipped_freq)
>>  			cpufreq_verify_within_limits(policy, 0, clipped_freq);
>> +
>> +		sched_update_thermal_pressure(policy->cpus,
>> +				policy->max, policy->cpuinfo.max_freq);
> 
> Is this something we could do this CPUFreq ? Directly in
> cpufreq_verify_within_limits() perhaps ?
> 
> That would re-define the 'thermal pressure' framework in a more abstract
> way and make the scheduler look at 'frequency capping' events,
> regardless of the reason for capping.
> 
> That would reflect user-defined frequency constraint into cpu_capacity,
> in addition to the thermal stuff. I'm not sure if there is another use
> case for frequency capping ?
Hi Quentin,
Thanks for the review. Sorry for the delay in response as I was on
vacation for the past few days.
I think there is one major difference between user-defined frequency
constraints and frequency constraints due to thermal events in terms of
the time period the system spends in the the constraint state.
Typically, a user constraint lasts for seconds if not minutes and I
think in this case cpu_capacity_orig should reflect this constraint and
not cpu_capacity like this patch set. Also, in case of the user
constraint, there is possibly no need to accumulate and average the
capacity constraints and instantaneous values can be directly applied to
cpu_capacity_orig. On the other hand thermal pressure is more spiky and
sometimes in the order of ms and us requiring the accumulating and
averaging.
> 
> Perhaps the Intel boost stuff could be factored in there ? That is,
> at times when the boost freq is not reachable capacity_of() would appear
> smaller ... Unless this wants to be reflected instantaneously ?
Again, do you think intel boost is more applicable to be reflected in
cpu_capacity_orig and not cpu_capacity?
> 
> Thoughts ?
> Quentin
> 


-- 
Regards
Thara