Re: [RFC PATCH] sched/cpufreq/schedutil: handling urgent frequency requests

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Claudio Scordino <claudio@evidence.eu.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Luca Abeni <luca.abeni@santannapisa.it>,
	Joel Fernandes <joelaf@google.com>,
	Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [RFC PATCH] sched/cpufreq/schedutil: handling urgent frequency requests
Date: Wed, 9 May 2018 10:41:48 +0200	[thread overview]
Message-ID: <CAJZ5v0j8RgzYFAc8Fj5D_or8v0rG_h2DGQQKqyCLKbN2BE1jUg@mail.gmail.com> (raw)
In-Reply-To: <20180509082259.GB76874@joelaf.mtv.corp.google.com>

On Wed, May 9, 2018 at 10:22 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> On Wed, May 09, 2018 at 10:05:09AM +0200, Rafael J. Wysocki wrote:
>> On Wed, May 9, 2018 at 9:01 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
>> > On Wed, May 09, 2018 at 12:24:49PM +0530, Viresh Kumar wrote:
>> >> On 09-05-18, 08:45, Juri Lelli wrote:
>> >> > On 08/05/18 21:54, Joel Fernandes wrote:
>> >> > Isn't this potentially introducing unneeded irq pressure (and doing the
>> >> > whole wakeup the kthread thing), while the already active kthread could
>> >> > simply handle multiple back-to-back requests before going to sleep?
>> >>
>> >> And then we may need more instances of the work item and need to store
>> >> a different value of next_freq with each work item, as we can't use
>> >> the common one anymore as there would be races around accessing it ?
>> >
>> > Exactly. I think it also doesn't make sense to over write an already
>> > committed request either so better to store them separate (?). After the
>> > "commit", that previous request is done..
>>
>> Why is it?
>>
>> In the non-fast-switch case the "commit" only means queuing up an
>> irq_work.  Which BTW is one of the reasons for having work_in_progress
>> even if your kthread can handle multiple work items in one go.
>
> Ok I agree. I just thought there was something funky with the meaning of
> commit from a cpufreq perspective.
>
> In the last diff I just sent out, I actually keep work_in_progress and
> consider its meaning to be what you're saying (has the kthread been kicked)
> and lets such "overwriting" of the next frequency to be made possible. Also
> with that we would be servicing just the latest request even if there were
> multiple ones made.

My understanding of this is that when the kthread actually starts
changing the frequency, it can't really roll back (at least not in
general), but there may be multiple following requests while the
frequency is being changed.  In that case the most recent of the new
requests is the only one that matters.  So IMO the kthread should make
a local copy of the most recent request to start with, process it and
let the irq_work update the most recent request data as new requests
come in.  When done with the previous frequency change, the kthread
can make a local copy of the most recent request again and so on.

This should work, because there is only one irq_work updating the most
recent request data.