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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <BANLkTi=FmJHjcvNLSj7fuEP6jGMnt0NK+O7-eHEPPoOaJ3Lk_w@mail.gmail.com>
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