linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] init: skip calibration delay if previously done
@ 2011-05-24 23:19 Sameer Nanda
  2011-06-03 21:00 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Sameer Nanda @ 2011-05-24 23:19 UTC (permalink / raw)
  To: akpm, ext-phil.2.carmody, Tim.Deegan, jbeulich, snanda
  Cc: linux-kernel, Sameer Nanda

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 <snanda@chromium.org>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] init: skip calibration delay if previously done
  2011-05-24 23:19 [PATCH] init: skip calibration delay if previously done Sameer Nanda
@ 2011-06-03 21:00 ` Andrew Morton
  2011-06-03 21:08   ` David Daney
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2011-06-03 21:00 UTC (permalink / raw)
  To: Sameer Nanda
  Cc: ext-phil.2.carmody, Tim.Deegan, jbeulich, snanda, linux-kernel

On Tue, 24 May 2011 16:19:06 -0700
Sameer Nanda <snanda@chromium.org> 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 <snanda@chromium.org>
> ---
>  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)
 {
_


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] init: skip calibration delay if previously done
  2011-06-03 21:00 ` Andrew Morton
@ 2011-06-03 21:08   ` David Daney
  2011-06-03 21:45     ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: David Daney @ 2011-06-03 21:08 UTC (permalink / raw)
  To: Sameer Nanda
  Cc: Andrew Morton, ext-phil.2.carmody, Tim.Deegan, jbeulich, snanda,
	linux-kernel

On 06/03/2011 02:00 PM, Andrew Morton wrote:
> On Tue, 24 May 2011 16:19:06 -0700
> Sameer Nanda<snanda@chromium.org>  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<snanda@chromium.org>
>> ---
>>   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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] init: skip calibration delay if previously done
  2011-06-03 21:08   ` David Daney
@ 2011-06-03 21:45     ` Andrew Morton
  2011-06-03 22:00       ` Joe Perches
  2011-06-03 22:15       ` Sameer Nanda
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2011-06-03 21:45 UTC (permalink / raw)
  To: David Daney
  Cc: Sameer Nanda, ext-phil.2.carmody, Tim.Deegan, jbeulich, snanda,
	linux-kernel

On Fri, 03 Jun 2011 14:08:01 -0700
David Daney <ddaney@caviumnetworks.com> wrote:

> On 06/03/2011 02:00 PM, Andrew Morton wrote:
> > On Tue, 24 May 2011 16:19:06 -0700
> > Sameer Nanda<snanda@chromium.org>  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<snanda@chromium.org>
> >> ---
> >>   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 <akpm@linux-foundation.org>

Make these messages more gramatically pleasing, more consistent and remove
strange ellipses.

Cc: Andrew Worsley <amworsley@gmail.com>
Cc: Phil Carmody <ext-phil.2.carmody@nokia.com>
Cc: Sameer Nanda <snanda@chromium.org>
Cc: David Daney <ddaney@caviumnetworks.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 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;
_


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] init: skip calibration delay if previously done
  2011-06-03 21:45     ` Andrew Morton
@ 2011-06-03 22:00       ` Joe Perches
  2011-06-03 22:15       ` Sameer Nanda
  1 sibling, 0 replies; 6+ messages in thread
From: Joe Perches @ 2011-06-03 22:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Daney, Sameer Nanda, ext-phil.2.carmody, Tim.Deegan,
	jbeulich, snanda, linux-kernel

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;
+	}
 }



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] init: skip calibration delay if previously done
  2011-06-03 21:45     ` Andrew Morton
  2011-06-03 22:00       ` Joe Perches
@ 2011-06-03 22:15       ` Sameer Nanda
  1 sibling, 0 replies; 6+ messages in thread
From: Sameer Nanda @ 2011-06-03 22:15 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Daney, ext-phil.2.carmody, Tim.Deegan, jbeulich, linux-kernel

On Fri, Jun 3, 2011 at 2:45 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 03 Jun 2011 14:08:01 -0700
> David Daney <ddaney@caviumnetworks.com> wrote:
>
> > On 06/03/2011 02:00 PM, Andrew Morton wrote:
> > > On Tue, 24 May 2011 16:19:06 -0700
> > > Sameer Nanda<snanda@chromium.org>  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<snanda@chromium.org>
> > >> ---
> > >>   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 <akpm@linux-foundation.org>
>
> Make these messages more gramatically pleasing, more consistent and remove
> strange ellipses.
>
> Cc: Andrew Worsley <amworsley@gmail.com>
> Cc: Phil Carmody <ext-phil.2.carmody@nokia.com>
> Cc: Sameer Nanda <snanda@chromium.org>
> Cc: David Daney <ddaney@caviumnetworks.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-06-03 22:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24 23:19 [PATCH] init: skip calibration delay if previously done Sameer Nanda
2011-06-03 21:00 ` Andrew Morton
2011-06-03 21:08   ` David Daney
2011-06-03 21:45     ` Andrew Morton
2011-06-03 22:00       ` Joe Perches
2011-06-03 22:15       ` Sameer Nanda

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).