[PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[Peter De Schrijver]

From: Diwakar Tundlam <dtundlam@nvidia.com>

The next_balance parameter of nohz_idle_balancer should be initialized
to jiffies since jiffies itself is initialized to 300 seconds shy of
overflow. Otherwise, nohz_idle_balancer does not run for the first 5
mins after bootup.

Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
Reviewed-by: Aleksandr Frid <afrid@nvidia.com>
Reviewed-by: Peter Boonstoppel <pboonstoppel@nvidia.com>
Reviewed-by: Satya Popuri <spopuri@nvidia.com>
---
 kernel/sched.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index b5efd6c..d3f291f 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -8383,6 +8383,7 @@ void __init sched_init(void)
 	atomic_set(&nohz.load_balancer, nr_cpu_ids);
 	atomic_set(&nohz.first_pick_cpu, nr_cpu_ids);
 	atomic_set(&nohz.second_pick_cpu, nr_cpu_ids);
+	nohz.next_balance = jiffies;
 #endif
 	/* May be allocated at isolcpus cmdline parse time */
 	if (cpu_isolated_map == NULL)
-- 
1.7.7.rc0.72.g4b5ea.dirty

Re: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[Peter Zijlstra]

On Mon, 2012-01-30 at 14:44 +0200, Peter De Schrijver wrote:
> From: Diwakar Tundlam <dtundlam@nvidia.com>
> 
> The next_balance parameter of nohz_idle_balancer should be initialized
> to jiffies since jiffies itself is initialized to 300 seconds shy of
> overflow. Otherwise, nohz_idle_balancer does not run for the first 5
> mins after bootup.
> 
> Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
> Reviewed-by: Aleksandr Frid <afrid@nvidia.com>
> Reviewed-by: Peter Boonstoppel <pboonstoppel@nvidia.com>
> Reviewed-by: Satya Popuri <spopuri@nvidia.com>
> ---
>  kernel/sched.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

You'll find that this file no longer exists...

> diff --git a/kernel/sched.c b/kernel/sched.c
> index b5efd6c..d3f291f 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -8383,6 +8383,7 @@ void __init sched_init(void)
>  	atomic_set(&nohz.load_balancer, nr_cpu_ids);
>  	atomic_set(&nohz.first_pick_cpu, nr_cpu_ids);
>  	atomic_set(&nohz.second_pick_cpu, nr_cpu_ids);
> +	nohz.next_balance = jiffies;
>  #endif
>  	/* May be allocated at isolcpus cmdline parse time */
>  	if (cpu_isolated_map == NULL)

Not does this code section..

RE: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[Diwakar Tundlam]

Yes, the change should be ported to kernel/sched/fair.c under init_sched_fair_class(). But that's for 3.3. I will send a separate patch for 3.3 shortly.

All previous kernels have the old structure where the patch can be applied as it is.

--Diwakar.

-----Original Message-----
From: Peter Zijlstra [mailto:peterz@infradead.org] 
Sent: Tuesday, January 31, 2012 7:45 AM
To: Peter De Schrijver
Cc: Diwakar Tundlam; Ingo Molnar; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

On Mon, 2012-01-30 at 14:44 +0200, Peter De Schrijver wrote:
> From: Diwakar Tundlam <dtundlam@nvidia.com>
> 
> The next_balance parameter of nohz_idle_balancer should be initialized 
> to jiffies since jiffies itself is initialized to 300 seconds shy of 
> overflow. Otherwise, nohz_idle_balancer does not run for the first 5 
> mins after bootup.
> 
> Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
> Reviewed-by: Aleksandr Frid <afrid@nvidia.com>
> Reviewed-by: Peter Boonstoppel <pboonstoppel@nvidia.com>
> Reviewed-by: Satya Popuri <spopuri@nvidia.com>
> ---
>  kernel/sched.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

You'll find that this file no longer exists...

> diff --git a/kernel/sched.c b/kernel/sched.c index b5efd6c..d3f291f 
> 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -8383,6 +8383,7 @@ void __init sched_init(void)
>  	atomic_set(&nohz.load_balancer, nr_cpu_ids);
>  	atomic_set(&nohz.first_pick_cpu, nr_cpu_ids);
>  	atomic_set(&nohz.second_pick_cpu, nr_cpu_ids);
> +	nohz.next_balance = jiffies;
>  #endif
>  	/* May be allocated at isolcpus cmdline parse time */
>  	if (cpu_isolated_map == NULL)

Not does this code section..

RE: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[Peter Zijlstra]

On Tue, 2012-01-31 at 11:18 -0800, Diwakar Tundlam wrote:
> Yes, the change should be ported to kernel/sched/fair.c under
> init_sched_fair_class(). But that's for 3.3. I will send a separate
> patch for 3.3 shortly.
> 
Patches are sent against the current (3.3-rc) or next (3.4) development
trees, anything older can possibly get backports through the stable
stuff but is completely irrelevant for patch submission.

RE: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[Diwakar Tundlam]

Here's the patch for kernel 3.3-rc1:

** kernel/sched/fair.c	2012-01-31 11:19:52.032421593 -0800
***************
*** 5585,5597 ****
  
  __init void init_sched_fair_class(void)
  {
  #ifdef CONFIG_SMP
  	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains);
  
  #ifdef CONFIG_NO_HZ
  	zalloc_cpumask_var(&nohz.idle_cpus_mask, GFP_NOWAIT);
+ 	nohz.next_balance = jiffies;
  #endif
  #endif /* SMP */
  
  }
