linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Quentin Perret <quentin.perret@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,
	dietmar.eggemann@arm.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 v10 15/15] OPTIONAL: cpufreq: dt: Register an Energy Model
Date: Wed, 9 Jan 2019 10:14:51 -0800	[thread overview]
Message-ID: <20190109181451.GT261387@google.com> (raw)
In-Reply-To: <20190109105757.2rowxn3anzyycuod@queper01-lin>

On Wed, Jan 09, 2019 at 10:57:57AM +0000, Quentin Perret wrote:
> Hi Matthias ,
> 
> On Tuesday 08 Jan 2019 at 12:38:13 (-0800), Matthias Kaehlcke wrote:
> > >  static int cpufreq_init(struct cpufreq_policy *policy)
> > >  {
> > > +	struct em_data_callback em_cb = EM_DATA_CB(of_est_power);
> > >  	struct cpufreq_frequency_table *freq_table;
> > >  	struct opp_table *opp_table = NULL;
> > >  	struct private_data *priv;
> > > @@ -160,7 +203,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> > >  	unsigned int transition_latency;
> > >  	bool fallback = false;
> > >  	const char *name;
> > > -	int ret;
> > > +	int ret, nr_opp;
> > >  
> > >  	cpu_dev = get_cpu_device(policy->cpu);
> > >  	if (!cpu_dev) {
> > > @@ -237,6 +280,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> > >  		ret = -EPROBE_DEFER;
> > >  		goto out_free_opp;
> > >  	}
> > > +	nr_opp = ret;
> > >  
> > >  	if (fallback) {
> > >  		cpumask_setall(policy->cpus);
> > > @@ -280,6 +324,8 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> > >  	policy->cpuinfo.transition_latency = transition_latency;
> > >  	policy->dvfs_possible_from_any_cpu = true;
> > >  
> > > +	em_register_perf_domain(policy->cpus, nr_opp, &em_cb);
> > 
> > Shouldn't there also be a function to unregister a perf domain to be
> > called from cpufreq_exit()?
> > 
> > ->exit() is called on suspend when the CPUs are taken offline, and
> > ->init() on resume, hence em_register_perf_domain() is called multiple
> > times, but without doing any cleanup.
> 
> Right, but this is safe to do as em_register_perf_domain() checks for
> the existence of the domain straight away. So the second time you call
> this em_register_perf_domain() should just bail out. Are you seeing an
> issue with this ?
> 
> As of now, the EM framework is really simple -- it justs allocates the
> pd tables once during boot, and they stay in memory forever. While
> arguably less than optimal, that makes things a whole lot easier to
> manage on the client side (i.e. the scheduler) w/o needing RCU
> protection or so.

I think registering the perf domain only once is fine, since the info
isn't supposed to change and will likely be used again after
_exit(). However since we have em_cpu_get() I'd suggest to use it and
only call em_register_perf_domain() if no perf domain is registered
yet for the CPU. This makes it more evident that the registration is
only done once and simplifies error handling (currently not done at
all), since it's not necessary to check for the special case -EEXIST.

Cheers

Matthias

  reply	other threads:[~2019-01-09 18:14 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-03  9:56 [PATCH v10 00/15] Energy Aware Scheduling Quentin Perret
2018-12-03  9:56 ` [PATCH v10 01/15] sched: Relocate arch_scale_cpu_capacity Quentin Perret
2018-12-11 15:32   ` [tip:sched/core] sched/topology: Relocate arch_scale_cpu_capacity() to the internal header tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 02/15] sched/cpufreq: Prepare schedutil for Energy Aware Scheduling Quentin Perret
2018-12-11 12:01   ` Rafael J. Wysocki
2018-12-11 12:17     ` Quentin Perret
2018-12-11 12:22       ` Rafael J. Wysocki
2018-12-11 12:24         ` Quentin Perret
2018-12-11 15:33   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 03/15] PM: Introduce an Energy Model management framework Quentin Perret
2018-12-11 15:33   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 04/15] PM / EM: Expose the Energy Model in sysfs Quentin Perret
2018-12-11 14:18   ` Ingo Molnar
2018-12-11 15:04     ` Quentin Perret
2018-12-03  9:56 ` [PATCH v10 05/15] sched/topology: Reference the Energy Model of CPUs when available Quentin Perret
2018-12-11 15:34   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 06/15] sched/topology: Lowest CPU asymmetry sched_domain level pointer Quentin Perret
2018-12-11 15:34   ` [tip:sched/core] sched/topology: Add lowest " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 07/15] sched/topology: Disable EAS on inappropriate platforms Quentin Perret
2018-12-11 15:35   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 08/15] sched/topology: Make Energy Aware Scheduling depend on schedutil Quentin Perret
2018-12-11 15:36   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 09/15] sched: Introduce sched_energy_present static key Quentin Perret
2018-12-11 15:36   ` [tip:sched/core] sched/toplogy: Introduce the 'sched_energy_present' " tip-bot for Quentin Perret
2018-12-13 13:56     ` Quentin Perret
2018-12-03  9:56 ` [PATCH v10 10/15] sched: Introduce a sysctl for Energy Aware Scheduling Quentin Perret
2018-12-11 14:15   ` Ingo Molnar
2018-12-11 14:49     ` Quentin Perret
2018-12-13 14:03       ` Peter Zijlstra
2019-01-21 11:35   ` [tip:sched/core] sched/topology: " tip-bot for Quentin Perret
2019-01-21 13:51   ` tip-bot for Quentin Perret
2019-01-27 11:34   ` tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 11/15] sched/fair: Clean-up update_sg_lb_stats parameters Quentin Perret
2018-12-11 15:37   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 12/15] sched: Add over-utilization/tipping point indicator Quentin Perret
2018-12-11 15:37   ` [tip:sched/core] sched/fair: " tip-bot for Morten Rasmussen
2018-12-03  9:56 ` [PATCH v10 13/15] sched/fair: Introduce an energy estimation helper function Quentin Perret
2018-12-11 15:38   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 14/15] sched/fair: Select an energy-efficient CPU on task wake-up Quentin Perret
2018-12-11 15:39   ` [tip:sched/core] " tip-bot for Quentin Perret
2018-12-03  9:56 ` [PATCH v10 15/15] OPTIONAL: cpufreq: dt: Register an Energy Model Quentin Perret
2019-01-08 20:38   ` Matthias Kaehlcke
2019-01-09 10:57     ` Quentin Perret
2019-01-09 18:14       ` Matthias Kaehlcke [this message]
2019-01-10  9:08         ` Quentin Perret

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=20190109181451.GT261387@google.com \
    --to=mka@chromium.org \
    --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=quentin.perret@arm.com \
    --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 \
    --subject='Re: [PATCH v10 15/15] OPTIONAL: cpufreq: dt: Register an Energy Model' \
    /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).