linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Quentin Perret <quentin.perret@arm.com>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: peterz@infradead.org, rjw@rjwysocki.net,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	gregkh@linuxfoundation.org, mingo@redhat.com,
	morten.rasmussen@arm.com, chris.redpath@arm.com,
	patrick.bellasi@arm.com, valentin.schneider@arm.com,
	vincent.guittot@linaro.org, thara.gopinath@linaro.org,
	viresh.kumar@linaro.org, tkjos@google.com,
	joel@joelfernandes.org, smuckle@google.com,
	adharmap@codeaurora.org, skannan@codeaurora.org,
	pkondeti@codeaurora.org, juri.lelli@redhat.com,
	edubezval@gmail.com, srinivas.pandruvada@linux.intel.com,
	currojerez@riseup.net, javi.merino@kernel.org
Subject: Re: [PATCH v6 07/14] sched/topology: Introduce sched_energy_present static key
Date: Thu, 6 Sep 2018 10:29:56 +0100	[thread overview]
Message-ID: <20180906092955.tq27mhzfkovo2ehn@queper01-lin> (raw)
In-Reply-To: <f206f192-1bbf-f868-c8c6-8215334639db@arm.com>

Hi Dietmar,

On Wednesday 05 Sep 2018 at 23:06:38 (-0700), Dietmar Eggemann wrote:
> On 08/20/2018 02:44 AM, Quentin Perret wrote:
> > In order to ensure a minimal performance impact on non-energy-aware
> > systems, introduce a static_key guarding the access to Energy-Aware
> > Scheduling (EAS) code.
> > 
> > The static key is set iff all the following conditions are met for at
> > least one root domain:
> >    1. all online CPUs of the root domain are covered by the Energy
> >       Model (EM);
> >    2. the complexity of the root domain's EM is low enough to keep
> >       scheduling overheads low;
> >    3. the root domain has an asymmetric CPU capacity topology (detected
> >       by looking for the SD_ASYM_CPUCAPACITY flag in the sched_domain
> >       hierarchy).
> 
> This is pretty much the list (+ is schedutil running) of conditions to set
> rd->pd != NULL in build_perf_domains().

Yes, exactly. I actually loop over the rds to check if one of them has a
pd != NULL in order to enable/disable the static key. So the check for
those conditions is always done at the rd level.

> So when testing 'static_branch_unlikely(&sched_energy_present) &&
> rcu_dereference(rd->pd)' don't you test two times the same thing?

Well, not exactly. You could have two rds in your system, and only one
of the two has an Energy Model. The static key is just a performance
optimization for !EAS systems really. But I must admit it sort of lost a
bit of its interest over the versions. I mean, it's not that clear now
that a static key is a better option than a sched_feat as you suggest
below.

> Also, if let's say somebody wants to run another EM user (e.g. a thermal
> governor, like IPA) but not EAS on a asymmetric CPU capacity system. This
> can't be achieved with the current static branch approach

I assume you're talking about IPA once migrated to using the EM
framework ? In this case, I think you're right, we won't have a single
tunable left to enable/disable EAS. On a big.LITTLE platform, if you
want IPA, EAS will always be enabled by side effect ...

That's a very good point actually. I think some people will not be happy
with that. There are big.LITTLE users (not very many of them, but still)
who don't care that much about energy, but do about performance. And
those guys might want to use IPA without EAS. So I guess we really need
a new knob.

> So what about using a (disabled by default ?) sched_feature + rd->pd != NULL
> instead?

Right, that's an option. I could remove the static key and
sched_energy_start() altogether and replace all the
"if (static_branch_unlikely(&sched_energy_present))" by
"if (sched_feat(ENERGY_AWARE))" for example. That should be equivalent
to what I did with the static key from a performance standpoint. But that
would mean users have to manually flip switches to get EAS up and
running ... I assume it's the price to pay for configurability.

Another option would be a KConfig option + static key. I could keep all
of the ifdefery inside an accessor function like the following:

	static inline bool sched_energy_aware(void)
	{
	#ifdef CONFIG_SCHED_ENERGY
		return static_branch_likely(&sched_energy_present);
	#else
		return false;
	#endif
	}

Now, I understand that scheduler-related KConfig options aren't welcome
in general, so I tend to prefer the sched_feat option.

Thoughts ?

