Re: [PATCH] cpufreq: schedutil: Don't skip freq update when limits change

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Doug Smythies <dsmythies@telus.net>
Cc: 'Rafael Wysocki' <rjw@rjwysocki.net>,
	'Ingo Molnar' <mingo@redhat.com>,
	'Peter Zijlstra' <peterz@infradead.org>,
	linux-pm@vger.kernel.org,
	'Vincent Guittot' <vincent.guittot@linaro.org>,
	joel@joelfernandes.org, "'v4 . 18+'" <stable@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] cpufreq: schedutil: Don't skip freq update when limits change
Date: Tue, 23 Jul 2019 14:45:51 +0530	[thread overview]
Message-ID: <20190723091551.nchopfpqlmdmzvge@vireshk-i7> (raw)
In-Reply-To: <001201d54125$a6a82350$f3f869f0$@net>

On 23-07-19, 00:10, Doug Smythies wrote:
> On 2019.07.21 23:52 Viresh Kumar wrote:
> 
> > To avoid reducing the frequency of a CPU prematurely, we skip reducing
> > the frequency if the CPU had been busy recently.
> > 
> > This should not be done when the limits of the policy are changed, for
> > example due to thermal throttling. We should always get the frequency
> > within limits as soon as possible.
> >
> > Fixes: ecd288429126 ("cpufreq: schedutil: Don't set next_freq to UINT_MAX")
> > Cc: v4.18+ <stable@vger.kernel.org> # v4.18+
> > Reported-by: Doug Smythies <doug.smythies@gmail.com>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > @Doug: Please try this patch, it must fix the issue you reported.
> 
> It fixes the driver = acpi-cpufreq ; governor = schedutil test case
> It does not fix the driver = intel_cpufreq ; governor = schedutil test case
> 
> I have checked my results twice, but will check again in the day or two.

The patch you tried to revert wasn't doing any driver specific stuff
but only schedutil. If that revert fixes your issue with both the
drivers, then this patch should do it as well.

I am clueless now on what can go wrong with intel_cpufreq driver with
schedutil now.

Though there is one difference between intel_cpufreq and acpi_cpufreq,
intel_cpufreq has fast_switch_possible=true and so it uses slightly
different path in schedutil. I tried to look from that perspective as
well but couldn't find anything wrong.

If you still find intel_cpufreq to be broken, even with this patch,
please set fast_switch_possible=false instead of true in
__intel_pstate_cpu_init() and try tests again. That shall make it very
much similar to acpi-cpufreq driver.

-- 
viresh