The comment was probably added when there were only two clock bases available for hrtimers, now there are four.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index a1120a0..3c05140 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -530,8 +530,7 @@ static inline int hrtimer_hres_active(void) }

/* - * Reprogram the event source with checking both queues for the - * next event + * Reprogram the event source with checking all queues for the next event. * Called with interrupts disabled and base->lock held */ static void

'for' loop in migrate_hrtimers() has only one line in its body and so doesn't require {} as per coding guidelines. Remove it.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index c87e896..514b53a 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1712,10 +1712,9 @@ static void migrate_hrtimers(int scpu) raw_spin_lock(&new_base->lock); raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);

- for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) { + for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) migrate_hrtimer_list(&old_base->clock_base[i], &new_base->clock_base[i]); - }

raw_spin_unlock(&old_base->lock); raw_spin_unlock(&new_base->lock);

hrtimer_force_reprogram() is only called from parts of kernel which are defined between #ifdef CONFIG_HIGH_RES_TIMERS and so its empty definition is never used. Remove it.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 2 -- 1 file changed, 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 4843238..f6b1968 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -730,8 +730,6 @@ void clock_was_set_delayed(void) static inline int hrtimer_hres_active(void) { return 0; } static inline int hrtimer_is_hres_enabled(void) { return 0; } static inline int hrtimer_switch_to_hres(void) { return 0; } -static inline void -hrtimer_force_reprogram(struct hrtimer_cpu_base *base, int skip_equal) { } static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer, struct hrtimer_clock_base *base) {

unlock_hrtimer_base() was handled specially at a separate place earlier as lock_hrtimer_base() had separate definitions for SMP and non-SMP cases, but unlock_hrtimer_base() had only a single definition. And so probably it was kept at the end of this #ifdef/endif CONFIG_SMP. But this #ifdef ends right after lock_hrtimer_base()'s definition (Atleast in the current code) and so we can move unlock_hrtimer_base() upwards, i.e. closer to its counterparts. This improves readability of the code.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index db580ab..3b29023 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -254,6 +254,12 @@ lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags)

#endif /* !CONFIG_SMP */

+static inline +void unlock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) +{ + raw_spin_unlock_irqrestore(&timer->base->cpu_base->lock, *flags); +} + /* * Functions for the union type storage format of ktime_t which are * too large for inlining: @@ -805,15 +811,6 @@ static inline void timer_stats_account_hrtimer(struct hrtimer *timer) #endif }

-/* - * Counterpart to lock_hrtimer_base above: - */ -static inline -void unlock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) -{ - raw_spin_unlock_irqrestore(&timer->base->cpu_base->lock, *flags); -} - /** * hrtimer_forward - forward the timer expiry * @timer: hrtimer to forward

In switch_hrtimer_base() we have created a local variable basenum which is set to base->index initially. This variable is used at only one place. It makes code more readable if we remove this variable and use base->index directly.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 3b29023..04e4c77 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -202,11 +202,10 @@ switch_hrtimer_base(struct hrtimer *timer, struct hrtimer_clock_base *base, struct hrtimer_cpu_base *new_cpu_base; int this_cpu = smp_processor_id(); int cpu = get_nohz_timer_target(pinned); - int basenum = base->index;

again: new_cpu_base = &per_cpu(hrtimer_bases, cpu); - new_base = &new_cpu_base->clock_base[basenum]; + new_base = &new_cpu_base->clock_base[base->index];

if (base != new_base) { /*

retrigger_next_event() is defined within #ifdef CONFIG_HIGH_RES_TIMERS as we already have pointer to base available. So it makes more sense to simple use base->hres_active instead of doing this by calling hrtimer_hres_active():

__this_cpu_read(hrtimer_bases.hres_active)

Also the same reason apply to code in __remove_hrtimer().

There is one more noticeable issue with __remove_hrtimer() without this patch. We are checking hres_active of *this cpu's* base, where as it is not guanrateed at all that __remove_hrtimer() would be called on this CPU. Specially from the migration code, timer's CPU is already down.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index b0fbf12..ad5b7ba 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -672,7 +672,7 @@ static void retrigger_next_event(void *arg) { struct hrtimer_cpu_base *base = &__get_cpu_var(hrtimer_bases);

- if (!hrtimer_hres_active()) + if (!base->hres_active) return;

raw_spin_lock(&base->lock); @@ -897,7 +897,7 @@ static void __remove_hrtimer(struct hrtimer *timer, if (&timer->node == next_timer) { #ifdef CONFIG_HIGH_RES_TIMERS /* Reprogram the clock event device. if enabled */ - if (reprogram && hrtimer_hres_active()) { + if (reprogram && base->cpu_base->hres_active) { ktime_t expires;

expires = ktime_sub(hrtimer_get_expires(timer),

This patch reorders code within __remove_hrtimer() routine to achieve this: - no need to check if timer is the next timer to expire when high resolution mode isn't configured in kernel. So, move this check within the #ifdef/endif block. - Validate 'reprogram' and hrtimer_hres_active() first as without these we don't need to check if 'timer' is the next one to fire.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 476ad5d..833db9f 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -891,19 +891,18 @@ static void __remove_hrtimer(struct hrtimer *timer,

next_timer = timerqueue_getnext(&base->active); timerqueue_del(&base->active, &timer->node); - if (&timer->node == next_timer) { + #ifdef CONFIG_HIGH_RES_TIMERS - /* Reprogram the clock event device. if enabled */ - if (reprogram && base->cpu_base->hres_active) { - ktime_t expires; - - expires = ktime_sub(hrtimer_get_expires(timer), - base->offset); - if (base->cpu_base->expires_next.tv64 == expires.tv64) - hrtimer_force_reprogram(base->cpu_base, 1); - } -#endif + /* Reprogram the clock event device. if enabled */ + if (reprogram && base->cpu_base->hres_active && + &timer->node == next_timer) { + ktime_t expires; + + expires = ktime_sub(hrtimer_get_expires(timer), base->offset); + if (base->cpu_base->expires_next.tv64 == expires.tv64) + hrtimer_force_reprogram(base->cpu_base, 1); } +#endif if (!timerqueue_getnext(&base->active)) base->cpu_base->active_bases &= ~(1 << base->index); out:

Following code is repeated at many places: ktime_sub(hrtimer_get_expires(timer), base->offset);

and so it makes sense to create a separate inlined routine for this.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/hrtimer.h | 6 ++++++ kernel/hrtimer.c | 11 +++++------ 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 17c08ca..d1836cb 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -233,6 +233,12 @@ static inline ktime_t hrtimer_get_expires(const struct hrtimer *timer) return timer->node.expires; }

+static inline ktime_t hrtimer_get_monoexpires(const struct hrtimer *timer, + struct hrtimer_clock_base *base) +{ + return ktime_sub(hrtimer_get_expires(timer), base->offset); +} + static inline ktime_t hrtimer_get_softexpires(const struct hrtimer *timer) { return timer->_softexpires; diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 5b0cbe7..1a1fdc0 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -184,7 +184,7 @@ hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base) if (!new_base->cpu_base->hres_active) return 0;

- expires = ktime_sub(hrtimer_get_expires(timer), new_base->offset); + expires = hrtimer_get_monoexpires(timer, new_base); return expires.tv64 <= new_base->cpu_base->expires_next.tv64; #else return 0; @@ -554,7 +554,7 @@ hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal) continue; timer = container_of(next, struct hrtimer, node);

- expires = ktime_sub(hrtimer_get_expires(timer), base->offset); + expires = hrtimer_get_monoexpires(timer, base); /* * clock_was_set() has changed base->offset so the * result might be negative. Fix it up to prevent a @@ -588,7 +588,7 @@ static int hrtimer_reprogram(struct hrtimer *timer, struct hrtimer_clock_base *base) { struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases); - ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset); + ktime_t expires = hrtimer_get_monoexpires(timer, base); int res;

WARN_ON_ONCE(hrtimer_get_expires_tv64(timer) < 0); @@ -898,7 +898,7 @@ static void __remove_hrtimer(struct hrtimer *timer, &timer->node == next_timer) { ktime_t expires;

- expires = ktime_sub(hrtimer_get_expires(timer), base->offset); + expires = hrtimer_get_monoexpires(timer, base); if (base->cpu_base->expires_next.tv64 == expires.tv64) hrtimer_force_reprogram(base->cpu_base, 1); } @@ -1309,8 +1309,7 @@ retry: if (basenow.tv64 < hrtimer_get_softexpires_tv64(timer)) { ktime_t expires;

- expires = ktime_sub(hrtimer_get_expires(timer), - base->offset); + expires = hrtimer_get_monoexpires(timer, base); if (expires.tv64 < 0) expires.tv64 = KTIME_MAX; if (expires.tv64 < expires_next.tv64)

Complete bottom part of switch_hrtimer_base() is part of a 'if' block and so all code present in that block has extra indentation level before it. Rewrite it to remove this extra indentation level.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 58b5e3f..fe13dcf 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -207,33 +207,34 @@ again: new_cpu_base = &per_cpu(hrtimer_bases, cpu); new_base = &new_cpu_base->clock_base[base->index];

- if (base != new_base) { - /* - * We are trying to move timer to new_base. - * However we can't change timer's base while it is running, - * so we keep it on the same CPU. No hassle vs. reprogramming - * the event source in the high resolution case. The softirq - * code will take care of this when the timer function has - * completed. There is no conflict as we hold the lock until - * the timer is enqueued. - */ - if (unlikely(hrtimer_callback_running(timer))) - return base; - - /* See the comment in lock_timer_base() */ - timer->base = NULL; - raw_spin_unlock(&base->cpu_base->lock); - raw_spin_lock(&new_base->cpu_base->lock); + if (base == new_base) + return base;

- if (cpu != this_cpu && hrtimer_check_target(timer, new_base)) { - cpu = this_cpu; - raw_spin_unlock(&new_base->cpu_base->lock); - raw_spin_lock(&base->cpu_base->lock); - timer->base = base; - goto again; - } - timer->base = new_base; + /* + * We are trying to move timer to new_base. However we can't change + * timer's base while it is running, so we keep it on the same CPU. No + * hassle vs. reprogramming the event source in the high resolution + * case. The softirq code will take care of this when the timer function + * has completed. There is no conflict as we hold the lock until the + * timer is enqueued. + */ + if (unlikely(hrtimer_callback_running(timer))) + return base; + + /* See the comment in lock_timer_base() */ + timer->base = NULL; + raw_spin_unlock(&base->cpu_base->lock); + raw_spin_lock(&new_base->cpu_base->lock); + + if (cpu != this_cpu && hrtimer_check_target(timer, new_base)) { + cpu = this_cpu; + raw_spin_unlock(&new_base->cpu_base->lock); + raw_spin_lock(&base->cpu_base->lock); + timer->base = base; + goto again; } + + timer->base = new_base; return new_base; }

This code was added long back by following commit:

commit 06027bdd278a32a84b273e41db68a5db8ffd2bb6 Author: Ingo Molnar mingo@elte.hu Date: Tue Feb 14 13:53:15 2006 -0800

[PATCH] hrtimer: round up relative start time on low-res arches

Don't know if it was a mistake or was intentional. But probably we must use new_base instead of base here to get resolution. Things might be working smoothly as resolution might be same for both the bases in most of the cases.

Also commit log of above commit has this: "This will go away with the GTOD framework". So, should we get this removed?

Cc: Ingo Molnar mingo@redhat.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 2ac423d..458b952 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -964,7 +964,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, * timeouts. This will go away with the GTOD framework. */ #ifdef CONFIG_TIME_LOW_RES - tim = ktime_add_safe(tim, base->resolution); + tim = ktime_add_safe(tim, new_base->resolution); #endif }

Timers are removed from the red-black trees from __remove_hrtimer() and after removing timer from the tree, it calls hrtimer_force_reprogram() which might use value of cpu_base->active_bases. If the timer being removed is the last one on that clock base, then cpu_base->active_bases wouldn't give the right value, as there are no timers queued on the base but active base still marks it as active.

So, clear entry from active_bases as soon as timer is removed.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 2d5bb9d..379d21a 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -895,6 +895,9 @@ static void __remove_hrtimer(struct hrtimer *timer, next_timer = timerqueue_getnext(&base->active); timerqueue_del(&base->active, &timer->node);

+ if (!timerqueue_getnext(&base->active)) + base->cpu_base->active_bases &= ~(1 << base->index); + #ifdef CONFIG_HIGH_RES_TIMERS /* Reprogram the clock event device. if enabled */ if (reprogram && base->cpu_base->hres_active && @@ -907,8 +910,6 @@ static void __remove_hrtimer(struct hrtimer *timer, } #endif timer->state = newstate; - if (!timerqueue_getnext(&base->active)) - base->cpu_base->active_bases &= ~(1 << base->index); }

/*

There are various places where we are currently running a loop of HRTIMER_MAX_CLOCK_BASES iterations. We just run 'continue;' if there are no timers added to a clock base. Instead we can use the new for_each_active_base() routine to iterate over only the bases which are currently active.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 58 ++++++++++++++++++++++---------------------------------- 1 file changed, 23 insertions(+), 35 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index ceadfa5..b3ab19a 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -557,21 +557,17 @@ static inline int hrtimer_hres_active(void) static void hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal) { - int i; - struct hrtimer_clock_base *base = cpu_base->clock_base; + struct hrtimer_clock_base *base; + struct hrtimer *timer; ktime_t expires, expires_next; + unsigned int active_bases; + int i;

expires_next.tv64 = KTIME_MAX;

- for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) { - struct hrtimer *timer; - struct timerqueue_node *next; - - next = timerqueue_getnext(&base->active); - if (!next) - continue; - timer = container_of(next, struct hrtimer, node); - + for_each_active_base(i, base, cpu_base, active_bases) { + timer = container_of(timerqueue_getnext(&base->active), + struct hrtimer, node); expires = hrtimer_get_monoexpires(timer, base); /* * clock_was_set() has changed base->offset so the @@ -1131,23 +1127,19 @@ EXPORT_SYMBOL_GPL(hrtimer_get_remaining); ktime_t hrtimer_get_next_event(void) { struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases); - struct hrtimer_clock_base *base = cpu_base->clock_base; + struct hrtimer_clock_base *base; ktime_t delta, mindelta = { .tv64 = KTIME_MAX }; + struct hrtimer *timer; + unsigned int active_bases; unsigned long flags; int i;

raw_spin_lock_irqsave(&cpu_base->lock, flags);

if (!hrtimer_hres_active()) { - for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) { - struct hrtimer *timer; - struct timerqueue_node *next; - - next = timerqueue_getnext(&base->active); - if (!next) - continue; - - timer = container_of(next, struct hrtimer, node); + for_each_active_base(i, base, cpu_base, active_bases) { + timer = container_of(timerqueue_getnext(&base->active), + struct hrtimer, node); delta.tv64 = hrtimer_get_expires_tv64(timer); delta = ktime_sub(delta, base->get_time()); if (delta.tv64 < mindelta.tv64) @@ -1270,7 +1262,9 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now) void hrtimer_interrupt(struct clock_event_device *dev) { struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases); + struct hrtimer_clock_base *base; ktime_t expires_next, now, entry_time, delta; + unsigned int active_bases; int i, retries = 0;

BUG_ON(!cpu_base->hres_active); @@ -1290,15 +1284,10 @@ retry: */ cpu_base->expires_next.tv64 = KTIME_MAX;

- for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) { - struct hrtimer_clock_base *base; + for_each_active_base(i, base, cpu_base, active_bases) { struct timerqueue_node *node; ktime_t basenow;

- if (!(cpu_base->active_bases & (1 << i))) - continue; - - base = cpu_base->clock_base + i; basenow = ktime_add(now, base->offset);

while ((node = timerqueue_getnext(&base->active))) { @@ -1468,16 +1457,13 @@ void hrtimer_run_queues(void) struct timerqueue_node *node; struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases); struct hrtimer_clock_base *base; + unsigned int active_bases; int index, gettime = 1;

if (hrtimer_hres_active()) return;

- for (index = 0; index < HRTIMER_MAX_CLOCK_BASES; index++) { - base = &cpu_base->clock_base[index]; - if (!timerqueue_getnext(&base->active)) - continue; - + for_each_active_base(index, base, cpu_base, active_bases) { if (gettime) { hrtimer_get_softirq_time(cpu_base); gettime = 0; @@ -1697,6 +1683,8 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base, static void migrate_hrtimers(int scpu) { struct hrtimer_cpu_base *old_base, *new_base; + struct hrtimer_clock_base *clock_base; + unsigned int active_bases; int i;

BUG_ON(cpu_online(scpu)); @@ -1712,9 +1700,9 @@ static void migrate_hrtimers(int scpu) raw_spin_lock(&new_base->lock); raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);

- for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) - migrate_hrtimer_list(&old_base->clock_base[i], - &new_base->clock_base[i]); + for_each_active_base(i, clock_base, old_base, active_bases) + migrate_hrtimer_list(clock_base, + &new_base->clock_base[clock_base->index]);

raw_spin_unlock(&old_base->lock); raw_spin_unlock(&new_base->lock);

We are taking cpu_base->lock for every clock-base in hrtimer_run_queues() and there is nothing in there which prevents us to take this lock only once. Modify code to take lock only once for a cpu_base.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 2d9a7e2..c712960 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1466,8 +1466,8 @@ void hrtimer_run_queues(void) if (cpu_base->active_bases) hrtimer_get_softirq_time(cpu_base);

+ raw_spin_lock(&cpu_base->lock); for_each_active_base(index, base, cpu_base, active_bases) { - raw_spin_lock(&cpu_base->lock); while ((node = timerqueue_getnext(&base->active))) { struct hrtimer *timer;

@@ -1478,8 +1478,8 @@ void hrtimer_run_queues(void)

__run_hrtimer(timer, &base->softirq_time); } - raw_spin_unlock(&cpu_base->lock); } + raw_spin_unlock(&cpu_base->lock); }

/*

clock 'base' can be obtained easily by doing timer->base and remove_timer()/ __remove_timer() never gets anything else than timer->base as its parameter. Which means, these routines doesn't require this parameter. Remove it.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 81e0251..a404436 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -895,10 +895,10 @@ static int enqueue_hrtimer(struct hrtimer *timer, * reprogram to zero. This is useful, when the context does a reprogramming * anyway (e.g. timer interrupt) */ -static void __remove_hrtimer(struct hrtimer *timer, - struct hrtimer_clock_base *base, - unsigned long newstate, int reprogram) +static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate, + int reprogram) { + struct hrtimer_clock_base *base = timer->base; struct timerqueue_node *next_timer;

next_timer = timerqueue_getnext(&base->active); @@ -921,11 +921,8 @@ static void __remove_hrtimer(struct hrtimer *timer, timer->state = newstate; }

-/* - * remove hrtimer, called with base lock held - */ -static inline int -remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base) +/* remove hrtimer, called with base lock held */ +static inline int remove_hrtimer(struct hrtimer *timer) { unsigned long state;

@@ -947,7 +944,7 @@ remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base) * move the timer base in switch_hrtimer_base. */ state = timer->state & HRTIMER_STATE_CALLBACK; - __remove_hrtimer(timer, base, state, base_on_this_cpu(base)); + __remove_hrtimer(timer, state, base_on_this_cpu(timer->base)); return 1; }

@@ -962,7 +959,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, base = lock_hrtimer_base(timer, &flags);

/* Remove an active timer from the queue: */ - ret = remove_hrtimer(timer, base); + ret = remove_hrtimer(timer);

if (mode & HRTIMER_MODE_REL) { tim = ktime_add_safe(tim, base->get_time()); @@ -1064,14 +1061,13 @@ EXPORT_SYMBOL_GPL(hrtimer_start); */ int hrtimer_try_to_cancel(struct hrtimer *timer) { - struct hrtimer_clock_base *base; unsigned long flags; int ret = -1;

- base = lock_hrtimer_base(timer, &flags); + lock_hrtimer_base(timer, &flags);

if (!hrtimer_callback_running(timer)) - ret = remove_hrtimer(timer, base); + ret = remove_hrtimer(timer);

unlock_hrtimer_base(timer, &flags);

@@ -1223,7 +1219,7 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now) WARN_ON(!irqs_disabled());

debug_deactivate(timer); - __remove_hrtimer(timer, base, HRTIMER_STATE_CALLBACK, 0); + __remove_hrtimer(timer, HRTIMER_STATE_CALLBACK, 0); timer_stats_account_hrtimer(timer); fn = timer->function;

@@ -1660,7 +1656,7 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base, * timer could be seen as !active and just vanish away * under us on another CPU */ - __remove_hrtimer(timer, old_base, HRTIMER_STATE_MIGRATE, 0); + __remove_hrtimer(timer, HRTIMER_STATE_MIGRATE, 0); timer->base = new_base; /* * Enqueue the timers on the new cpu. This does not

clock 'base' can be obtained easily by doing timer->base and enqueue_hrtimer() never gets anything else than timer->base as its parameter. And so this routines doesn't require this parameter. Remove it.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index c35dc36..abbf155 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -867,9 +867,10 @@ EXPORT_SYMBOL_GPL(hrtimer_forward); * * Returns 1 when the new timer is the leftmost timer in the tree. */ -static int enqueue_hrtimer(struct hrtimer *timer, - struct hrtimer_clock_base *base) +static int enqueue_hrtimer(struct hrtimer *timer) { + struct hrtimer_clock_base *base = timer->base; + debug_activate(timer);

timerqueue_add(&base->active, &timer->node); @@ -981,7 +982,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,

timer_stats_hrtimer_set_start_info(timer);

- leftmost = enqueue_hrtimer(timer, new_base); + leftmost = enqueue_hrtimer(timer);

/* * Only allow reprogramming if the new base is on this CPU. @@ -1210,8 +1211,7 @@ EXPORT_SYMBOL_GPL(hrtimer_get_res);

static void __run_hrtimer(struct hrtimer *timer, ktime_t *now) { - struct hrtimer_clock_base *base = timer->base; - struct hrtimer_cpu_base *cpu_base = base->cpu_base; + struct hrtimer_cpu_base *cpu_base = timer->base->cpu_base; enum hrtimer_restart (*fn)(struct hrtimer *); int restart;

@@ -1240,7 +1240,7 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now) */ if (restart != HRTIMER_NORESTART) { BUG_ON(timer->state != HRTIMER_STATE_CALLBACK); - enqueue_hrtimer(timer, base); + enqueue_hrtimer(timer); }

WARN_ON_ONCE(!(timer->state & HRTIMER_STATE_CALLBACK)); @@ -1665,7 +1665,7 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base, * sort out already expired timers and reprogram the * event device. */ - enqueue_hrtimer(timer, new_base); + enqueue_hrtimer(timer);

/* Clear the migration state bit */ timer->state &= ~HRTIMER_STATE_MIGRATE;

switch_hrtimer_base() always sets timer->base to the right base and so the caller can obtain it easily. So, this routine doesn't need to return anything.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index fcbabcf..e581bba 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -211,8 +211,7 @@ hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base) /* * Switch the timer base to the current CPU when possible. */ -static inline struct hrtimer_clock_base * -switch_hrtimer_base(struct hrtimer *timer, int pinned) +static inline void switch_hrtimer_base(struct hrtimer *timer, int pinned) { struct hrtimer_clock_base *new_base, *base = timer->base; struct hrtimer_cpu_base *new_cpu_base; @@ -224,7 +223,7 @@ again: new_base = &new_cpu_base->clock_base[base->index];

if (base == new_base) - return base; + return;

/* * We are trying to move timer to new_base. However we can't change @@ -235,7 +234,7 @@ again: * timer is enqueued. */ if (unlikely(hrtimer_callback_running(timer))) - return base; + return;

/* See the comment in lock_timer_base() */ timer->base = NULL; @@ -251,7 +250,6 @@ again: }

timer->base = new_base; - return new_base; }

#else /* CONFIG_SMP */ @@ -266,7 +264,7 @@ lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) return base; }

-# define switch_hrtimer_base(t, p) (t->base) +static inline void switch_hrtimer_base(struct hrtimer *timer, int pinned) {}

#endif /* !CONFIG_SMP */

@@ -949,7 +947,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, unsigned long delta_ns, const enum hrtimer_mode mode, int wakeup) { - struct hrtimer_clock_base *base, *new_base; + struct hrtimer_clock_base *base; unsigned long flags; int ret, leftmost;

@@ -975,7 +973,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, hrtimer_set_expires_range_ns(timer, tim, delta_ns);

/* Switch the timer base, if necessary: */ - new_base = switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED); + switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED);

timer_stats_hrtimer_set_start_info(timer);

@@ -987,7 +985,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, * * XXX send_remote_softirq() ? */ - if (leftmost && base_on_this_cpu(new_base) + if (leftmost && base_on_this_cpu(timer->base) && hrtimer_enqueue_reprogram(timer)) { if (wakeup) { /*

enqueue_hrtimer() routine is called from three places and only one of them effectively uses its return value. Also, by its name enqueue_hrtimer() isn't supposed to return "if the queued timer is the leftmost". So it makes more sense to separate this routine into two parts, first one enqueues a timer and the other one tells if the timer is leftmost or not.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/hrtimer.h | 5 +++++ kernel/hrtimer.c | 10 ++++------ 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index d1836cb..435ac4c 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -263,6 +263,11 @@ static inline ktime_t hrtimer_expires_remaining(const struct hrtimer *timer) return ktime_sub(timer->node.expires, timer->base->get_time()); }

+static inline int hrtimer_is_leftmost(struct hrtimer *timer) +{ + return &timer->node == timer->base->active.next; +} + #ifdef CONFIG_HIGH_RES_TIMERS struct clock_event_device;

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index ea620e5..d62fe32 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -856,7 +856,7 @@ EXPORT_SYMBOL_GPL(hrtimer_forward); * * Returns 1 when the new timer is the leftmost timer in the tree. */ -static int enqueue_hrtimer(struct hrtimer *timer) +static void enqueue_hrtimer(struct hrtimer *timer) { struct hrtimer_clock_base *base = timer->base;

@@ -870,8 +870,6 @@ static int enqueue_hrtimer(struct hrtimer *timer) * state of a possibly running callback. */ timer->state |= HRTIMER_STATE_ENQUEUED; - - return (&timer->node == base->active.next); }

/* @@ -943,7 +941,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, { struct hrtimer_clock_base *base; unsigned long flags; - int ret, leftmost; + int ret;

lock_hrtimer_base(timer, &flags); base = timer->base; @@ -972,7 +970,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,

timer_stats_hrtimer_set_start_info(timer);

- leftmost = enqueue_hrtimer(timer); + enqueue_hrtimer(timer);

/* * Only allow reprogramming if the new base is on this CPU. @@ -980,7 +978,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, * * XXX send_remote_softirq() ? */ - if (leftmost && base_on_this_cpu(timer->base) + if (hrtimer_is_leftmost(timer) && base_on_this_cpu(timer->base) && hrtimer_enqueue_reprogram(timer)) { if (wakeup) { /*

In init_timers() we need to call init_timers_cpu() for boot cpu. For this, currently we are emulating a call to hotplug notifier. Probably this was done initially to get rid of code redundancy. But this sequence always called a single routine, i.e. init_timers_cpu(), and so calling that routine directly would be better. This would get rid of emulating a notifier call, few typecasts and the extra steps we are doing in notifier callback.

So, this patch calls init_timers_cpu() directly from init_timers().

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/timer.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/kernel/timer.c b/kernel/timer.c index 4360edc..d13eb56 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -1666,15 +1666,9 @@ static struct notifier_block timers_nb = {

void __init init_timers(void) { - int err; - /* ensure there are enough low bits for flags in timer->base pointer */ BUILD_BUG_ON(__alignof__(struct tvec_base) & TIMER_FLAG_MASK); - - err = timer_cpu_notify(&timers_nb, (unsigned long)CPU_UP_PREPARE, - (void *)(long)smp_processor_id()); - BUG_ON(err != NOTIFY_OK); - + BUG_ON(init_timers_cpu(smp_processor_id())); init_timer_stats(); register_cpu_notifier(&timers_nb); open_softirq(TIMER_SOFTIRQ, run_timer_softirq);