On 2/17/21 10:39 AM, Viresh Kumar wrote:
On 17-02-21, 10:29, Lukasz Luba wrote:
On 2/17/21 5:48 AM, Viresh Kumar wrote:
freq_qos_update_request() returns 1 if the effective constraint value
has changed, 0 if the effective constraint value has not changed, or a
negative error code on failures.
The frequency constraints for CPUs can be set by different parts of the
kernel. If the maximum frequency constraint set by other parts of the
kernel are set at a lower value than the one corresponding to cooling
state 0, then we will never be able to cool down the system as
freq_qos_update_request() will keep on returning 0 and we will skip
updating cpufreq_state and thermal pressure.
To be precised, thermal pressure signal is not so important in this
mechanism and the 'cpufreq_state' has changed recently:
Right, I wasn't concerned only about no thermal cooling, but both
thermal cooling and pressure.
236761f19a4f373354 thermal/drivers/cpufreq_cooling: Update cpufreq_state
only if state has changed
This moved the assignment to a more logical place for me, i.e. not to
do that on errors, just that the block in which it landed may not get
called at all :(
Fix that by doing the updates even in the case where
freq_qos_update_request() returns 0, as we have effectively set the
constraint to a new value even if the consolidated value of the
actual constraint is unchanged because of external factors.
Cc: v5.7+ <stable@xxxxxxxxxxxxxxx> # v5.7+
Reported-by: Thara Gopinath <thara.gopinath@xxxxxxxxxx>
Fixes: f12e4f66ab6a ("thermal/cpu-cooling: Update thermal pressure in case of a maximum frequency capping")
I'm not sure if that f12e4f is the root cause.
Hmm, depends on how we define the problem :)
If this was just about thermal-cooling not happening, then may be yes,
but to me it is rather about mishandled return value of
freq_qos_update_request() which has more than one side effects and so
I went for the main commit.
This is also important as f12e4f66ab6a got merged in 5.7 and 236761f19
merged in 5.11 and this patch needs to get applied in stable kernels
since 5.7 to fix it all.
'to fix it all' - I agree
