| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 3 Jun 2011 15:15:34 -0700
From: Sameer Nanda <snanda@...gle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: David Daney <ddaney@...iumnetworks.com>,
ext-phil.2.carmody@...ia.com, Tim.Deegan@...rix.com,
jbeulich@...ell.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] init: skip calibration delay if previously done
On Fri, Jun 3, 2011 at 2:45 PM, Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Fri, 03 Jun 2011 14:08:01 -0700
> David Daney <ddaney@...iumnetworks.com> wrote:
>
> > On 06/03/2011 02:00 PM, Andrew Morton wrote:
> > > On Tue, 24 May 2011 16:19:06 -0700
> > > Sameer Nanda<snanda@...omium.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@...omium.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@...ux-foundation.org>
>
> Make these messages more gramatically pleasing, more consistent and remove
> strange ellipses.
>
> Cc: Andrew Worsley <amworsley@...il.com>
> Cc: Phil Carmody <ext-phil.2.carmody@...ia.com>
> Cc: Sameer Nanda <snanda@...omium.org>
> Cc: David Daney <ddaney@...iumnetworks.com>
> Signed-off-by: Andrew Morton <akpm@...ux-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists