On Tue, Mar 18, 2014 at 04:26:07PM +0530, Viresh Kumar wrote:

There are only two users of get_nohz_timer_target(): timer and hrtimer. Both call it under same circumstances, i.e.

#ifdef CONFIG_NO_HZ_COMMON if (!pinned && get_sysctl_timer_migration() && idle_cpu(this_cpu)) return get_nohz_timer_target(); #endif

So, it makes more sense to get all this as part of get_nohz_timer_target() instead of duplicating code at two places. For this another parameter is required to be passed to this routine, pinned.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org

include/linux/sched.h | 6 +++++- kernel/hrtimer.c | 15 +-------------- kernel/sched/core.c | 5 ++++- kernel/timer.c | 7 +------ 4 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h index 6dbb9e5..9819f93 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -299,10 +299,14 @@ extern int runqueue_is_locked(int cpu); #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON) extern void nohz_balance_enter_idle(int cpu); extern void set_cpu_sd_state_idle(void); -extern int get_nohz_timer_target(void); +extern int get_nohz_timer_target(int pinned); #else static inline void nohz_balance_enter_idle(int cpu) { } static inline void set_cpu_sd_state_idle(void) { } +static inline int get_nohz_timer_target(int pinned) +{

• return smp_processor_id();

+} #endif /* diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index f60dce8..7eef615 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -168,19 +168,6 @@ struct hrtimer_clock_base *lock_hrtimer_base(const struct hrtimer *timer, } }

-/*

• • Get the preferred target CPU for NOHZ
• */

-static int hrtimer_get_target(int this_cpu, int pinned) -{ -#ifdef CONFIG_NO_HZ_COMMON

• if (!pinned && get_sysctl_timer_migration() && idle_cpu(this_cpu))

• return get_nohz_timer_target();
  

-#endif

• return this_cpu;

-}

/*

• With HIGHRES=y we do not migrate the timer when it is expiring
• before the next event on the target cpu because we cannot reprogram

@@ -214,7 +201,7 @@ switch_hrtimer_base(struct hrtimer *timer, struct hrtimer_clock_base *base, struct hrtimer_clock_base *new_base; struct hrtimer_cpu_base *new_cpu_base; int this_cpu = smp_processor_id();

• int cpu = hrtimer_get_target(this_cpu, pinned);

• int cpu = get_nohz_timer_target(pinned); int basenum = base->index;

again: diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 6fde755..a58e24d 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -556,12 +556,15 @@ void resched_cpu(int cpu)

• selecting an idle cpu will add more delays to the timers than intended
• (as that cpu's timer base may not be uptodate wrt jiffies etc).

*/ -int get_nohz_timer_target(void) +int get_nohz_timer_target(int pinned) { int cpu = smp_processor_id(); int i; struct sched_domain *sd;

• if (pinned || !get_sysctl_timer_migration() || !idle_cpu(cpu))

• return cpu;
  

Maybe the pinned part should stay a caller detail. Although I don't really mind.

The patch looks good!

Thanks.

• rcu_read_lock(); for_each_domain(cpu, sd) { for_each_cpu(i, sched_domain_span(sd)) {

diff --git a/kernel/timer.c b/kernel/timer.c index e6868bf..2cf8616 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -760,12 +760,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, debug_activate(timer, expires);

• cpu = smp_processor_id();

-#if defined(CONFIG_NO_HZ_COMMON) && defined(CONFIG_SMP)

• if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu))

• cpu = get_nohz_timer_target();
  

-#endif

• cpu = get_nohz_timer_target(pinned); new_base = per_cpu(tvec_bases, cpu);

if (base != new_base) { -- 1.7.12.rc2.18.g61b472e