* [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II
@ 2014-03-28 11:41 Viresh Kumar
2014-03-28 11:41 ` [PATCH 01/16] hrtimer: move unlock_hrtimer_base() upwards Viresh Kumar
` (16 more replies)
0 siblings, 17 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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(-)
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 01/16] hrtimer: move unlock_hrtimer_base() upwards
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 02/16] hrtimer: reorder code in __remove_hrtimer() Viresh Kumar
` (15 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 02/16] hrtimer: reorder code in __remove_hrtimer()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
2014-03-28 11:41 ` [PATCH 01/16] hrtimer: move unlock_hrtimer_base() upwards Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-04-02 5:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 03/16] hrtimer: Create hrtimer_get_monoexpires() Viresh Kumar
` (14 subsequent siblings)
16 siblings, 1 reply; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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:
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 03/16] hrtimer: Create hrtimer_get_monoexpires()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
2014-03-28 11:41 ` [PATCH 01/16] hrtimer: move unlock_hrtimer_base() upwards Viresh Kumar
2014-03-28 11:41 ` [PATCH 02/16] hrtimer: reorder code in __remove_hrtimer() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 04/16] hrtimer: remove 'base' parameter from remove_timer() and __remove_timer() Viresh Kumar
` (13 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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)
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 04/16] hrtimer: remove 'base' parameter from remove_timer() and __remove_timer()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (2 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 03/16] hrtimer: Create hrtimer_get_monoexpires() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 05/16] hrtimer: remove 'base' parameter from switch_hrtimer_base() Viresh Kumar
` (12 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 05/16] hrtimer: remove 'base' parameter from switch_hrtimer_base()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (3 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 04/16] hrtimer: remove 'base' parameter from remove_timer() and __remove_timer() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 06/16] hrtimer: remove 'base' parameter from enqueue_hrtimer() Viresh Kumar
` (11 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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());
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 06/16] hrtimer: remove 'base' parameter from enqueue_hrtimer()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (4 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 05/16] hrtimer: remove 'base' parameter from switch_hrtimer_base() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 07/16] hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram() Viresh Kumar
` (10 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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;
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 07/16] hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (5 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 06/16] hrtimer: remove 'base' parameter from enqueue_hrtimer() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 08/16] hrtimer: make switch_hrtimer_base() return void Viresh Kumar
` (9 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 08/16] hrtimer: make switch_hrtimer_base() return void
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (6 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 07/16] hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 09/16] hrtimer: make lock_hrtimer_base() " Viresh Kumar
` (8 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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());
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 09/16] hrtimer: make lock_hrtimer_base() return void
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (7 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 08/16] hrtimer: make switch_hrtimer_base() return void Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 10/16] hrtimer: make enqueue_hrtimer() " Viresh Kumar
` (7 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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);
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 10/16] hrtimer: make enqueue_hrtimer() return void
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (8 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 09/16] hrtimer: make lock_hrtimer_base() " Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 11/16] hrtimer: don't check if timer is queued in __remove_hrtimer() Viresh Kumar
` (6 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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) {
/*
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 11/16] hrtimer: don't check if timer is queued in __remove_hrtimer()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (9 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 10/16] hrtimer: make enqueue_hrtimer() " Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 12/16] hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level Viresh Kumar
` (5 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
__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 */
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 12/16] hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (10 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 11/16] hrtimer: don't check if timer is queued in __remove_hrtimer() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 13/16] hrtimer: rewrite remove_hrtimer() " Viresh Kumar
` (4 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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 */
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 13/16] hrtimer: rewrite remove_hrtimer() to remove extra indentation level
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (11 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 12/16] hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 14/16] hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns() Viresh Kumar
` (3 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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,
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 14/16] hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (12 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 13/16] hrtimer: rewrite remove_hrtimer() " Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 15/16] hrtimer: create base_on_this_cpu() Viresh Kumar
` (2 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar, Ingo Molnar
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
}
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 15/16] hrtimer: create base_on_this_cpu()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (13 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 14/16] hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active() Viresh Kumar
2014-03-31 13:38 ` [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Thomas Gleixner
16 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (14 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 15/16] hrtimer: create base_on_this_cpu() Viresh Kumar
@ 2014-03-28 11:41 ` Viresh Kumar
2014-03-28 12:24 ` Viresh Kumar
2014-03-28 12:26 ` [PATCH V2 " Viresh Kumar
2014-03-31 13:38 ` [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Thomas Gleixner
16 siblings, 2 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 11:41 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
Arvind.Chauhan, Viresh Kumar
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++) {
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
2014-03-28 11:41 ` [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active() Viresh Kumar
@ 2014-03-28 12:24 ` Viresh Kumar
2014-03-28 12:26 ` [PATCH V2 " Viresh Kumar
1 sibling, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 12:24 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Lists linaro-kernel, Linux Kernel Mailing List,
Frédéric Weisbecker, Linaro Networking, Arvind Chauhan,
Viresh Kumar
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..
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V2 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active()
2014-03-28 11:41 ` [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active() Viresh Kumar
2014-03-28 12:24 ` Viresh Kumar
@ 2014-03-28 12:26 ` Viresh Kumar
1 sibling, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-03-28 12:26 UTC (permalink / raw)
To: tglx
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking,
arvind.chauhan, Viresh Kumar
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;
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
` (15 preceding siblings ...)
2014-03-28 11:41 ` [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active() Viresh Kumar
@ 2014-03-31 13:38 ` Thomas Gleixner
2014-03-31 14:31 ` Viresh Kumar
2014-04-01 6:33 ` Viresh Kumar
16 siblings, 2 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-03-31 13:38 UTC (permalink / raw)
To: Viresh Kumar
Cc: linaro-kernel, linux-kernel, fweisbec, linaro-networking, Arvind.Chauhan
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II
2014-03-31 13:38 ` [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Thomas Gleixner
@ 2014-03-31 14:31 ` Viresh Kumar
2014-03-31 15:51 ` Thomas Gleixner
2014-04-01 6:33 ` Viresh Kumar
1 sibling, 1 reply; 25+ messages in thread
From: Viresh Kumar @ 2014-03-31 14:31 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Lists linaro-kernel, Linux Kernel Mailing List,
Frédéric Weisbecker, Linaro Networking, Arvind Chauhan
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II
2014-03-31 14:31 ` Viresh Kumar
@ 2014-03-31 15:51 ` Thomas Gleixner
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-03-31 15:51 UTC (permalink / raw)
To: Viresh Kumar
Cc: Lists linaro-kernel, Linux Kernel Mailing List,
Frédéric Weisbecker, Linaro Networking, Arvind Chauhan
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II
2014-03-31 13:38 ` [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Thomas Gleixner
2014-03-31 14:31 ` Viresh Kumar
@ 2014-04-01 6:33 ` Viresh Kumar
2014-04-01 7:02 ` Viresh Kumar
1 sibling, 1 reply; 25+ messages in thread
From: Viresh Kumar @ 2014-04-01 6:33 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Lists linaro-kernel, Linux Kernel Mailing List,
Frédéric Weisbecker, Linaro Networking, Arvind Chauhan
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 :)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II
2014-04-01 6:33 ` Viresh Kumar
@ 2014-04-01 7:02 ` Viresh Kumar
0 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-04-01 7:02 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Lists linaro-kernel, Linux Kernel Mailing List,
Frédéric Weisbecker, Linaro Networking, Arvind Chauhan
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..
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 02/16] hrtimer: reorder code in __remove_hrtimer()
2014-03-28 11:41 ` [PATCH 02/16] hrtimer: reorder code in __remove_hrtimer() Viresh Kumar
@ 2014-04-02 5:41 ` Viresh Kumar
0 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2014-04-02 5:41 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Lists linaro-kernel, Linux Kernel Mailing List,
Frédéric Weisbecker, Linaro Networking, Arvind Chauhan,
Viresh Kumar
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);
^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2014-04-02 5:41 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-28 11:41 [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Viresh Kumar
2014-03-28 11:41 ` [PATCH 01/16] hrtimer: move unlock_hrtimer_base() upwards Viresh Kumar
2014-03-28 11:41 ` [PATCH 02/16] hrtimer: reorder code in __remove_hrtimer() Viresh Kumar
2014-04-02 5:41 ` Viresh Kumar
2014-03-28 11:41 ` [PATCH 03/16] hrtimer: Create hrtimer_get_monoexpires() Viresh Kumar
2014-03-28 11:41 ` [PATCH 04/16] hrtimer: remove 'base' parameter from remove_timer() and __remove_timer() Viresh Kumar
2014-03-28 11:41 ` [PATCH 05/16] hrtimer: remove 'base' parameter from switch_hrtimer_base() Viresh Kumar
2014-03-28 11:41 ` [PATCH 06/16] hrtimer: remove 'base' parameter from enqueue_hrtimer() Viresh Kumar
2014-03-28 11:41 ` [PATCH 07/16] hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram() Viresh Kumar
2014-03-28 11:41 ` [PATCH 08/16] hrtimer: make switch_hrtimer_base() return void Viresh Kumar
2014-03-28 11:41 ` [PATCH 09/16] hrtimer: make lock_hrtimer_base() " Viresh Kumar
2014-03-28 11:41 ` [PATCH 10/16] hrtimer: make enqueue_hrtimer() " Viresh Kumar
2014-03-28 11:41 ` [PATCH 11/16] hrtimer: don't check if timer is queued in __remove_hrtimer() Viresh Kumar
2014-03-28 11:41 ` [PATCH 12/16] hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level Viresh Kumar
2014-03-28 11:41 ` [PATCH 13/16] hrtimer: rewrite remove_hrtimer() " Viresh Kumar
2014-03-28 11:41 ` [PATCH 14/16] hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns() Viresh Kumar
2014-03-28 11:41 ` [PATCH 15/16] hrtimer: create base_on_this_cpu() Viresh Kumar
2014-03-28 11:41 ` [PATCH 16/16] hrtimer: use base->hres_active directly instead of hrtimer_hres_active() Viresh Kumar
2014-03-28 12:24 ` Viresh Kumar
2014-03-28 12:26 ` [PATCH V2 " Viresh Kumar
2014-03-31 13:38 ` [PATCH 00/16] timers/hrtimers: Minor cleanups: Part II Thomas Gleixner
2014-03-31 14:31 ` Viresh Kumar
2014-03-31 15:51 ` Thomas Gleixner
2014-04-01 6:33 ` Viresh Kumar
2014-04-01 7:02 ` Viresh Kumar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.