[PATCH v2 0/3] nohz_full clean up and bug fix

[PATCH v2 0/3] nohz_full clean up and bug fix

[Alex Shi]

The first used a wrong signed-off email addr, Sorry!

--
Thanks
	Alex

[PATCH v2 1/3] nohz_full: fix code sytle issue of tick_nohz_full_stop_tick

[Alex Shi]

Code usually starts with 'tab' instead of 7 'space'es in kernel

Signed-off-by: Alex Shi <alex.shi@linaro.org>
---
 kernel/time/tick-sched.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3612fc7..b55d1c0 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -681,18 +681,18 @@ out:
 static void tick_nohz_full_stop_tick(struct tick_sched *ts)
 {
 #ifdef CONFIG_NO_HZ_FULL
-       int cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
-       if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
-               return;
+	if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
+		return;
 
-       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
-	       return;
+	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
+		return;
 
-       if (!can_stop_full_tick())
-               return;
+	if (!can_stop_full_tick())
+		return;
 
-       tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+	tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
 #endif
 }
 
-- 
1.8.1.2

Re: [PATCH v2 1/3] nohz_full: fix code sytle issue of tick_nohz_full_stop_tick

[Morten Rasmussen]

In subject: s/sytle/style/

On Tue, Dec 03, 2013 at 12:35:10PM +0000, Alex Shi wrote:
> Code usually starts with 'tab' instead of 7 'space'es in kernel
> 
> Signed-off-by: Alex Shi <alex.shi@linaro.org>
> ---
>  kernel/time/tick-sched.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 3612fc7..b55d1c0 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -681,18 +681,18 @@ out:
>  static void tick_nohz_full_stop_tick(struct tick_sched *ts)
>  {
>  #ifdef CONFIG_NO_HZ_FULL
> -       int cpu = smp_processor_id();
> +	int cpu = smp_processor_id();
>  
> -       if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
> -               return;
> +	if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
> +		return;
>  
> -       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
> -	       return;
> +	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
> +		return;
>  
> -       if (!can_stop_full_tick())
> -               return;
> +	if (!can_stop_full_tick())
> +		return;
>  
> -       tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
> +	tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
>  #endif
>  }
>  
> -- 
> 1.8.1.2
> 
> 

Re: [PATCH v2 1/3] nohz_full: fix code sytle issue of tick_nohz_full_stop_tick

[Alex Shi]

On 12/03/2013 11:00 PM, Morten Rasmussen wrote:
> In subject: s/sytle/style/
> 

Thanks for this catch. updated.

===
>From 8d430d26c06c31d536c5ad38b8711d738fc25eff Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@linaro.org>
Date: Thu, 28 Nov 2013 14:27:11 +0800
Subject: [PATCH v2 1/3] nohz_full: fix code style issue of
 tick_nohz_full_stop_tick

Code usually starts with 'tab' instead of 7 'space' in kernel

Signed-off-by: Alex Shi <alex.shi@linaro.org>
---
 kernel/time/tick-sched.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3612fc7..b55d1c0 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -681,18 +681,18 @@ out:
 static void tick_nohz_full_stop_tick(struct tick_sched *ts)
 {
 #ifdef CONFIG_NO_HZ_FULL
-       int cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
-       if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
-               return;
+	if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
+		return;
 
-       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
-	       return;
+	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
+		return;
 
-       if (!can_stop_full_tick())
-               return;
+	if (!can_stop_full_tick())
+		return;
 
-       tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+	tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
 #endif
 }
 
-- 
1.8.1.2


-- 
Thanks
    Alex

Re: [PATCH v2 1/3] nohz_full: fix code sytle issue of tick_nohz_full_stop_tick

[Alex Shi]

On 12/03/2013 11:00 PM, Morten Rasmussen wrote:
> In subject: s/sytle/style/
> 
> On Tue, Dec 03, 2013 at 12:35:10PM +0000, Alex Shi wrote:
>> > Code usually starts with 'tab' instead of 7 'space'es in kernel
>> > 

Found another function has the code style issue too.

>From f594f0b721e57f90b51d0dd1692e5aae67d83b81 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@linaro.org>
Date: Thu, 28 Nov 2013 14:27:11 +0800
Subject: [PATCH 1/4] nohz_full: fix code style issue

Code usually starts with 'tab' instead of 7 'space'es in kernel.

