mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Doug Smythies <dsmythies@telus.net>,
	"'Srinivas Pandruvada'" <srinivas.pandruvada@linux.intel.com>,
	"'Viresh Kumar'" <viresh.kumar@linaro.org>,
	"'Linux Kernel Mailing List'" <linux-kernel@vger.kernel.org>,
	"'Steve Muckle'" <steve.muckle@linaro.org>,
	"'Juri Lelli'" <juri.lelli@arm.com>,
	"'Ingo Molnar'" <mingo@kernel.org>,
	"'Linux PM list'" <linux-pm@vger.kernel.org>
Subject: Re: [RFC][PATCH 7/7] cpufreq: intel_pstate: Change P-state selection algorithm for Core
Date: Sat, 20 Aug 2016 03:06:06 +0200	[thread overview]
Message-ID: <15494617.fGGDIpuQfN@vostro.rjw.lan> (raw)
In-Reply-To: <20160819144729.GL10121@twins.programming.kicks-ass.net>

On Friday, August 19, 2016 04:47:29 PM Peter Zijlstra wrote:
> On Sat, Aug 13, 2016 at 08:59:01AM -0700, Doug Smythies wrote:
> > My previous replies (and see below) have suggested that some filtering
> > is needed on the target pstate, otherwise, and dependant on the type of
> > workload, it tends to oscillate.
> > 
> > I added the IIR (Infinite Impulse Response) filter that I have suggested in the past:
> 
> One question though; why is this filter intel_pstate specific? Should we
> not do this in generic code?

The intel_pstate algorithm is based on the feedback registers and I'm not sure
if the same effect appears if utilization is computed in a different way.

> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index c43ef55..262ec5f 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -1313,7 +1318,74 @@ static inline int32_t get_target_pstate_default(struct cpudata *cpu)
> >         cpu->iowait_boost >>= 1;
> > 
> >         pstate = cpu->pstate.turbo_pstate;
> 
> > +       unfiltered_target = (pstate + (pstate >> 2)) * busy_frac;
> 
> > +       duration_ns = cpu->sample.time - cpu->last_sample_time;
> > +
> > +       scaled_gain = div_u64(int_tofp(duration_ns) *
> > +               int_tofp(pid_params.p_gain_pct), int_tofp(pid_params.sample_rate_ns));
> 
> Drop int_to_fp() on one of the dividend terms and in the divisor. Same
> end result since they divide away against one another but reduces the
> risk of overflow.
> 
> Also, sample_rate_ns, really!? A rate is in [1/s], should that thing be
> called period_ns ?

That's an old name that hasn't been changed for quite a while.  That said
"period" or "interval" would be better.

Thanks,
Rafael

  reply	other threads:[~2016-08-20  1:00 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-31 23:31 [RFC][PATCH 0/7] cpufreq / sched: cpufreq_update_util() flags and iowait boosting Rafael J. Wysocki
2016-07-31 23:34 ` [RFC][PATCH 1/7] cpufreq / sched: Make schedutil access utilization data directly Rafael J. Wysocki
2016-08-01 19:28   ` Steve Muckle
2016-08-01 23:46     ` Rafael J. Wysocki
2016-08-02 10:38       ` Juri Lelli
2016-08-02 14:28         ` Steve Muckle
2016-08-02 14:43           ` Juri Lelli
2016-08-08 10:38       ` Peter Zijlstra
2016-07-31 23:35 ` [RFC][PATCH 2/7] cpufreq / sched: Drop cpufreq_trigger_update() Rafael J. Wysocki
2016-07-31 23:36 ` [RFC][PATCH 3/7] cpufreq / sched: Check cpu_of(rq) in cpufreq_update_util() Rafael J. Wysocki
2016-08-01  7:29   ` Dominik Brodowski
2016-08-01 14:57     ` Rafael J. Wysocki
2016-08-01 19:48     ` Steve Muckle
2016-08-01 23:43       ` Rafael J. Wysocki
2016-07-31 23:36 ` [RFC][PATCH 4/7] cpufreq / sched: Add flags argument to cpufreq_update_util() Rafael J. Wysocki
2016-08-01  7:33   ` Dominik Brodowski
2016-08-01 14:57     ` Rafael J. Wysocki
2016-08-01 19:59       ` Steve Muckle
2016-08-01 23:44         ` Rafael J. Wysocki
2016-08-02  1:36           ` Steve Muckle
2016-07-31 23:37 ` [RFC][PATCH 5/7] cpufreq / sched: UUF_IO flag to indicate iowait condition Rafael J. Wysocki
2016-08-02  1:22   ` Steve Muckle
2016-08-02  1:37     ` Rafael J. Wysocki
2016-08-02 22:02       ` Steve Muckle
2016-08-02 22:38         ` Rafael J. Wysocki
2016-08-04  2:24           ` Steve Muckle
2016-08-04 21:19             ` Rafael J. Wysocki
2016-08-04 22:09               ` Steve Muckle
2016-08-05 23:36                 ` Rafael J. Wysocki
2016-07-31 23:37 ` [RFC][PATCH 6/7] cpufreq: schedutil: Add iowait boosting Rafael J. Wysocki
2016-08-02  1:35   ` Steve Muckle
2016-08-02 23:03     ` Rafael J. Wysocki
2016-07-31 23:38 ` [RFC][PATCH 7/7] cpufreq: intel_pstate: Change P-state selection algorithm for Core Rafael J. Wysocki
2016-08-04  4:18   ` Doug Smythies
2016-08-04  6:53   ` Doug Smythies
2016-08-06  0:02     ` Rafael J. Wysocki
2016-08-09 17:16       ` Doug Smythies
2016-08-13 15:59       ` Doug Smythies
2016-08-19 14:47         ` Peter Zijlstra
2016-08-20  1:06           ` Rafael J. Wysocki [this message]
2016-08-20  6:40           ` Doug Smythies
2016-08-22 18:53         ` Srinivas Pandruvada
2016-08-22 22:53           ` Doug Smythies
2016-08-23  3:48   ` Wanpeng Li
2016-08-23  4:08     ` Srinivas Pandruvada
2016-08-23  4:50       ` Wanpeng Li
2016-08-23 17:30         ` Rafael J. Wysocki
2016-08-01 15:26 ` [RFC][PATCH 0/7] cpufreq / sched: cpufreq_update_util() flags and iowait boosting Doug Smythies
2016-08-01 16:30   ` Rafael J. Wysocki
2016-08-08 11:08     ` Peter Zijlstra
2016-08-08 13:01       ` 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=15494617.fGGDIpuQfN@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=dsmythies@telus.net \
    --cc=juri.lelli@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=srinivas.pandruvada@linux.intel.com \
    --cc=steve.muckle@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

all inboxes | Powered by JetHome®