2015-04-28 02:21:56

by Joonwoo Park

[permalink] [raw]
Subject: [PATCH 1/2] timer: avoid unnecessary waking up of nohz CPU

At present, internal_add_timer() examines flags with 'base' which doesn't
contain flags. Examine with 'timer->base' to avoid unnecessary waking up
of nohz CPU when timer base has TIMER_DEFERRABLE.

CC: Thomas Gleixner <[email protected]>
CC: John Stultz <[email protected]>
Signed-off-by: Joonwoo Park <[email protected]>
---
kernel/time/timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 2ece3aa..e5d5733c 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -434,7 +434,7 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
* require special care against races with idle_cpu(), lets deal
* with that later.
*/
- if (!tbase_get_deferrable(base) || tick_nohz_full_cpu(base->cpu))
+ if (!tbase_get_deferrable(timer->base) || tick_nohz_full_cpu(base->cpu))
wake_up_nohz_cpu(base->cpu);
}

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


2015-04-28 21:59:08

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 1/2] timer: avoid unnecessary waking up of nohz CPU

On 04/27/15 19:21, Joonwoo Park wrote:
> At present, internal_add_timer() examines flags with 'base' which doesn't
> contain flags. Examine with 'timer->base' to avoid unnecessary waking up
> of nohz CPU when timer base has TIMER_DEFERRABLE.
>
> CC: Thomas Gleixner <[email protected]>
> CC: John Stultz <[email protected]>
> Signed-off-by: Joonwoo Park <[email protected]>
> ---
> kernel/time/timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 2ece3aa..e5d5733c 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -434,7 +434,7 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
> * require special care against races with idle_cpu(), lets deal
> * with that later.
> */
> - if (!tbase_get_deferrable(base) || tick_nohz_full_cpu(base->cpu))
> + if (!tbase_get_deferrable(timer->base) || tick_nohz_full_cpu(base->cpu))
> wake_up_nohz_cpu(base->cpu);
> }
>

This looks like a fix for commit 9f6d9baaa8ca (timer: Kick dynticks
targets on mod_timer*() calls, 2014-06-22) which changed the code to use
base instead of timer->base when it moved this condition from
add_timer_on() into internal_add_timer(). Is that right?

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-04-29 02:42:01

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] timer: avoid unnecessary waking up of nohz CPU

On Tue, Apr 28, 2015 at 7:51 AM, Joonwoo Park <[email protected]> wrote:
> At present, internal_add_timer() examines flags with 'base' which doesn't
> contain flags. Examine with 'timer->base' to avoid unnecessary waking up
> of nohz CPU when timer base has TIMER_DEFERRABLE.
>
> CC: Thomas Gleixner <[email protected]>
> CC: John Stultz <[email protected]>
> Signed-off-by: Joonwoo Park <[email protected]>
> ---
> kernel/time/timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 2ece3aa..e5d5733c 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -434,7 +434,7 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
> * require special care against races with idle_cpu(), lets deal
> * with that later.
> */
> - if (!tbase_get_deferrable(base) || tick_nohz_full_cpu(base->cpu))
> + if (!tbase_get_deferrable(timer->base) || tick_nohz_full_cpu(base->cpu))
> wake_up_nohz_cpu(base->cpu);
> }

Oops !!

Fixes: 9f6d9baaa8ca ("timer: Kick dynticks targets on mod_timer*() calls")
Cc: 3.17+ <[email protected]> # 3.17+

Reviewed-by: Viresh Kumar <[email protected]>

Subject: [tip:timers/core] timer: Use timer->base for flag checks

Commit-ID: 781978e6e156101209f62b9ebc8783b70ef248de
Gitweb: http://git.kernel.org/tip/781978e6e156101209f62b9ebc8783b70ef248de
Author: Joonwoo Park <[email protected]>
AuthorDate: Mon, 27 Apr 2015 19:21:49 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Tue, 5 May 2015 10:40:43 +0200

timer: Use timer->base for flag checks

At present, internal_add_timer() examines flags with 'base' which doesn't
contain flags. Examine with 'timer->base' to avoid unnecessary waking up
of nohz CPU when timer base has TIMER_DEFERRABLE set.

Signed-off-by: Joonwoo Park <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: John Stultz <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/time/timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 03f926c..d4af7c5 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -436,7 +436,7 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
* require special care against races with idle_cpu(), lets deal
* with that later.
*/
- if (!tbase_get_deferrable(base) || tick_nohz_full_cpu(base->cpu))
+ if (!tbase_get_deferrable(timer->base) || tick_nohz_full_cpu(base->cpu))
wake_up_nohz_cpu(base->cpu);
}