For each CPU, do the calibration delay only once. For subsequent calls,
use the cached per-CPU value of loops_per_jiffy.
This saves about 200ms of resume time on dual core Intel Atom N5xx based
systems. This helps bring down the kernel resume time on such systems from
about 500ms to about 300ms.
Signed-off-by: Sameer Nanda <[email protected]>
---
init/calibrate.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/init/calibrate.c b/init/calibrate.c
index 76ac919..47d3408 100644
--- a/init/calibrate.c
+++ b/init/calibrate.c
@@ -183,11 +183,18 @@ recalibrate:
return lpj;
}
+DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
+
void __cpuinit calibrate_delay(void)
{
static bool printed;
+ int this_cpu = smp_processor_id();
- if (preset_lpj) {
+ if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
+ loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
+ pr_info("Calibrating delay loop (skipped) "
+ "already calibrated this CPU previously.. ");
+ } else if (preset_lpj) {
loops_per_jiffy = preset_lpj;
if (!printed)
pr_info("Calibrating delay loop (skipped) "
@@ -205,6 +212,7 @@ void __cpuinit calibrate_delay(void)
pr_info("Calibrating delay loop... ");
loops_per_jiffy = calibrate_delay_converge();
}
+ per_cpu(cpu_loops_per_jiffy, this_cpu) = loops_per_jiffy;
if (!printed)
pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
loops_per_jiffy/(500000/HZ),
--
1.7.3.1
On Tue, 24 May 2011 16:19:06 -0700
Sameer Nanda <[email protected]> wrote:
> For each CPU, do the calibration delay only once. For subsequent calls,
> use the cached per-CPU value of loops_per_jiffy.
>
> This saves about 200ms of resume time on dual core Intel Atom N5xx based
> systems. This helps bring down the kernel resume time on such systems from
> about 500ms to about 300ms.
>
> Signed-off-by: Sameer Nanda <[email protected]>
> ---
> init/calibrate.c | 10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/init/calibrate.c b/init/calibrate.c
> index 76ac919..47d3408 100644
> --- a/init/calibrate.c
> +++ b/init/calibrate.c
> @@ -183,11 +183,18 @@ recalibrate:
> return lpj;
> }
>
> +DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
> +
> void __cpuinit calibrate_delay(void)
> {
> static bool printed;
> + int this_cpu = smp_processor_id();
>
> - if (preset_lpj) {
> + if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
> + loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
> + pr_info("Calibrating delay loop (skipped) "
> + "already calibrated this CPU previously.. ");
> + } else if (preset_lpj) {
> loops_per_jiffy = preset_lpj;
> if (!printed)
> pr_info("Calibrating delay loop (skipped) "
> @@ -205,6 +212,7 @@ void __cpuinit calibrate_delay(void)
> pr_info("Calibrating delay loop... ");
> loops_per_jiffy = calibrate_delay_converge();
> }
> + per_cpu(cpu_loops_per_jiffy, this_cpu) = loops_per_jiffy;
> if (!printed)
> pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
> loops_per_jiffy/(500000/HZ),
Seems reasonable.
On resume, the kernel will print "already calibrated this CPU
previously.." in all situations, such as when preset_lpj was set. I
don't see a problem with that.
Let's be nice to the namespace:
--- a/init/calibrate.c~init-skip-calibration-delay-if-previously-done-fix
+++ a/init/calibrate.c
@@ -246,7 +246,7 @@ recalibrate:
return lpj;
}
-DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
+static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
void __cpuinit calibrate_delay(void)
{
_
On 06/03/2011 02:00 PM, Andrew Morton wrote:
> On Tue, 24 May 2011 16:19:06 -0700
> Sameer Nanda<[email protected]> wrote:
>
>> For each CPU, do the calibration delay only once. For subsequent calls,
>> use the cached per-CPU value of loops_per_jiffy.
>>
>> This saves about 200ms of resume time on dual core Intel Atom N5xx based
>> systems. This helps bring down the kernel resume time on such systems from
>> about 500ms to about 300ms.
>>
>> Signed-off-by: Sameer Nanda<[email protected]>
>> ---
>> init/calibrate.c | 10 +++++++++-
>> 1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> diff --git a/init/calibrate.c b/init/calibrate.c
>> index 76ac919..47d3408 100644
>> --- a/init/calibrate.c
>> +++ b/init/calibrate.c
>> @@ -183,11 +183,18 @@ recalibrate:
>> return lpj;
>> }
>>
>> +DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
>> +
>> void __cpuinit calibrate_delay(void)
>> {
>> static bool printed;
>> + int this_cpu = smp_processor_id();
>>
>> - if (preset_lpj) {
>> + if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
>> + loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
>> + pr_info("Calibrating delay loop (skipped) "
>> + "already calibrated this CPU previously.. ");
That wording seems a little redundant, and there are two '.' at the end.
How about:
s/"already calibrated this CPU previously.. "/", this CPU previously
calibrated."/
David Daney
On Fri, 03 Jun 2011 14:08:01 -0700
David Daney <[email protected]> wrote:
> On 06/03/2011 02:00 PM, Andrew Morton wrote:
> > On Tue, 24 May 2011 16:19:06 -0700
> > Sameer Nanda<[email protected]> wrote:
> >
> >> For each CPU, do the calibration delay only once. For subsequent calls,
> >> use the cached per-CPU value of loops_per_jiffy.
> >>
> >> This saves about 200ms of resume time on dual core Intel Atom N5xx based
> >> systems. This helps bring down the kernel resume time on such systems from
> >> about 500ms to about 300ms.
> >>
> >> Signed-off-by: Sameer Nanda<[email protected]>
> >> ---
> >> init/calibrate.c | 10 +++++++++-
> >> 1 files changed, 9 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/init/calibrate.c b/init/calibrate.c
> >> index 76ac919..47d3408 100644
> >> --- a/init/calibrate.c
> >> +++ b/init/calibrate.c
> >> @@ -183,11 +183,18 @@ recalibrate:
> >> return lpj;
> >> }
> >>
> >> +DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
> >> +
> >> void __cpuinit calibrate_delay(void)
> >> {
> >> static bool printed;
> >> + int this_cpu = smp_processor_id();
> >>
> >> - if (preset_lpj) {
> >> + if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
> >> + loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
> >> + pr_info("Calibrating delay loop (skipped) "
> >> + "already calibrated this CPU previously.. ");
>
> That wording seems a little redundant, and there are two '.' at the end.
>
> How about:
> s/"already calibrated this CPU previously.. "/", this CPU previously
> calibrated."/
>
Pedant ;)
--- a/init/calibrate.c~init-skip-calibration-delay-if-previously-done-fix-fix
+++ a/init/calibrate.c
@@ -256,7 +256,7 @@ void __cpuinit calibrate_delay(void)
if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
pr_info("Calibrating delay loop (skipped) "
- "already calibrated this CPU previously.. ");
+ "already calibrated this CPU");
} else if (preset_lpj) {
loops_per_jiffy = preset_lpj;
if (!printed)
_
But the whole thing is a bit weird. Does this look better?
From: Andrew Morton <[email protected]>
Make these messages more gramatically pleasing, more consistent and remove
strange ellipses.
Cc: Andrew Worsley <[email protected]>
Cc: Phil Carmody <[email protected]>
Cc: Sameer Nanda <[email protected]>
Cc: David Daney <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
init/calibrate.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff -puN init/calibrate.c~init-calibratec-calibrate_delay-tidy-up-the-pr_info-messages init/calibrate.c
--- a/init/calibrate.c~init-calibratec-calibrate_delay-tidy-up-the-pr_info-messages
+++ a/init/calibrate.c
@@ -255,24 +255,24 @@ void __cpuinit calibrate_delay(void)
if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
- pr_info("Calibrating delay loop (skipped) "
- "already calibrated this CPU");
+ pr_info("Calibrating delay loop. Skipped: already calibrated "
+ "this CPU");
} else if (preset_lpj) {
loops_per_jiffy = preset_lpj;
if (!printed)
- pr_info("Calibrating delay loop (skipped) "
- "preset value.. ");
+ pr_info("Calibrating delay loop. Skipped: "
+ "preset value");
} else if ((!printed) && lpj_fine) {
loops_per_jiffy = lpj_fine;
- pr_info("Calibrating delay loop (skipped), "
- "value calculated using timer frequency.. ");
+ pr_info("Calibrating delay loop. Skipped: value calculated "
+ "using timer frequency");
} else if ((loops_per_jiffy = calibrate_delay_direct()) != 0) {
if (!printed)
- pr_info("Calibrating delay using timer "
- "specific routine.. ");
+ pr_info("Calibrating delay loop using timer-specific "
+ "routine");
} else {
if (!printed)
- pr_info("Calibrating delay loop... ");
+ pr_info("Calibrating delay loop");
loops_per_jiffy = calibrate_delay_converge();
}
per_cpu(cpu_loops_per_jiffy, this_cpu) = loops_per_jiffy;
_
On Fri, 2011-06-03 at 14:45 -0700, Andrew Morton wrote:
> But the whole thing is a bit weird. Does this look better?
> Make these messages more gramatically pleasing, more consistent and remove
> strange ellipses.
or maybe something like this:
init/calibrate.c | 34 ++++++++++++++++++++--------------
1 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/init/calibrate.c b/init/calibrate.c
index cfd7000..827a45c 100644
--- a/init/calibrate.c
+++ b/init/calibrate.c
@@ -246,32 +246,38 @@ recalibrate:
return lpj;
}
+static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
+
void __cpuinit calibrate_delay(void)
{
+ int this_cpu = smp_processor_id();
static bool printed;
+ const char *msg;
- if (preset_lpj) {
+ if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
+ loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
+ msg = " CPU previously calibrated";
+ } else if (preset_lpj) {
loops_per_jiffy = preset_lpj;
- if (!printed)
- pr_info("Calibrating delay loop (skipped) "
- "preset value.. ");
+ msg = " preset value";
} else if ((!printed) && lpj_fine) {
loops_per_jiffy = lpj_fine;
- pr_info("Calibrating delay loop (skipped), "
- "value calculated using timer frequency.. ");
+ msg = " using timer frequency";
} else if ((loops_per_jiffy = calibrate_delay_direct()) != 0) {
- if (!printed)
- pr_info("Calibrating delay using timer "
- "specific routine.. ");
+ msg = " using timer specific routine";
} else {
- if (!printed)
- pr_info("Calibrating delay loop... ");
+ pr_info("Calibrating delay loop...\n");
loops_per_jiffy = calibrate_delay_converge();
+ per_cpu(cpu_loops_per_jiffy, this_cpu) = loops_per_jiffy;
+ msg = "";
}
- if (!printed)
- pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
+
+ if (!printed) {
+ pr_info("Delay loop calibration:%s %lu.%02lu BogoMIPS (lpj=%lu)\n",
+ msg,
loops_per_jiffy/(500000/HZ),
(loops_per_jiffy/(5000/HZ)) % 100, loops_per_jiffy);
- printed = true;
+ printed = true;
+ }
}
On Fri, Jun 3, 2011 at 2:45 PM, Andrew Morton <[email protected]> wrote:
>
> On Fri, 03 Jun 2011 14:08:01 -0700
> David Daney <[email protected]> wrote:
>
> > On 06/03/2011 02:00 PM, Andrew Morton wrote:
> > > On Tue, 24 May 2011 16:19:06 -0700
> > > Sameer Nanda<[email protected]> wrote:
> > >
> > >> For each CPU, do the calibration delay only once. For subsequent calls,
> > >> use the cached per-CPU value of loops_per_jiffy.
> > >>
> > >> This saves about 200ms of resume time on dual core Intel Atom N5xx based
> > >> systems. This helps bring down the kernel resume time on such systems from
> > >> about 500ms to about 300ms.
> > >>
> > >> Signed-off-by: Sameer Nanda<[email protected]>
> > >> ---
> > >> init/calibrate.c | 10 +++++++++-
> > >> 1 files changed, 9 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/init/calibrate.c b/init/calibrate.c
> > >> index 76ac919..47d3408 100644
> > >> --- a/init/calibrate.c
> > >> +++ b/init/calibrate.c
> > >> @@ -183,11 +183,18 @@ recalibrate:
> > >> return lpj;
> > >> }
> > >>
> > >> +DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
> > >> +
> > >> void __cpuinit calibrate_delay(void)
> > >> {
> > >> static bool printed;
> > >> + int this_cpu = smp_processor_id();
> > >>
> > >> - if (preset_lpj) {
> > >> + if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
> > >> + loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
> > >> + pr_info("Calibrating delay loop (skipped) "
> > >> + "already calibrated this CPU previously.. ");
> >
> > That wording seems a little redundant, and there are two '.' at the end.
> >
> > How about:
> > s/"already calibrated this CPU previously.. "/", this CPU previously
> > calibrated."/
> >
>
> Pedant ;)
>
> --- a/init/calibrate.c~init-skip-calibration-delay-if-previously-done-fix-fix
> +++ a/init/calibrate.c
> @@ -256,7 +256,7 @@ void __cpuinit calibrate_delay(void)
> if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
> loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
> pr_info("Calibrating delay loop (skipped) "
> - "already calibrated this CPU previously.. ");
> + "already calibrated this CPU");
> } else if (preset_lpj) {
> loops_per_jiffy = preset_lpj;
> if (!printed)
> _
>
> But the whole thing is a bit weird. Does this look better?
>
> From: Andrew Morton <[email protected]>
>
> Make these messages more gramatically pleasing, more consistent and remove
> strange ellipses.
>
> Cc: Andrew Worsley <[email protected]>
> Cc: Phil Carmody <[email protected]>
> Cc: Sameer Nanda <[email protected]>
> Cc: David Daney <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> init/calibrate.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff -puN init/calibrate.c~init-calibratec-calibrate_delay-tidy-up-the-pr_info-messages init/calibrate.c
> --- a/init/calibrate.c~init-calibratec-calibrate_delay-tidy-up-the-pr_info-messages
> +++ a/init/calibrate.c
> @@ -255,24 +255,24 @@ void __cpuinit calibrate_delay(void)
>
> if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
> loops_per_jiffy = per_cpu(cpu_loops_per_jiffy, this_cpu);
> - pr_info("Calibrating delay loop (skipped) "
> - "already calibrated this CPU");
> + pr_info("Calibrating delay loop. Skipped: already calibrated "
> + "this CPU");
> } else if (preset_lpj) {
> loops_per_jiffy = preset_lpj;
> if (!printed)
> - pr_info("Calibrating delay loop (skipped) "
> - "preset value.. ");
> + pr_info("Calibrating delay loop. Skipped: "
> + "preset value");
> } else if ((!printed) && lpj_fine) {
> loops_per_jiffy = lpj_fine;
> - pr_info("Calibrating delay loop (skipped), "
> - "value calculated using timer frequency.. ");
> + pr_info("Calibrating delay loop. Skipped: value calculated "
> + "using timer frequency");
> } else if ((loops_per_jiffy = calibrate_delay_direct()) != 0) {
> if (!printed)
> - pr_info("Calibrating delay using timer "
> - "specific routine.. ");
> + pr_info("Calibrating delay loop using timer-specific "
> + "routine");
> } else {
> if (!printed)
> - pr_info("Calibrating delay loop... ");
> + pr_info("Calibrating delay loop");
> loops_per_jiffy = calibrate_delay_converge();
> }
> per_cpu(cpu_loops_per_jiffy, this_cpu) = loops_per_jiffy;
> _
>
Thanks for picking this up.
Mind adding the following to this patch to prevent ARM build breakage :)
diff --git a/init/calibrate.c b/init/calibrate.c
index ec1e528..1b76597 100644
--- a/init/calibrate.c
+++ b/init/calibrate.c
@@ -9,6 +9,7 @@
#include <linux/init.h>
#include <linux/timex.h>
#include <linux/smp.h>
+#include <linux/percpu.h>
unsigned long lpj_fine;
unsigned long preset_lpj;
--
Sameer