linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Russell King <linux@arm.linux.org.uk>, Will Deacon <will.deacon@arm.com>
Cc: John Stultz <john.stultz@linaro.org>,
	David Riley <davidriley@chromium.org>,
	olof@lixom.net, Sonny Rao <sonnyrao@chromium.org>,
	Richard Zhao <richard.zhao@linaro.org>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Shawn Guo <shawn.guo@linaro.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Doug Anderson <dianders@chromium.org>,
	nicolas.pitre@linaro.org, sboyd@codeaurora.org,
	marc.zyngier@arm.com, swarren@nvidia.com,
	paul.gortmaker@windriver.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems
Date: Wed,  7 May 2014 16:23:02 -0700	[thread overview]
Message-ID: <1399504982-31181-1-git-send-email-dianders@chromium.org> (raw)

Downscaling loops_per_jiffy on SMP ARM systems really doesn't work.
You could really only do this if:

* Each CPU is has independent frequency changes (changing one CPU
  doesn't affect another).
* We change the generic ARM udelay() code to actually look at percpu
  loops_per_jiffy.

I don't know of any ARM CPUs that are totally independent that don't
just use a timer-based delay anyway.  For those that don't have a
timer-based delay, we should be conservative and overestimate
loops_per_jiffy.

Note that on some systems you might sometimes see (in the extreme case
when we're all the way downclocked) a udelay(100) become a
udelay(1000) now.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Note that I don't have an board that has cpufreq enabled upstream so
I'm relying on the testing I did on our local kernel-3.8.  Hopefully
someone out there can test using David's nifty udelay tests.  In order
to see this you'd need to make sure that you _don't_ have arch timers
enabled.  See:
* https://patchwork.kernel.org/patch/4124721/
* https://patchwork.kernel.org/patch/4124731/

 arch/arm/kernel/smp.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 7c4fada..9d944f6 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -649,39 +649,50 @@ int setup_profiling_timer(unsigned int multiplier)
 
 #ifdef CONFIG_CPU_FREQ
 
-static DEFINE_PER_CPU(unsigned long, l_p_j_ref);
-static DEFINE_PER_CPU(unsigned long, l_p_j_ref_freq);
 static unsigned long global_l_p_j_ref;
 static unsigned long global_l_p_j_ref_freq;
+static unsigned long global_l_p_j_max_freq;
+
+/**
+ * cpufreq_callback - Adjust loops_per_jiffies when frequency changes
+ *
+ * When the CPU frequency changes we need to adjust loops_per_jiffies, which
+ * we assume scales linearly with frequency.
+ *
+ * This function is fairly castrated and only ever adjust loops_per_jiffies
+ * upward.  It also doesn't adjust the PER_CPU loops_per_jiffies.  Here's why:
+ * 1. The ARM udelay only ever looks at the global loops_per_jiffy not the
+ *    percpu one.  If your CPUs _are not_ changed in lockstep you could run
+ *    into problems by decreasing loops_per_jiffies since one of the other
+ *    processors might still be running slower.
+ * 2. The ARM udelay reads the loops_per_jiffy at the beginning of its loop and
+ *    no other times.  If your CPUs _are_ changed in lockstep you could run
+ *    into a race where one CPU has started its loop with old (slower)
+ *    loops_per_jiffy and then suddenly is running faster.
+ *
+ * Anyone who wants a good udelay() should be using a timer-based solution
+ * anyway.  If you don't have a timer solution, you just gotta be conservative.
+ */
 
 static int cpufreq_callback(struct notifier_block *nb,
 					unsigned long val, void *data)
 {
 	struct cpufreq_freqs *freq = data;
-	int cpu = freq->cpu;
 
 	if (freq->flags & CPUFREQ_CONST_LOOPS)
 		return NOTIFY_OK;
 
-	if (!per_cpu(l_p_j_ref, cpu)) {
-		per_cpu(l_p_j_ref, cpu) =
-			per_cpu(cpu_data, cpu).loops_per_jiffy;
-		per_cpu(l_p_j_ref_freq, cpu) = freq->old;
-		if (!global_l_p_j_ref) {
-			global_l_p_j_ref = loops_per_jiffy;
-			global_l_p_j_ref_freq = freq->old;
-		}
+	if (!global_l_p_j_ref) {
+		global_l_p_j_ref = loops_per_jiffy;
+		global_l_p_j_ref_freq = freq->old;
+		global_l_p_j_max_freq = freq->old;
 	}
 
-	if ((val == CPUFREQ_PRECHANGE  && freq->old < freq->new) ||
-	    (val == CPUFREQ_POSTCHANGE && freq->old > freq->new)) {
+	if (freq->new > global_l_p_j_max_freq) {
 		loops_per_jiffy = cpufreq_scale(global_l_p_j_ref,
 						global_l_p_j_ref_freq,
 						freq->new);
-		per_cpu(cpu_data, cpu).loops_per_jiffy =
-			cpufreq_scale(per_cpu(l_p_j_ref, cpu),
-					per_cpu(l_p_j_ref_freq, cpu),
-					freq->new);
+		global_l_p_j_max_freq = freq->new;
 	}
 	return NOTIFY_OK;
 }
-- 
1.9.1.423.g4596e3a


             reply	other threads:[~2014-05-07 23:23 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-07 23:23 Doug Anderson [this message]
2014-05-08 10:41 ` [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems Viresh Kumar
2014-05-08 15:25   ` Doug Anderson
2014-05-08 16:04     ` Nicolas Pitre
2014-05-08 16:41       ` Doug Anderson
2014-05-08 17:43         ` Nicolas Pitre
2014-05-08 18:06           ` Doug Anderson
2014-05-08 19:59             ` Nicolas Pitre
2014-05-08 20:55             ` Russell King - ARM Linux
2014-05-09  0:02               ` Doug Anderson
2014-05-09  0:23                 ` Russell King - ARM Linux
2014-05-09  4:41                   ` Doug Anderson
2014-05-08 19:22           ` Russell King - ARM Linux
2014-05-08 20:12             ` Nicolas Pitre
2014-05-08 20:39               ` John Stultz
2014-05-08 20:52               ` Russell King - ARM Linux
2014-05-09  1:37                 ` Nicolas Pitre
2014-05-09  4:43                   ` Doug Anderson
2014-05-09  9:18                   ` [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems# Russell King - ARM Linux
2014-05-09 18:00                     ` Nicolas Pitre
2014-05-09 18:22                       ` Russell King - ARM Linux
2014-05-09 21:05                         ` Nicolas Pitre
2014-05-12 23:51                           ` Doug Anderson
2014-05-13 21:50                             ` Doug Anderson
2014-05-13 22:15                               ` Stephen Warren
2014-05-13 23:15                                 ` Nicolas Pitre
2014-05-13 23:29                                   ` Nicolas Pitre
2014-05-13 23:36                                     ` Russell King - ARM Linux
2014-05-14 21:42                                     ` Doug Anderson
2014-05-15  6:12                               ` Viresh Kumar
2014-05-09  9:25     ` [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems Viresh Kumar

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=1399504982-31181-1-git-send-email-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=davidriley@chromium.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=marc.zyngier@arm.com \
    --cc=nicolas.pitre@linaro.org \
    --cc=olof@lixom.net \
    --cc=paul.gortmaker@windriver.com \
    --cc=richard.zhao@linaro.org \
    --cc=rjw@sisk.pl \
    --cc=santosh.shilimkar@ti.com \
    --cc=sboyd@codeaurora.org \
    --cc=shawn.guo@linaro.org \
    --cc=sonnyrao@chromium.org \
    --cc=swarren@nvidia.com \
    --cc=will.deacon@arm.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: 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).