Hi Thomas,
I have got few more cleanups other than what I posted yesterday: https://lkml.org/lkml/2014/3/26/107
All 30 are pushed here (i.e. Part I and II)
git://git.linaro.org/people/viresh.kumar/linux.git timer-cleanup-for-tglx
Please see if they make sense.
Viresh Kumar (16): hrtimer: move unlock_hrtimer_base() upwards hrtimer: reorder code in __remove_hrtimer() hrtimer: Create hrtimer_get_monoexpires() hrtimer: remove 'base' parameter from remove_timer() and __remove_timer() hrtimer: remove 'base' parameter from switch_hrtimer_base() hrtimer: remove 'base' parameter from enqueue_hrtimer() hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram() hrtimer: make switch_hrtimer_base() return void hrtimer: make lock_hrtimer_base() return void hrtimer: make enqueue_hrtimer() return void hrtimer: don't check if timer is queued in __remove_hrtimer() hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level hrtimer: rewrite remove_hrtimer() to remove extra indentation level hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns() hrtimer: create base_on_this_cpu() hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
include/linux/hrtimer.h | 11 +++ kernel/hrtimer.c | 242 +++++++++++++++++++++--------------------------- 2 files changed, 119 insertions(+), 134 deletions(-)
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 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 e486a14..a710638 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -253,6 +253,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: @@ -801,15 +807,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
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. - no need of local variable 'next_timer' - Validate 'reprogram' and hrtimer_hres_active() first as without these we don't need to check if timer is the next timer to fire.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index a710638..6476152 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -887,25 +887,22 @@ static void __remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, unsigned long newstate, int reprogram) { - struct timerqueue_node *next_timer; if (!(timer->state & HRTIMER_STATE_ENQUEUED)) goto out;
- 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 && hrtimer_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 && hrtimer_hres_active() && + &timer->node == timerqueue_getnext(&base->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 if (!timerqueue_getnext(&base->active)) base->cpu_base->active_bases &= ~(1 << base->index); out:
On 28 March 2014 17:11, Viresh Kumar viresh.kumar@linaro.org wrote:
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
@@ -887,25 +887,22 @@ static void __remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, unsigned long newstate, int reprogram) {
struct timerqueue_node *next_timer; if (!(timer->state & HRTIMER_STATE_ENQUEUED)) goto out;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 && hrtimer_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 && hrtimer_hres_active() &&&timer->node == timerqueue_getnext(&base->active)) {
Last comparison here would always fail as we have already removed the timer. Here is a fixup for that, will include that in my V2:
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 516c67f..189c525 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -886,15 +886,16 @@ static void __remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, unsigned long newstate, int reprogram) { + struct timerqueue_node *next_timer; if (!(timer->state & HRTIMER_STATE_ENQUEUED)) goto out;
+ next_timer = timerqueue_getnext(&base->active); timerqueue_del(&base->active, &timer->node);
#ifdef CONFIG_HIGH_RES_TIMERS /* Reprogram the clock event device. if enabled */ - if (reprogram && hrtimer_hres_active() && - &timer->node == timerqueue_getnext(&base->active)) { + if (reprogram && hrtimer_hres_active() && &timer->node == next_timer) { ktime_t expires;
expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
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 6f524db..377023b 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 6476152..675fe9e 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; @@ -555,7 +555,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 @@ -589,7 +589,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 == timerqueue_getnext(&base->active)) { 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); } @@ -1306,8 +1306,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)
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 | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 675fe9e..343fe99 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -883,10 +883,11 @@ 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; + if (!(timer->state & HRTIMER_STATE_ENQUEUED)) goto out;
@@ -909,11 +910,8 @@ out: 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) { if (hrtimer_is_queued(timer)) { unsigned long state; @@ -929,14 +927,15 @@ remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base) */ debug_deactivate(timer); timer_stats_hrtimer_clear_start_info(timer); - reprogram = base->cpu_base == &__get_cpu_var(hrtimer_bases); + reprogram = + timer->base->cpu_base == &__get_cpu_var(hrtimer_bases); /* * We must preserve the CALLBACK state flag here, * otherwise we could move the timer base in * switch_hrtimer_base. */ state = timer->state & HRTIMER_STATE_CALLBACK; - __remove_hrtimer(timer, base, state, reprogram); + __remove_hrtimer(timer, state, reprogram); return 1; } return 0; @@ -953,7 +952,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);
/* Switch the timer base, if necessary: */ new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED); @@ -1055,14 +1054,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);
@@ -1218,7 +1216,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;
@@ -1663,7 +1661,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 switch_hrtimer_base() 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 | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 343fe99..9561336 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -195,10 +195,9 @@ 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, struct hrtimer_clock_base *base, - int pinned) +switch_hrtimer_base(struct hrtimer *timer, int pinned) { - struct hrtimer_clock_base *new_base; + struct hrtimer_clock_base *new_base, *base = timer->base; struct hrtimer_cpu_base *new_cpu_base; int this_cpu = smp_processor_id(); int cpu = get_nohz_timer_target(pinned); @@ -249,7 +248,7 @@ lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) return base; }
-# define switch_hrtimer_base(t, b, p) (b) +# define switch_hrtimer_base(t, p) (t->base)
#endif /* !CONFIG_SMP */
@@ -955,7 +954,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, ret = remove_hrtimer(timer);
/* Switch the timer base, if necessary: */ - new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED); + new_base = switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED);
if (mode & HRTIMER_MODE_REL) { tim = ktime_add_safe(tim, new_base->get_time());
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 9561336..d6724b5 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -855,9 +855,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); @@ -974,7 +975,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. @@ -1207,8 +1208,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;
@@ -1237,7 +1237,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)); @@ -1670,7 +1670,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;
clock 'base' can be obtained easily by doing timer->base and hrtimer_reprogram() & hrtimer_enqueue_reprogram() never gets anything else than timer->base as its parameter. And so these routines doesn't require this parameter. Remove it.
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 d6724b5..98a73d9 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -584,11 +584,10 @@ hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal) * * Called with interrupts disabled and base->cpu_base.lock held */ -static int hrtimer_reprogram(struct hrtimer *timer, - struct hrtimer_clock_base *base) +static int hrtimer_reprogram(struct hrtimer *timer) { struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases); - ktime_t expires = hrtimer_get_monoexpires(timer, base); + ktime_t expires = hrtimer_get_monoexpires(timer, timer->base); int res;
WARN_ON_ONCE(hrtimer_get_expires_tv64(timer) < 0); @@ -648,10 +647,9 @@ static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base) * check happens. The timer gets enqueued into the rbtree. The reprogramming * and expiry check is done in the hrtimer_interrupt or in the softirq. */ -static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer, - struct hrtimer_clock_base *base) +static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer) { - return base->cpu_base->hres_active && hrtimer_reprogram(timer, base); + return timer->base->cpu_base->hres_active && hrtimer_reprogram(timer); }
static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base) @@ -730,8 +728,7 @@ static 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 int hrtimer_enqueue_reprogram(struct hrtimer *timer, - struct hrtimer_clock_base *base) +static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer) { return 0; } @@ -984,7 +981,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, * XXX send_remote_softirq() ? */ if (leftmost && new_base->cpu_base == &__get_cpu_var(hrtimer_bases) - && hrtimer_enqueue_reprogram(timer, new_base)) { + && hrtimer_enqueue_reprogram(timer)) { if (wakeup) { /* * We need to drop cpu_base->lock to avoid a
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 | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 98a73d9..d98c1ec 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -194,8 +194,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; @@ -217,7 +216,7 @@ again: * the timer is enqueued. */ if (unlikely(hrtimer_callback_running(timer))) - return base; + return;
/* See the comment in lock_timer_base() */ timer->base = NULL; @@ -233,7 +232,6 @@ again: } timer->base = new_base; } - return new_base; }
#else /* CONFIG_SMP */ @@ -248,7 +246,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 */
@@ -952,7 +950,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, ret = remove_hrtimer(timer);
/* Switch the timer base, if necessary: */ - new_base = switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED); + switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED); + new_base = timer->base;
if (mode & HRTIMER_MODE_REL) { tim = ktime_add_safe(tim, new_base->get_time());
lock_hrtimer_base() always returns after taking lock and so timer->base can't change further. So, callers of this routine can simply do timer->base to get the base and so we can make this routine return void.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index d98c1ec..7d85b8f 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -149,9 +149,7 @@ static void hrtimer_get_softirq_time(struct hrtimer_cpu_base *base) * possible to set timer->base = NULL and drop the lock: the timer remains * locked. */ -static -struct hrtimer_clock_base *lock_hrtimer_base(const struct hrtimer *timer, - unsigned long *flags) +static void lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) { struct hrtimer_clock_base *base;
@@ -160,7 +158,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const struct hrtimer *timer, if (likely(base != NULL)) { raw_spin_lock_irqsave(&base->cpu_base->lock, *flags); if (likely(base == timer->base)) - return base; + return; /* The timer has migrated to another CPU: */ raw_spin_unlock_irqrestore(&base->cpu_base->lock, *flags); } @@ -236,14 +234,10 @@ again:
#else /* CONFIG_SMP */
-static inline struct hrtimer_clock_base * +static inline void lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) { - struct hrtimer_clock_base *base = timer->base; - - raw_spin_lock_irqsave(&base->cpu_base->lock, *flags); - - return base; + raw_spin_lock_irqsave(&timer->base->cpu_base->lock, *flags); }
static inline void switch_hrtimer_base(struct hrtimer *timer, int pinned) {} @@ -944,7 +938,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, unsigned long flags; int ret, leftmost;
- base = lock_hrtimer_base(timer, &flags); + lock_hrtimer_base(timer, &flags); + base = timer->base;
/* Remove an active timer from the queue: */ ret = remove_hrtimer(timer);
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 | 11 +++++------ 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 377023b..7ca9fd0 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 7d85b8f..2702185 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -844,7 +844,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;
@@ -858,8 +858,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); }
/* @@ -936,7 +934,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, { struct hrtimer_clock_base *base, *new_base; unsigned long flags; - int ret, leftmost; + int ret;
lock_hrtimer_base(timer, &flags); base = timer->base; @@ -966,7 +964,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. @@ -974,7 +972,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, * * XXX send_remote_softirq() ? */ - if (leftmost && new_base->cpu_base == &__get_cpu_var(hrtimer_bases) + if (hrtimer_is_leftmost(timer) && + new_base->cpu_base == &__get_cpu_var(hrtimer_bases) && hrtimer_enqueue_reprogram(timer)) { if (wakeup) { /*
__remove_hrtimer() is called from three locations: remove_hrtimer(), __run_hrtimer() and migrate_hrtimer_list(). And all these guarantee that timer was queued earlier. And so there is no need to check if the timer is queued or not in __remove_hrtimer().
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 2702185..04f8e44 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -875,9 +875,6 @@ static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate, { struct hrtimer_clock_base *base = timer->base;
- if (!(timer->state & HRTIMER_STATE_ENQUEUED)) - goto out; - timerqueue_del(&base->active, &timer->node);
#ifdef CONFIG_HIGH_RES_TIMERS @@ -891,10 +888,9 @@ static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate, hrtimer_force_reprogram(base->cpu_base, 1); } #endif + timer->state = newstate; if (!timerqueue_getnext(&base->active)) base->cpu_base->active_bases &= ~(1 << base->index); -out: - timer->state = newstate; }
/* remove hrtimer, called with base lock held */
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 | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 04f8e44..30efa1c 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -203,33 +203,33 @@ 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; + if (base == new_base) + return;
- /* 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); + /* + * 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;
- 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; + /* 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; }
#else /* CONFIG_SMP */
Complete bottom part of remove_hrtimer() 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 | 47 ++++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 30efa1c..0ae0fbf 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -896,32 +896,29 @@ static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate, /* remove hrtimer, called with base lock held */ static inline int remove_hrtimer(struct hrtimer *timer) { - if (hrtimer_is_queued(timer)) { - unsigned long state; - int reprogram; + unsigned long state;
- /* - * Remove the timer and force reprogramming when high - * resolution mode is active and the timer is on the current - * CPU. If we remove a timer on another CPU, reprogramming is - * skipped. The interrupt event on this CPU is fired and - * reprogramming happens in the interrupt handler. This is a - * rare case and less expensive than a smp call. - */ - debug_deactivate(timer); - timer_stats_hrtimer_clear_start_info(timer); - reprogram = - timer->base->cpu_base == &__get_cpu_var(hrtimer_bases); - /* - * We must preserve the CALLBACK state flag here, - * otherwise we could move the timer base in - * switch_hrtimer_base. - */ - state = timer->state & HRTIMER_STATE_CALLBACK; - __remove_hrtimer(timer, state, reprogram); - return 1; - } - return 0; + if (!hrtimer_is_queued(timer)) + return 0; + + /* + * Remove the timer and force reprogramming when high resolution mode is + * active and the timer is on the current CPU. If we remove a timer on + * another CPU, reprogramming is skipped. The interrupt event on this + * CPU is fired and reprogramming happens in the interrupt handler. This + * is a rare case and less expensive than a smp call. + */ + debug_deactivate(timer); + timer_stats_hrtimer_clear_start_info(timer); + + /* + * We must preserve the CALLBACK state flag here, otherwise we could + * move the timer base in switch_hrtimer_base. + */ + state = timer->state & HRTIMER_STATE_CALLBACK; + __remove_hrtimer(timer, state, + timer->base->cpu_base == &__get_cpu_var(hrtimer_bases)); + return 1; }
int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
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.
Cc: Ingo Molnar mingo@redhat.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- Also commit log of above commit has this: "This will go away with the GTOD framework". So, should we get this removed?
kernel/hrtimer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 0ae0fbf..5d77f36 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -925,12 +925,11 @@ 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 *new_base; unsigned long flags; int ret;
lock_hrtimer_base(timer, &flags); - base = timer->base;
/* Remove an active timer from the queue: */ ret = remove_hrtimer(timer); @@ -949,7 +948,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 }
We had this code at two places to find if a clock base belongs to current CPU: base->cpu_base == &__get_cpu_var(hrtimer_bases)
Better to get a inlined routine for that.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 5d77f36..83e5f2d 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -107,6 +107,10 @@ static inline int hrtimer_clockid_to_base(clockid_t clock_id) return hrtimer_clock_to_base_table[clock_id]; }
+static inline bool base_on_this_cpu(struct hrtimer_clock_base *base) +{ + return base->cpu_base == &__get_cpu_var(hrtimer_bases); +}
/* * Get the coarse grained time at the softirq based on xtime and @@ -916,8 +920,7 @@ static inline int remove_hrtimer(struct hrtimer *timer) * move the timer base in switch_hrtimer_base. */ state = timer->state & HRTIMER_STATE_CALLBACK; - __remove_hrtimer(timer, state, - timer->base->cpu_base == &__get_cpu_var(hrtimer_bases)); + __remove_hrtimer(timer, state, base_on_this_cpu(timer->base)); return 1; }
@@ -964,9 +967,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, * * XXX send_remote_softirq() ? */ - if (hrtimer_is_leftmost(timer) && - new_base->cpu_base == &__get_cpu_var(hrtimer_bases) - && hrtimer_enqueue_reprogram(timer)) { + if (hrtimer_is_leftmost(timer) && base_on_this_cpu(new_base) && + hrtimer_enqueue_reprogram(timer)) { if (wakeup) { /* * We need to drop cpu_base->lock to avoid a
use base->hres_active directly when we already have a pointer to base instead of calling hrtimer_hres_active(). As that would lead to:
__this_cpu_read(hrtimer_bases.hres_active)
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- kernel/hrtimer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 83e5f2d..f379821 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -883,7 +883,7 @@ static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate,
#ifdef CONFIG_HIGH_RES_TIMERS /* Reprogram the clock event device. if enabled */ - if (reprogram && hrtimer_hres_active() && + if (reprogram && base->cpu_base->hres_active && &timer->node == timerqueue_getnext(&base->active)) { ktime_t expires;
@@ -1107,7 +1107,7 @@ ktime_t hrtimer_get_next_event(void)
raw_spin_lock_irqsave(&cpu_base->lock, flags);
- if (!hrtimer_hres_active()) { + if (!cpu_base->hres_active) { for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) { struct hrtimer *timer; struct timerqueue_node *next; @@ -1437,7 +1437,7 @@ void hrtimer_run_queues(void) struct hrtimer_clock_base *base; int index, gettime = 1;
- if (hrtimer_hres_active()) + if (cpu_base->hres_active) return;
for (index = 0; index < HRTIMER_MAX_CLOCK_BASES; index++) {
On 28 March 2014 17:11, Viresh Kumar viresh.kumar@linaro.org wrote:
@@ -1107,7 +1107,7 @@ ktime_t hrtimer_get_next_event(void)
raw_spin_lock_irqsave(&cpu_base->lock, flags);
if (!hrtimer_hres_active()) {
if (!cpu_base->hres_active) { for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) { struct hrtimer *timer; struct timerqueue_node *next;@@ -1437,7 +1437,7 @@ void hrtimer_run_queues(void) struct hrtimer_clock_base *base; int index, gettime = 1;
if (hrtimer_hres_active())
if (cpu_base->hres_active) return; for (index = 0; index < HRTIMER_MAX_CLOCK_BASES; index++) {
These two changes are broken.. Would remove these and resend.. My tree is fixed though..
use base->hres_active directly when we already have a pointer to base instead of calling hrtimer_hres_active(). As that would lead to:
__this_cpu_read(hrtimer_bases.hres_active)
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- V1->V2: Removed few entries as hres_active isn't available if CONFIG_HIGH_RES_TIMERS isn't enabled.
kernel/hrtimer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 83e5f2d..9e18145 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -883,7 +883,7 @@ static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate,
#ifdef CONFIG_HIGH_RES_TIMERS /* Reprogram the clock event device. if enabled */ - if (reprogram && hrtimer_hres_active() && + if (reprogram && base->cpu_base->hres_active && &timer->node == timerqueue_getnext(&base->active)) { ktime_t expires;
On Fri, 28 Mar 2014, Viresh Kumar wrote:
I have got few more cleanups other than what I posted yesterday: https://lkml.org/lkml/2014/3/26/107
And those cleanups make the compiler generate worse code at least on x86_64:
text data bss dec hex filename 7475 554 0 8029 1f5d kernel/hrtimer.o 7706 554 0 8260 2044 kernel/hrtimer.o
So just removing parameters and return values because you can get the same information from a datastructure is not necessarily a good thing.
Thanks,
tglx
On 31 March 2014 19:08, Thomas Gleixner tglx@linutronix.de wrote:
And those cleanups make the compiler generate worse code at least on x86_64:
text data bss dec hex filename 7475 554 0 8029 1f5d kernel/hrtimer.o 7706 554 0 8260 2044 kernel/hrtimer.o
So just removing parameters and return values because you can get the same information from a datastructure is not necessarily a good thing.
Hmm.. Nice.
Okay, I will have another look at patches and do this kind of investigation before sending it next time :) Its been fun going through these frameworks.
How do you want to proceed now? I mean, you will take the other patches (which don't play with function parameters) as is or want me to send a single unified patchset with all the pending patches that I have?
Thanks for your support :)
-- viresh
On Mon, 31 Mar 2014, Viresh Kumar wrote:
On 31 March 2014 19:08, Thomas Gleixner tglx@linutronix.de wrote:
And those cleanups make the compiler generate worse code at least on x86_64:
text data bss dec hex filename 7475 554 0 8029 1f5d kernel/hrtimer.o 7706 554 0 8260 2044 kernel/hrtimer.o
So just removing parameters and return values because you can get the same information from a datastructure is not necessarily a good thing.
Hmm.. Nice.
Okay, I will have another look at patches and do this kind of investigation before sending it next time :) Its been fun going through these frameworks.
How do you want to proceed now? I mean, you will take the other patches (which don't play with function parameters) as is or want me to send a single unified patchset with all the pending patches that I have?
I still want to go through the lot and review them, but I wont take anything before the end of the merge window.
Thanks,
tglx
On 31 March 2014 19:08, Thomas Gleixner tglx@linutronix.de wrote:
And those cleanups make the compiler generate worse code at least on x86_64:
text data bss dec hex filename 7475 554 0 8029 1f5d kernel/hrtimer.o 7706 554 0 8260 2044 kernel/hrtimer.o
So just removing parameters and return values because you can get the same information from a datastructure is not necessarily a good thing.
So here is the patch by patch analysis (With x86_64_defconfig):
Initial details: text data bss dec hex filename 7989 408 0 8397 20cd ../bx86/kernel/hrtimer.o
73a6cd8 hrtimer: replace 'tab' with 'space' after comma ',' 04223a8 hrtimer: Coalesce format fragments in printk() 52fac3f hrtimer: call hrtimer_set_expires_range() from hrtimer_set_expires_range_ns() 535a552 hrtimer: use base->index instead of basenum in switch_hrtimer_base()
7989 408 0 8397 20cd ../bx86/kernel/hrtimer.o
f3a2cdd hrtimer: no need to rewrite '1' to hrtimer_hres_enabled
7974 408 0 8382 20be ../bx86/kernel/hrtimer.o
This one actually made a smaller :)
479d66f hrtimer: don't rewrite same value to expires_next in hrtimer_force_reprogram()
7990 408 0 8398 20ce ../bx86/kernel/hrtimer.o
0e134df hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
8006 408 0 8414 20de ../bx86/kernel/hrtimer.o
So, reading from a per-cpu variable is efficient as compared to base->hres_active. In that case this patch can be dropped :)
0030ddd hrtimer: remove dummy definition of hrtimer_force_reprogram()
8006 408 0 8414 20de ../bx86/kernel/hrtimer.o
4a0c909 hrtimer: don't check state of base->hres_active in hrtimer_switch_to_hres()
8022 408 0 8430 20ee ../bx86/kernel/hrtimer.o
This one actually removed some code, how come it is bigger :(
213bc01 hrtimer: remove clock_was_set_delayed() from hrtimer.h
8006 408 0 8414 20de ../bx86/kernel/hrtimer.o
Though this one made it shorter, we probably need to drop it as clock_was_set_delayed() is now used in kernel/time/timekeeping.c
3defaf8 hrtimer: don't emulate notifier call to initialize timer base
8071 408 0 8479 211f ../bx86/kernel/hrtimer.o
I thought this will surely generate a smaller text segment :(
e438576 hrtimer: use ffs() to iterate over valid bits from active_bases
8151 408 0 8559 216f ../bx86/kernel/hrtimer.o
1ab3d15 timer: simplify CPU_UP_PREPARE notifier code path c5111c9 timer: don't emulate notifier call to initialize timer base a576aa9 hrtimer: move unlock_hrtimer_base() upwards 9ea8619 hrtimer: reorder code in __remove_hrtimer() 2619470 hrtimer: Create hrtimer_get_monoexpires()
8135 408 0 8543 215f ../bx86/kernel/hrtimer.o
b64d746 hrtimer: remove 'base' parameter from remove_timer() and __remove_timer()
8183 408 0 8591 218f ../bx86/kernel/hrtimer.o
ced918e2 hrtimer: remove 'base' parameter from switch_hrtimer_base()
8135 408 0 8543 215f ../bx86/kernel/hrtimer.o
f046106b hrtimer: remove 'base' parameter from enqueue_hrtimer()
8119 408 0 8527 214f ../bx86/kernel/hrtimer.o
b102337 hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram()
8135 408 0 8543 215f ../bx86/kernel/hrtimer.o
So, these are the four patches that have removed parameters from routines. and it started with 8135 and ended up with 8135 :)
d43b5d1 hrtimer: make switch_hrtimer_base() return void
8199 408 0 8607 219f ../bx86/kernel/hrtimer.o
cd08cd9 hrtimer: make lock_hrtimer_base() return void
8199 408 0 8607 219f ../bx86/kernel/hrtimer.o
74a3a18 hrtimer: make enqueue_hrtimer() return void
8167 408 0 8575 217f ../bx86/kernel/hrtimer.o
b5cb057 hrtimer: don't check if timer is queued in __remove_hrtimer()
8135 408 0 8543 215f ../bx86/kernel/hrtimer.o
b56d3fc hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level fccd30d hrtimer: rewrite remove_hrtimer() to remove extra indentation level d4a6c58 hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns() 24bf5c0 hrtimer: create base_on_this_cpu() 3f690eb hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
8135 408 0 8543 215f ../bx86/kernel/hrtimer.o
And finally we ended up at 8135 :)
On 1 April 2014 12:03, Viresh Kumar viresh.kumar@linaro.org wrote:
So here is the patch by patch analysis (With x86_64_defconfig):
Initial details: text data bss dec hex filename 7989 408 0 8397 20cd ../bx86/kernel/hrtimer.o
73a6cd8 hrtimer: replace 'tab' with 'space' after comma ',' 04223a8 hrtimer: Coalesce format fragments in printk() 52fac3f hrtimer: call hrtimer_set_expires_range() from hrtimer_set_expires_range_ns() 535a552 hrtimer: use base->index instead of basenum in switch_hrtimer_base()
7989 408 0 8397 20cd ../bx86/kernel/hrtimer.o
f3a2cdd hrtimer: no need to rewrite '1' to hrtimer_hres_enabled
7974 408 0 8382 20be ../bx86/kernel/hrtimer.o
This one actually made a smaller :)
479d66f hrtimer: don't rewrite same value to expires_next in hrtimer_force_reprogram()
7990 408 0 8398 20ce ../bx86/kernel/hrtimer.o
0e134df hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
8006 408 0 8414 20de ../bx86/kernel/hrtimer.o
So, reading from a per-cpu variable is efficient as compared to base->hres_active. In that case this patch can be dropped :)
Its a bit awkward now. When I keep this patch at this location it exactly generates above numbers, i.e. 7990 -> 8006.
But if I revert this patch over top of all 30 commits I have, it doesn't make a difference. Even applying it again over the revert doesn't make a difference.
There are no other changes to the concerned routine, retrigger_next_event() in my commits..
linaro-kernel@lists.linaro.org
-
Thomas Gleixner -
Viresh Kumar