Signed-off-by: Alex Shi <alex.shi@linaro.org>
---
 kernel/sched/core.c      | 16 ++++++++--------
 kernel/time/tick-sched.c | 16 ++++++++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c180860..636d89e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -664,18 +664,18 @@ static inline bool got_nohz_idle_kick(void)
 #ifdef CONFIG_NO_HZ_FULL
 bool sched_can_stop_tick(void)
 {
-       struct rq *rq;
+	struct rq *rq;
 
-       rq = this_rq();
+	rq = this_rq();
 
-       /* Make sure rq->nr_running update is visible after the IPI */
-       smp_rmb();
+	/* Make sure rq->nr_running update is visible after the IPI */
+	smp_rmb();
 
-       /* More than one running task need preemption */
-       if (rq->nr_running > 1)
-               return false;
+	/* More than one running task need preemption */
+	if (rq->nr_running > 1)
+		return false;
 
-       return true;
+	return true;
 }
 #endif /* CONFIG_NO_HZ_FULL */
 
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3612fc7..b55d1c0 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -681,18 +681,18 @@ out:
 static void tick_nohz_full_stop_tick(struct tick_sched *ts)
 {
 #ifdef CONFIG_NO_HZ_FULL
-       int cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
-       if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
-               return;
+	if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
+		return;
 
-       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
-	       return;
+	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
+		return;
 
-       if (!can_stop_full_tick())
-               return;
+	if (!can_stop_full_tick())
+		return;
 
-       tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+	tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
 #endif
 }
 
-- 
1.8.1.2


-- 
Thanks
    Alex

[tip:timers/urgent] nohz_full: fix code style issue of tick_nohz_full_stop_tick

[tip-bot for Alex Shi]

Commit-ID:  e9a2eb403bd953788cd2abfd0d2646d43bd22671
Gitweb:     http://git.kernel.org/tip/e9a2eb403bd953788cd2abfd0d2646d43bd22671
Author:     Alex Shi <alex.shi@linaro.org>
AuthorDate: Thu, 28 Nov 2013 14:27:11 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Wed, 15 Jan 2014 23:07:11 +0100

nohz_full: fix code style issue of tick_nohz_full_stop_tick

Code usually starts with 'tab' instead of 7 'space' in kernel

Signed-off-by: Alex Shi <alex.shi@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alex Shi <alex.shi@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Kevin Hilman <khilman@linaro.org>
Link: http://lkml.kernel.org/r/1386074112-30754-2-git-send-email-alex.shi@linaro.org
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/time/tick-sched.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 68331d1..d603bad 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -679,18 +679,18 @@ out:
 static void tick_nohz_full_stop_tick(struct tick_sched *ts)
 {
 #ifdef CONFIG_NO_HZ_FULL
-       int cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
-       if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
-               return;
+	if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
+		return;
 
-       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
-	       return;
+	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
+		return;
 
-       if (!can_stop_full_tick())
-               return;
+	if (!can_stop_full_tick())
+		return;
 
-       tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+	tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
 #endif
 }
 

[PATCH v2 2/3] nohz_full: unify nohz_full funcs

[Alex Shi]

All other nohz_full funcs named like xxx_nohz_full_xxx.
So renaming wake_up_full_nohz_cpu to wake_up_nohz_full_cpu to unify
funs.

Signed-off-by: Alex Shi <alex.shi@linaro.org>
---
 kernel/sched/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c180860..6c9f9b0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -616,7 +616,7 @@ static void wake_up_idle_cpu(int cpu)
 		smp_send_reschedule(cpu);
 }
 
-static bool wake_up_full_nohz_cpu(int cpu)
+static bool wake_up_nohz_full_cpu(int cpu)
 {
 	if (tick_nohz_full_cpu(cpu)) {
 		if (cpu != smp_processor_id() ||
@@ -630,7 +630,7 @@ static bool wake_up_full_nohz_cpu(int cpu)
 
 void wake_up_nohz_cpu(int cpu)
 {
-	if (!wake_up_full_nohz_cpu(cpu))
+	if (!wake_up_nohz_full_cpu(cpu))
 		wake_up_idle_cpu(cpu);
 }
 
-- 
1.8.1.2

[PATCH v2 3/3] nohz_full: update cpu load fix in nohz_full

[Alex Shi]

We are not always 0 when update nohz cpu load, after nohz_full enabled.
But current code still treat the cpu as idle. that is incorrect.
Fix it to use correct cpu_load.

Signed-off-by: Alex Shi <alex.shi@linaro.org>
---
 kernel/sched/proc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/proc.c b/kernel/sched/proc.c
index 16f5a30..f1441f0 100644
--- a/kernel/sched/proc.c
+++ b/kernel/sched/proc.c
@@ -568,8 +568,14 @@ void update_cpu_load_nohz(void)
 		/*
 		 * We were idle, this means load 0, the current load might be
 		 * !0 due to remote wakeups and the sort.
+		 * or we may has only one task and in NO_HZ_FULL, then still use
+		 * normal cpu load.
 		 */
-		__update_cpu_load(this_rq, 0, pending_updates);
+		if (this_rq->cfs.h_nr_running) {
+			unsigned load = get_rq_runnable_load(this_rq);
+			__update_cpu_load(this_rq, load, pending_updates);
+		} else
+			__update_cpu_load(this_rq, 0, pending_updates);
 	}
 	raw_spin_unlock(&this_rq->lock);
 }
-- 
1.8.1.2

Re: [PATCH v2 3/3] nohz_full: update cpu load fix in nohz_full

[Alex Shi]

On 12/03/2013 08:35 PM, Alex Shi wrote:
> We are not always 0 when update nohz cpu load, after nohz_full enabled.
> But current code still treat the cpu as idle. that is incorrect.
> Fix it to use correct cpu_load.

Frederic, Could you like to give some comments?

> 
> Signed-off-by: Alex Shi <alex.shi@linaro.org>
> ---
>  kernel/sched/proc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/proc.c b/kernel/sched/proc.c
> index 16f5a30..f1441f0 100644
> --- a/kernel/sched/proc.c
> +++ b/kernel/sched/proc.c
> @@ -568,8 +568,14 @@ void update_cpu_load_nohz(void)
>  		/*
>  		 * We were idle, this means load 0, the current load might be
>  		 * !0 due to remote wakeups and the sort.
> +		 * or we may has only one task and in NO_HZ_FULL, then still use
> +		 * normal cpu load.
>  		 */
> -		__update_cpu_load(this_rq, 0, pending_updates);
> +		if (this_rq->cfs.h_nr_running) {
> +			unsigned load = get_rq_runnable_load(this_rq);
> +			__update_cpu_load(this_rq, load, pending_updates);
> +		} else
> +			__update_cpu_load(this_rq, 0, pending_updates);
>  	}
>  	raw_spin_unlock(&this_rq->lock);
>  }
> 