--- 5585,5596 ----

-----Original Message-----
From: Peter Zijlstra [mailto:peterz@infradead.org] 
Sent: Tuesday, January 31, 2012 7:45 AM
To: Peter De Schrijver
Cc: Diwakar Tundlam; Ingo Molnar; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

On Mon, 2012-01-30 at 14:44 +0200, Peter De Schrijver wrote:
> From: Diwakar Tundlam <dtundlam@nvidia.com>
> 
> The next_balance parameter of nohz_idle_balancer should be initialized 
> to jiffies since jiffies itself is initialized to 300 seconds shy of 
> overflow. Otherwise, nohz_idle_balancer does not run for the first 5 
> mins after bootup.
> 
> Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
> Reviewed-by: Aleksandr Frid <afrid@nvidia.com>
> Reviewed-by: Peter Boonstoppel <pboonstoppel@nvidia.com>
> Reviewed-by: Satya Popuri <spopuri@nvidia.com>
> ---
>  kernel/sched.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

You'll find that this file no longer exists...

> diff --git a/kernel/sched.c b/kernel/sched.c index b5efd6c..d3f291f 
> 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -8383,6 +8383,7 @@ void __init sched_init(void)
>  	atomic_set(&nohz.load_balancer, nr_cpu_ids);
>  	atomic_set(&nohz.first_pick_cpu, nr_cpu_ids);
>  	atomic_set(&nohz.second_pick_cpu, nr_cpu_ids);
> +	nohz.next_balance = jiffies;
>  #endif
>  	/* May be allocated at isolcpus cmdline parse time */
>  	if (cpu_isolated_map == NULL)

Not does this code section..

RE: [PATCH] scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

[Peter Zijlstra]

On Tue, 2012-01-31 at 11:23 -0800, Diwakar Tundlam wrote:
> Here's the patch for kernel 3.3-rc1:
> 
> ** kernel/sched/fair.c	2012-01-31 11:19:52.032421593 -0800
> ***************
> *** 5585,5597 ****
>   
>   __init void init_sched_fair_class(void)
>   {
>   #ifdef CONFIG_SMP
>   	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains);
>   
>   #ifdef CONFIG_NO_HZ
>   	zalloc_cpumask_var(&nohz.idle_cpus_mask, GFP_NOWAIT);
> + 	nohz.next_balance = jiffies;
>   #endif
>   #endif /* SMP */
>   
>   }

This is not something I can apply, please see
Documentation/SubmittingPatches.

[PATCH] scheduler: domain: correctly initialize 'next_balance' in 'nohz' idle balancer

[Diwakar Tundlam]

scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

The 'next_balance' field of 'nohz' idle balancer must be initialized
to jiffies. Since jiffies is initialized to negative 300 seconds the
'nohz' idle balancer does not run for the first 300s (5mins) after
bootup. If no new processes are spawed or no idle cycles happen, the
load on the cpus will remain unbalanced for that duration.

Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
---
 1 files changed, 1 insertions(+), 0 deletions(-)

