LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-pm@vger.kernel.org, Pavan Kondeti <pkondeti@codeaurora.org>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Joel Fernandes <joelaf@google.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Quentin Perret <quentin.perret@arm.com>
Subject: Re: [PATCH] Revert "cpufreq: schedutil: Don't restrict kthread to related_cpus unnecessarily"
Date: Tue, 8 May 2018 12:36:58 +0200	[thread overview]
Message-ID: <7922d081-0bfb-0e55-7caf-ec9fb5d7bab0@arm.com> (raw)
In-Reply-To: <20180508094526.ajyjrwytguhv4xpe@vireshk-i7>

On 05/08/2018 11:45 AM, Viresh Kumar wrote:
> On 08-05-18, 11:09, Dietmar Eggemann wrote:
>> This would make sure that the kthreads are bound to the correct set of cpus
>> for platforms with those cpufreq drivers (cpufreq-dt (h960), scmi-cpufreq,
>> scpi-cpufreq) but it will also change the logic (e.g.
>> sugov_should_update_freq() -> cpufreq_can_do_remote_dvfs()).
> 
> Yeah, I misunderstood your patch a bit. So you are not disabling
> remote updates but only limiting the CPUs where the kthread runs.

Yes, remote updates are possible even if the sugov kthread is bound to 
the cpus of the policy.

cross policy (cluster) remote callback example:
...
migration/1-14 [001] enqueue_task_fair: this_cpu=1 cpu_of(rq)=7
migration/1-14 [001] sugov_update_shared: this_cpu=1 sg_cpu->cpu=7
                      sg_cpu->sg_policy->policy->related_cpus=4-7
   sugov:4-1492 [004] sugov_work: this_cpu=4
                      sg_cpu->sg_policy->policy->related_cpus=4-7
...

cpu=1 updates cpu=7 remotely. This is independent from where the actual 
sugov kthread is running.

> That still looks to be a big little specific problem to me right now
> and I am not sure why should we specially handle these kthreads ?
> Isn't the same true for any other threads/tasks in the kernel which
> may end up running on big CPUs ? And this problem still occurs with
> the EAS patches applied ? As I thought we may end up keeping such
> small tasks on little cores then.

Binding it to the cpus of the policy makes sense since if the OPP should 
be changed for these cpus it should be done on one of these cpus, making 
sure we don't disturb the others. EAS and big.LITTLE will profit from 
this, but potentially every system with multiple frequency domains.

Binding them on to little cpus is an overhead to me which will gain us 
very little. Keeping the sugov threads to the cpus of the policy doesn't 
cost much and helps a lot.

>> I'm still struggling to understand when a driver/platform should set
>> dvfs_possible_from_any_cpu to true and what the actual benefit would be.
> 
> Ideally it should be set by default for all ARM platforms at least
> which have more than one cpufreq policy, as there is no hardware
> limitation for changing frequency from other CPUs. If you look at the
> commit logs of patches which added remote updates, you will see
> interesting cases where this can be very useful.

That's true but where is the benefit by doing so? (Multiple) per-cluster 
or per-cpu frequency domains, why should the sugov kthread run on a 
foreign cpu?

> commit 674e75411fc2 ("sched: cpufreq: Allow remote cpufreq callbacks")

IMHO, this describes the remote cpufreq callback functionality itself 
which works perfectly fine even if we leave the sugov ktrhead bound to 
the cpus of the policy.

  parent reply	other threads:[~2018-05-08 10:37 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-08  7:33 Dietmar Eggemann
2018-05-08  8:22 ` Viresh Kumar
2018-05-08  9:09   ` Dietmar Eggemann
2018-05-08  9:42     ` Quentin Perret
2018-05-13  5:19       ` Joel Fernandes
2018-05-17 19:10         ` Saravana Kannan
2018-05-17 19:13           ` Joel Fernandes
2018-05-08  9:45     ` Viresh Kumar
2018-05-08 10:02       ` Quentin Perret
2018-05-08 10:34         ` Viresh Kumar
2018-05-08 11:00           ` Quentin Perret
2018-05-08 11:14             ` Viresh Kumar
2018-05-08 11:24               ` Quentin Perret
2018-05-08 12:20                 ` Juri Lelli
2018-05-08 20:36           ` Rafael J. Wysocki
2018-05-09  4:55             ` Viresh Kumar
2018-05-08 10:36       ` Dietmar Eggemann [this message]
2018-05-08 10:53         ` Viresh Kumar
2018-05-08 12:17           ` Juri Lelli
2018-05-09  4:55 ` Viresh Kumar
2018-05-17 10:32   ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7922d081-0bfb-0e55-7caf-ec9fb5d7bab0@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=joelaf@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=patrick.bellasi@arm.com \
    --cc=peterz@infradead.org \
    --cc=pkondeti@codeaurora.org \
    --cc=quentin.perret@arm.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=viresh.kumar@linaro.org \
    --subject='Re: [PATCH] Revert "cpufreq: schedutil: Don'\''t restrict kthread to related_cpus unnecessarily"' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).