From: "Rafael J. Wysocki" <rjw@rjwysocki.net> To: Peter Zijlstra <peterz@infradead.org>, Linux PM list <linux-pm@vger.kernel.org> Cc: kernel test robot <ying.huang@linux.intel.com>, Steve Muckle <steve.muckle@linaro.org>, lkp@01.org, linux-kernel@vger.kernel.org, Vincent Guittot <vincent.guittot@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, "Rafael J. Wysocki" <rafael@kernel.org>, Patrick Bellasi <patrick.bellasi@arm.com>, Morten Rasmussen <morten.rasmussen@arm.com>, Mike Galbraith <efault@gmx.de>, Michael Turquette <mturquette@baylibre.com>, Juri Lelli <Juri.Lelli@arm.com>, Dietmar Eggemann <dietmar.eggemann@arm.com>, Steve Muckle <smuckle@linaro.org>, Ingo Molnar <mingo@kernel.org>, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Subject: [PATCH] intel_pstate: Fix intel_pstate_get() Date: Wed, 04 May 2016 14:01:10 +0200 [thread overview] Message-ID: <54177738.QIcPrzf0Y7@vostro.rjw.lan> (raw) In-Reply-To: <20160503083231.GG3430@twins.programming.kicks-ass.net> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> After commit 8fa520af5081 "intel_pstate: Remove freq calculation from intel_pstate_calc_busy()" intel_pstate_get() calls get_avg_frequency() to compute the average frequency, which is problematic for two reasons. First, intel_pstate_get() may be invoked before the driver reads the CPU feedback registers for the first time and if that happens, get_avg_frequency() will attempt to divide by zero. Second, the get_avg_frequency() call in intel_pstate_get() is racy with respect to intel_pstate_sample() and it may end up returning completely meaningless values for this reason. Moreover, after commit 7349ec0470b6 "intel_pstate: Move intel_pstate_calc_busy() into get_target_pstate_use_performance()" sample.core_pct_busy is never computed on Atom, but it is used in intel_pstate_adjust_busy_pstate() in that case too. To address those problems notice that if sample.core_pct_busy was used in the average frequency computation carried out by get_avg_frequency(), both the divide by zero problem and the race with respect to intel_pstate_sample() would be avoided. Accordingly, move the invocation of intel_pstate_calc_busy() from get_target_pstate_use_performance() to intel_pstate_update_util(), which also will take care of the uninitialized sample.core_pct_busy on Atom, and modify get_avg_frequency() to use sample.core_pct_busy as per the above. Reported-by: kernel test robot <ying.huang@linux.intel.com> Link: http://marc.info/?l=linux-kernel&m=146226437623173&w=4 Fixes: 8fa520af5081 "intel_pstate: Remove freq calculation from intel_pstate_calc_busy()" Fixes: 7349ec0470b6 "intel_pstate: Move intel_pstate_calc_busy() into get_target_pstate_use_performance()" Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/cpufreq/intel_pstate.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -928,8 +928,9 @@ static inline bool intel_pstate_sample(s static inline int32_t get_avg_frequency(struct cpudata *cpu) { - return div64_u64(cpu->pstate.max_pstate_physical * cpu->sample.aperf * - cpu->pstate.scaling, cpu->sample.mperf); + return fp_toint(mul_fp(cpu->sample.core_pct_busy, + int_tofp(cpu->pstate.max_pstate_physical * + cpu->pstate.scaling / 100))); } static inline int32_t get_target_pstate_use_cpu_load(struct cpudata *cpu) @@ -972,8 +973,6 @@ static inline int32_t get_target_pstate_ int32_t core_busy, max_pstate, current_pstate, sample_ratio; u64 duration_ns; - intel_pstate_calc_busy(cpu); - /* * core_busy is the ratio of actual performance to max * max_pstate is the max non turbo pstate available @@ -1056,8 +1055,11 @@ static void intel_pstate_update_util(str if ((s64)delta_ns >= pid_params.sample_rate_ns) { bool sample_taken = intel_pstate_sample(cpu, time); - if (sample_taken && !hwp_active) - intel_pstate_adjust_busy_pstate(cpu); + if (sample_taken) { + intel_pstate_calc_busy(cpu); + if (!hwp_active) + intel_pstate_adjust_busy_pstate(cpu); + } } }
WARNING: multiple messages have this Message-ID (diff)
From: Rafael J. Wysocki <rjw@rjwysocki.net> To: lkp@lists.01.org Subject: [PATCH] intel_pstate: Fix intel_pstate_get() Date: Wed, 04 May 2016 14:01:10 +0200 [thread overview] Message-ID: <54177738.QIcPrzf0Y7@vostro.rjw.lan> (raw) In-Reply-To: <20160503083231.GG3430@twins.programming.kicks-ass.net> [-- Attachment #1: Type: text/plain, Size: 3333 bytes --] From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> After commit 8fa520af5081 "intel_pstate: Remove freq calculation from intel_pstate_calc_busy()" intel_pstate_get() calls get_avg_frequency() to compute the average frequency, which is problematic for two reasons. First, intel_pstate_get() may be invoked before the driver reads the CPU feedback registers for the first time and if that happens, get_avg_frequency() will attempt to divide by zero. Second, the get_avg_frequency() call in intel_pstate_get() is racy with respect to intel_pstate_sample() and it may end up returning completely meaningless values for this reason. Moreover, after commit 7349ec0470b6 "intel_pstate: Move intel_pstate_calc_busy() into get_target_pstate_use_performance()" sample.core_pct_busy is never computed on Atom, but it is used in intel_pstate_adjust_busy_pstate() in that case too. To address those problems notice that if sample.core_pct_busy was used in the average frequency computation carried out by get_avg_frequency(), both the divide by zero problem and the race with respect to intel_pstate_sample() would be avoided. Accordingly, move the invocation of intel_pstate_calc_busy() from get_target_pstate_use_performance() to intel_pstate_update_util(), which also will take care of the uninitialized sample.core_pct_busy on Atom, and modify get_avg_frequency() to use sample.core_pct_busy as per the above. Reported-by: kernel test robot <ying.huang@linux.intel.com> Link: http://marc.info/?l=linux-kernel&m=146226437623173&w=4 Fixes: 8fa520af5081 "intel_pstate: Remove freq calculation from intel_pstate_calc_busy()" Fixes: 7349ec0470b6 "intel_pstate: Move intel_pstate_calc_busy() into get_target_pstate_use_performance()" Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/cpufreq/intel_pstate.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -928,8 +928,9 @@ static inline bool intel_pstate_sample(s static inline int32_t get_avg_frequency(struct cpudata *cpu) { - return div64_u64(cpu->pstate.max_pstate_physical * cpu->sample.aperf * - cpu->pstate.scaling, cpu->sample.mperf); + return fp_toint(mul_fp(cpu->sample.core_pct_busy, + int_tofp(cpu->pstate.max_pstate_physical * + cpu->pstate.scaling / 100))); } static inline int32_t get_target_pstate_use_cpu_load(struct cpudata *cpu) @@ -972,8 +973,6 @@ static inline int32_t get_target_pstate_ int32_t core_busy, max_pstate, current_pstate, sample_ratio; u64 duration_ns; - intel_pstate_calc_busy(cpu); - /* * core_busy is the ratio of actual performance to max * max_pstate is the max non turbo pstate available @@ -1056,8 +1055,11 @@ static void intel_pstate_update_util(str if ((s64)delta_ns >= pid_params.sample_rate_ns) { bool sample_taken = intel_pstate_sample(cpu, time); - if (sample_taken && !hwp_active) - intel_pstate_adjust_busy_pstate(cpu); + if (sample_taken) { + intel_pstate_calc_busy(cpu); + if (!hwp_active) + intel_pstate_adjust_busy_pstate(cpu); + } } }
next prev parent reply other threads:[~2016-05-04 11:58 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-05-03 1:10 [sched/fair] 41e0d37f7a: divide error: 0000 [#1] SMP kernel test robot 2016-05-03 8:32 ` [lkp] " Peter Zijlstra 2016-05-03 8:32 ` Peter Zijlstra 2016-05-03 9:19 ` [lkp] " Wanpeng Li 2016-05-03 9:25 ` Wanpeng Li 2016-05-03 13:33 ` Rafael J. Wysocki 2016-05-03 13:33 ` Rafael J. Wysocki 2016-05-04 0:53 ` [lkp] " Wanpeng Li 2016-05-04 11:41 ` Rafael J. Wysocki 2016-05-04 11:41 ` Rafael J. Wysocki 2016-05-04 11:46 ` [lkp] " Wanpeng Li 2016-05-03 12:15 ` Rafael J. Wysocki 2016-05-03 12:15 ` Rafael J. Wysocki 2016-05-03 12:54 ` [lkp] " Rafael J. Wysocki 2016-05-03 12:54 ` Rafael J. Wysocki 2016-05-03 12:58 ` [lkp] " Rafael J. Wysocki 2016-05-03 12:58 ` Rafael J. Wysocki 2016-05-03 13:22 ` [lkp] " Rafael J. Wysocki 2016-05-03 13:22 ` Rafael J. Wysocki 2016-05-03 13:53 ` [lkp] " Rafael J. Wysocki 2016-05-03 13:53 ` Rafael J. Wysocki 2016-05-03 15:10 ` [lkp] " Rafael J. Wysocki 2016-05-03 15:10 ` Rafael J. Wysocki 2016-05-05 5:05 ` [lkp] " Wanpeng Li 2016-05-05 13:46 ` Rafael J. Wysocki 2016-05-05 13:46 ` Rafael J. Wysocki 2016-05-06 7:06 ` [lkp] " Wanpeng Li 2016-05-06 12:29 ` Rafael J. Wysocki 2016-05-06 12:29 ` Rafael J. Wysocki 2016-05-04 0:58 ` [lkp] " Wanpeng Li 2016-05-04 11:44 ` Rafael J. Wysocki 2016-05-04 11:44 ` Rafael J. Wysocki 2016-05-04 11:51 ` [lkp] " Wanpeng Li 2016-05-04 11:56 ` Rafael J. Wysocki 2016-05-04 11:56 ` Rafael J. Wysocki 2016-05-04 12:04 ` [lkp] " Wanpeng Li 2016-05-04 12:01 ` Rafael J. Wysocki [this message] 2016-05-04 12:01 ` [PATCH] intel_pstate: Fix intel_pstate_get() 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=54177738.QIcPrzf0Y7@vostro.rjw.lan \ --to=rjw@rjwysocki.net \ --cc=Juri.Lelli@arm.com \ --cc=dietmar.eggemann@arm.com \ --cc=efault@gmx.de \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=lkp@01.org \ --cc=mingo@kernel.org \ --cc=morten.rasmussen@arm.com \ --cc=mturquette@baylibre.com \ --cc=patrick.bellasi@arm.com \ --cc=peterz@infradead.org \ --cc=rafael@kernel.org \ --cc=smuckle@linaro.org \ --cc=srinivas.pandruvada@linux.intel.com \ --cc=steve.muckle@linaro.org \ --cc=tglx@linutronix.de \ --cc=vincent.guittot@linaro.org \ --cc=ying.huang@linux.intel.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.