diff -up sched/fair.c{.orig,}
--- sched/fair.c.orig	2012-03-07 08:33:03.000000000 -0800
+++ sched/fair.c	2012-03-07 13:50:30.787265795 -0800
@@ -5614,6 +5614,7 @@ __init void init_sched_fair_class(void)
 	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains);
 
 #ifdef CONFIG_NO_HZ
+	nohz.next_balance = jiffies;
 	zalloc_cpumask_var(&nohz.idle_cpus_mask, GFP_NOWAIT);
 	cpu_notifier(sched_ilb_notifier, 0);
 #endif

Re: [PATCH] scheduler: domain: correctly initialize 'next_balance' in 'nohz' idle balancer

[Peter Zijlstra]

On Wed, 2012-03-07 at 14:44 -0800, Diwakar Tundlam wrote:
> scheduler: domain: init next_balance in nohz_idle_balancer with jiffies

That's already in the subject, no need to repeat that in the body.

> The 'next_balance' field of 'nohz' idle balancer must be initialized
> to jiffies. Since jiffies is initialized to negative 300 seconds the
> 'nohz' idle balancer does not run for the first 300s (5mins) after
> bootup. If no new processes are spawed or no idle cycles happen, the
> load on the cpus will remain unbalanced for that duration.
> 
> Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
> ---
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff -up sched/fair.c{.orig,}
> --- sched/fair.c.orig	2012-03-07 08:33:03.000000000 -0800
> +++ sched/fair.c	2012-03-07 13:50:30.787265795 -0800

This doesn't work, patches should apply with -p1. I just read
Documentation/SubmittingPatches and I must admit to that not being very
clear.

In any case, I can recommend to using quilt or git to create patches.

I've hand edited the patch for now, but in future please learn to use
either of these tools.

Final patch for re{f,v}erence.. :-)
---
Subject: scheduler: domain: correctly initialize 'next_balance' in 'nohz' idle balancer
From: Diwakar Tundlam <dtundlam@nvidia.com>
Date: Wed, 7 Mar 2012 14:44:26 -0800

The 'next_balance' field of 'nohz' idle balancer must be initialized
to jiffies. Since jiffies is initialized to negative 300 seconds the
'nohz' idle balancer does not run for the first 300s (5mins) after
bootup. If no new processes are spawed or no idle cycles happen, the
load on the cpus will remain unbalanced for that duration.

Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1DD7BFEDD3147247B1355BEFEFE4665237994F30EF@HQMAIL04.nvidia.com
---
 kernel/sched/fair.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5603,6 +5603,7 @@ __init void init_sched_fair_class(void)
 	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains);
 
 #ifdef CONFIG_NO_HZ
+	nohz.next_balance = jiffies;
 	zalloc_cpumask_var(&nohz.idle_cpus_mask, GFP_NOWAIT);
 	cpu_notifier(sched_ilb_notifier, 0);
 #endif

[PATCH] init: check printed flag to skip printing message

[Diwakar Tundlam]

Otherwise the 'Calibration skipped' message gets printed everytime a CPU is
hotplugged in, cluttering console for systems that frequently hotplug CPUs.

Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
---
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/init/calibrate.c b/init/calibrate.c
index 5f117ca..fda0a7b 100644
--- a/init/calibrate.c
+++ b/init/calibrate.c
@@ -267,7 +267,8 @@ void __cpuinit calibrate_delay(void)
 
	if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
		lpj = per_cpu(cpu_loops_per_jiffy, this_cpu);