Thanks,
Quentin

  reply	other threads:[~2018-09-06  9:30 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-20  9:44 [PATCH v6 00/14] Energy Aware Scheduling Quentin Perret
2018-08-20  9:44 ` [PATCH v6 01/14] sched: Relocate arch_scale_cpu_capacity Quentin Perret
2018-08-20  9:44 ` [PATCH v6 02/14] sched/cpufreq: Factor out utilization to frequency mapping Quentin Perret
2018-09-10  9:29   ` Rafael J. Wysocki
2018-08-20  9:44 ` [PATCH v6 03/14] PM: Introduce an Energy Model management framework Quentin Perret
2018-08-29 10:04   ` Patrick Bellasi
2018-08-29 13:28     ` Quentin Perret
2018-08-31  9:04       ` Patrick Bellasi
2018-09-11  9:34       ` Andrea Parri
2018-09-11 12:32         ` Quentin Perret
2018-09-11 13:31           ` Andrea Parri
2018-09-10  9:44   ` Rafael J. Wysocki
2018-09-10 10:38     ` Quentin Perret
2018-09-10 10:40       ` Rafael J. Wysocki
2018-08-20  9:44 ` [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs Quentin Perret
2018-09-06  6:56   ` Dietmar Eggemann
2018-09-06 14:09     ` Quentin Perret
2018-09-07  0:14       ` Dietmar Eggemann
2018-08-20  9:44 ` [PATCH v6 05/14] sched/topology: Reference the Energy Model of CPUs when available Quentin Perret
2018-08-29 16:22   ` Patrick Bellasi
2018-08-29 16:56     ` Quentin Perret
2018-08-30 10:00       ` Patrick Bellasi
2018-08-30 10:47         ` Quentin Perret
2018-08-30 12:50           ` Patrick Bellasi
2018-08-20  9:44 ` [PATCH v6 06/14] sched/topology: Lowest CPU asymmetry sched_domain level pointer Quentin Perret
2018-08-20  9:44 ` [PATCH v6 07/14] sched/topology: Introduce sched_energy_present static key Quentin Perret
2018-08-29 16:50   ` Patrick Bellasi
2018-08-29 17:20     ` Quentin Perret
2018-08-30  9:23       ` Patrick Bellasi
2018-08-30  9:57         ` Quentin Perret
2018-08-30 10:18           ` Patrick Bellasi
2018-09-06  6:06   ` Dietmar Eggemann
2018-09-06  9:29     ` Quentin Perret [this message]
2018-09-06 23:49       ` Dietmar Eggemann
2018-09-07  8:24         ` Quentin Perret
2018-08-20  9:44 ` [PATCH v6 08/14] sched/fair: Clean-up update_sg_lb_stats parameters Quentin Perret
2018-08-20  9:44 ` [PATCH v6 09/14] sched: Add over-utilization/tipping point indicator Quentin Perret
2018-08-20  9:44 ` [PATCH v6 10/14] sched/cpufreq: Refactor the utilization aggregation method Quentin Perret
2018-09-10  9:53   ` Rafael J. Wysocki
2018-09-10 10:07     ` Quentin Perret
2018-09-10 10:25       ` Rafael J. Wysocki
2018-08-20  9:44 ` [PATCH v6 11/14] sched/fair: Introduce an energy estimation helper function Quentin Perret
2018-08-20  9:44 ` [PATCH v6 12/14] sched/fair: Select an energy-efficient CPU on task wake-up Quentin Perret
2018-08-20  9:44 ` [PATCH v6 13/14] sched/topology: Make Energy Aware Scheduling depend on schedutil Quentin Perret
2018-09-04 10:59   ` Quentin Perret
2018-09-06  9:18     ` Rafael J. Wysocki
2018-09-06 14:38       ` Quentin Perret
2018-09-07  8:52         ` Rafael J. Wysocki
2018-09-07  8:56           ` Rafael J. Wysocki
2018-09-07  9:02             ` Quentin Perret
2018-09-07 15:29           ` Quentin Perret
2018-09-09 20:13             ` Rafael J. Wysocki
2018-09-10  8:24               ` Quentin Perret
2018-09-10  8:55                 ` Rafael J. Wysocki
2018-09-10  9:43                   ` Quentin Perret
2018-08-20  9:44 ` [PATCH v6 14/14] OPTIONAL: cpufreq: dt: Register an Energy Model Quentin Perret
2018-09-10  9:12 ` [PATCH v6 00/14] Energy Aware Scheduling 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=20180906092955.tq27mhzfkovo2ehn@queper01-lin \
    --to=quentin.perret@arm.com \
    --cc=adharmap@codeaurora.org \
    --cc=chris.redpath@arm.com \
    --cc=currojerez@riseup.net \
    --cc=dietmar.eggemann@arm.com \
    --cc=edubezval@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=javi.merino@kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=patrick.bellasi@arm.com \
    --cc=peterz@infradead.org \
    --cc=pkondeti@codeaurora.org \
    --cc=rjw@rjwysocki.net \
    --cc=skannan@codeaurora.org \
    --cc=smuckle@google.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    --cc=thara.gopinath@linaro.org \
    --cc=tkjos@google.com \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).