-- 
Thanks
    Alex

Re: [PATCH v2 3/3] nohz_full: update cpu load fix in nohz_full

[Alex Shi]

On 12/04/2013 02:17 PM, Alex Shi wrote:
> On 12/03/2013 08:35 PM, Alex Shi wrote:
>> > We are not always 0 when update nohz cpu load, after nohz_full enabled.
>> > But current code still treat the cpu as idle. that is incorrect.
>> > Fix it to use correct cpu_load.
> Frederic, Could you like to give some comments?
> 

ping Frederic.

-- 
Thanks
    Alex

Re: [PATCH v2 3/3] nohz_full: update cpu load fix in nohz_full

[Frederic Weisbecker]

On Tue, Dec 03, 2013 at 08:35:12PM +0800, Alex Shi wrote:
> We are not always 0 when update nohz cpu load, after nohz_full enabled.
> But current code still treat the cpu as idle. that is incorrect.
> Fix it to use correct cpu_load.
> 
> Signed-off-by: Alex Shi <alex.shi@linaro.org>
> ---
>  kernel/sched/proc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/proc.c b/kernel/sched/proc.c
> index 16f5a30..f1441f0 100644
> --- a/kernel/sched/proc.c
> +++ b/kernel/sched/proc.c
> @@ -568,8 +568,14 @@ void update_cpu_load_nohz(void)
>  		/*
>  		 * We were idle, this means load 0, the current load might be
>  		 * !0 due to remote wakeups and the sort.
> +		 * or we may has only one task and in NO_HZ_FULL, then still use
> +		 * normal cpu load.
>  		 */
> -		__update_cpu_load(this_rq, 0, pending_updates);
> +		if (this_rq->cfs.h_nr_running) {
> +			unsigned load = get_rq_runnable_load(this_rq);
> +			__update_cpu_load(this_rq, load, pending_updates);
> +		} else
> +			__update_cpu_load(this_rq, 0, pending_updates);

But decay_load_missed() doesnt handle non 0 loads, right? It probably make more sense
to first fix __update_cpu_load() to make it handle this kind of thing before fixing the caller.

Now you had patches that remove the cpu_load secondary idx I think? You should move this patch to
that series.

>  	}
>  	raw_spin_unlock(&this_rq->lock);
>  }
> -- 
> 1.8.1.2
> 

Re: [PATCH v2 3/3] nohz_full: update cpu load fix in nohz_full

[Alex Shi]

On 12/10/2013 10:02 PM, Frederic Weisbecker wrote:
>> >  		 * We were idle, this means load 0, the current load might be
>> >  		 * !0 due to remote wakeups and the sort.
>> > +		 * or we may has only one task and in NO_HZ_FULL, then still use
>> > +		 * normal cpu load.
>> >  		 */
>> > -		__update_cpu_load(this_rq, 0, pending_updates);
>> > +		if (this_rq->cfs.h_nr_running) {
>> > +			unsigned load = get_rq_runnable_load(this_rq);
>> > +			__update_cpu_load(this_rq, load, pending_updates);
>> > +		} else
>> > +			__update_cpu_load(this_rq, 0, pending_updates);
> But decay_load_missed() doesnt handle non 0 loads, right? It probably make more sense
> to first fix __update_cpu_load() to make it handle this kind of thing before fixing the caller.
> 
> Now you had patches that remove the cpu_load secondary idx I think? You should move this patc


Thanks for response!
Yes, after get your review, I plan to merge this patch with cpu_load
decay removing, if that patchset possible get to upstream. :)

-- 
Thanks
    Alex

Re: [PATCH v2 0/3] nohz_full clean up and bug fix

[Alex Shi]

On 12/03/2013 08:35 PM, Alex Shi wrote:
> The first used a wrong signed-off email addr, Sorry!

The first version -- v1 used a wrong email address. Sorry!
> 
> --
> Thanks
> 	Alex
> 


-- 
Thanks
    Alex