-		pr_info("Calibrating delay loop (skipped) "
+		if (!printed)
+			pr_info("Calibrating delay loop (skipped) "
 			"already calibrated this CPU");
	} else if (preset_lpj) {
		lpj = preset_lpj;

Re: [PATCH] init: check printed flag to skip printing message

[Andrew Morton]

On Thu, 8 Mar 2012 15:44:16 -0800
Diwakar Tundlam <dtundlam@nvidia.com> wrote:

> Otherwise the 'Calibration skipped' message gets printed everytime a CPU is
> hotplugged in, cluttering console for systems that frequently hotplug CPUs.
> 
> Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
> ---
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/init/calibrate.c b/init/calibrate.c
> index 5f117ca..fda0a7b 100644
> --- a/init/calibrate.c
> +++ b/init/calibrate.c
> @@ -267,7 +267,8 @@ void __cpuinit calibrate_delay(void)
>  
> 	if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
> 		lpj = per_cpu(cpu_loops_per_jiffy, this_cpu);
> -		pr_info("Calibrating delay loop (skipped) "
> +		if (!printed)
> +			pr_info("Calibrating delay loop (skipped) "
>  			"already calibrated this CPU");
> 	} else if (preset_lpj) {
> 		lpj = preset_lpj;

oops, yes.



Looking at that code, I think Mike broke it in 2009 with feae3203d71
("timers, init: Limit the number of per cpu calibration bootup
messages"):


: diff --git a/init/calibrate.c b/init/calibrate.c
: index a379c90..6eb48e5 100644
: --- a/init/calibrate.c
: +++ b/init/calibrate.c
: @@ -123,23 +123,26 @@ void __cpuinit calibrate_delay(void)
:  {
:  	unsigned long ticks, loopbit;
:  	int lps_precision = LPS_PREC;
: +	static bool printed;
:  
:  	if (preset_lpj) {
:  		loops_per_jiffy = preset_lpj;
: -		printk(KERN_INFO
: -			"Calibrating delay loop (skipped) preset value.. ");
: -	} else if ((smp_processor_id() == 0) && lpj_fine) {
: +		if (!printed)
: +			pr_info("Calibrating delay loop (skipped) "
: +				"preset value.. ");
: +	} else if ((!printed) && lpj_fine) {
:  		loops_per_jiffy = lpj_fine;

What's going on here?  Why should the value of `printed' affect the
value of loops_per_jiffy?

(lpj_fine is set only in x86 tsc code, under late_time_init(), and
late_time_init() is called before calibrate_delay(), so we got that
part right).

:  		loops_per_jiffy = lpj_fine;
: -		printk(KERN_INFO
: -			"Calibrating delay loop (skipped), "
: +		pr_info("Calibrating delay loop (skipped), "
:  			"value calculated using timer frequency.. ");
:  	} else if ((loops_per_jiffy = calibrate_delay_direct()) != 0) {
: -		printk(KERN_INFO
: -			"Calibrating delay using timer specific routine.. ");
: +		if (!printed)
: +			pr_info("Calibrating delay using timer "
: +				"specific routine.. ");
:  	} else {
:  		loops_per_jiffy = (1<<12);
:  
: -		printk(KERN_INFO "Calibrating delay loop... ");
: +		if (!printed)
: +			pr_info("Calibrating delay loop... ");
:  		while ((loops_per_jiffy <<= 1) != 0) {
:  			/* wait for "start of" clock tick */
:  			ticks = jiffies;
: @@ -170,7 +173,10 @@ void __cpuinit calibrate_delay(void)
:  				loops_per_jiffy &= ~loopbit;
:  		}
:  	}
: -	printk(KERN_CONT "%lu.%02lu BogoMIPS (lpj=%lu)\n",
: +	if (!printed)
: +		pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
:  			loops_per_jiffy/(500000/HZ),
:  			(loops_per_jiffy/(5000/HZ)) % 100, loops_per_jiffy);
: +
: +	printed = true;
:  }
: diff --git a/kernel/cpu.c b/kernel/cpu.c
: index 6ba0f1e..7c4e271 100644
: --- a/kernel/cpu.c
: +++ b/kernel/cpu.c
: @@ -392,10 +392,9 @@ int disable_nonboot_cpus(void)
:  		if (cpu == first_cpu)
:  			continue;
:  		error = _cpu_down(cpu, 1);
: -		if (!error) {
: +		if (!error)
:  			cpumask_set_cpu(cpu, frozen_cpus);
: -			printk("CPU%d is down\n", cpu);
: -		} else {
: +		else {
:  			printk(KERN_ERR "Error taking CPU%d down: %d\n",
:  				cpu, error);
:  			break;

Re: [PATCH] init: check printed flag to skip printing message

[Joe Perches]

On Thu, 2012-03-08 at 15:44 -0800, Diwakar Tundlam wrote:
> Otherwise the 'Calibration skipped' message gets printed everytime a CPU is
> hotplugged in, cluttering console for systems that frequently hotplug CPUs.

Perhaps this is more uniform and it saves a few bytes too.

$ size init/calibrate.o*
   text	   data	    bss	    dec	    hex	filename
   2933	     73	    521	   3527	    dc7	init/calibrate.o.new
   2957	     73	    513	   3543	    dd7	init/calibrate.o.old

 init/calibrate.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/init/calibrate.c b/init/calibrate.c
index 5f117ca..4f0af03 100644
--- a/init/calibrate.c
+++ b/init/calibrate.c
@@ -267,31 +267,32 @@ void __cpuinit calibrate_delay(void)
 
 	if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
 		lpj = per_cpu(cpu_loops_per_jiffy, this_cpu);
-		pr_info("Calibrating delay loop (skipped) "
-				"already calibrated this CPU");
+		if (!printed)
+			pr_info("Calibrating delay loop%s",
+				" (skipped) already calibrated this CPU");
 	} else if (preset_lpj) {
 		lpj = preset_lpj;
 		if (!printed)
-			pr_info("Calibrating delay loop (skipped) "
-				"preset value.. ");
+			pr_info("Calibrating delay loop%s",
+				" (skipped) preset value..");
 	} else if ((!printed) && lpj_fine) {
 		lpj = lpj_fine;
-		pr_info("Calibrating delay loop (skipped), "
-			"value calculated using timer frequency.. ");
+		pr_info("Calibrating delay loop%s",
+			" (skipped), value calculated using timer frequency..");
 	} else if ((lpj = calibrate_delay_is_known())) {
 		;
 	} else if ((lpj = calibrate_delay_direct()) != 0) {
 		if (!printed)
-			pr_info("Calibrating delay using timer "
-				"specific routine.. ");
+			pr_info("Calibrating delay loop%s",
+				" using timer specific routine..");
 	} else {
 		if (!printed)
-			pr_info("Calibrating delay loop... ");
+			pr_info("Calibrating delay loop%s", "...");
 		lpj = calibrate_delay_converge();
 	}
 	per_cpu(cpu_loops_per_jiffy, this_cpu) = lpj;
 	if (!printed)
-		pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
+		pr_cont(" %lu.%02lu BogoMIPS (lpj=%lu)\n",
 			lpj/(500000/HZ),
 			(lpj/(5000/HZ)) % 100, lpj);
 

[tip:sched/core] sched/nohz: Correctly initialize 'next_balance' in 'nohz' idle balancer

[tip-bot for Diwakar Tundlam]

Commit-ID:  554cecaf733623b327eef9652b65965eb1081b81
Gitweb:     http://git.kernel.org/tip/554cecaf733623b327eef9652b65965eb1081b81
Author:     Diwakar Tundlam <dtundlam@nvidia.com>
AuthorDate: Wed, 7 Mar 2012 14:44:26 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 12 Mar 2012 20:43:16 +0100

sched/nohz: Correctly initialize 'next_balance' in 'nohz' idle balancer

The 'next_balance' field of 'nohz' idle balancer must be initialized
to jiffies. Since jiffies is initialized to negative 300 seconds the
'nohz' idle balancer does not run for the first 300s (5mins) after
bootup. If no new processes are spawed or no idle cycles happen, the
load on the cpus will remain unbalanced for that duration.

Signed-off-by: Diwakar Tundlam <dtundlam@nvidia.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1DD7BFEDD3147247B1355BEFEFE4665237994F30EF@HQMAIL04.nvidia.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched/fair.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index def17aa..11f3979 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5571,6 +5571,7 @@ __init void init_sched_fair_class(void)
 	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains);
 
 #ifdef CONFIG_NO_HZ
+	nohz.next_balance = jiffies;
 	zalloc_cpumask_var(&nohz.idle_cpus_mask, GFP_NOWAIT);
 	cpu_notifier(sched_ilb_notifier, 0);
 #endif