[PATCH V2 00/36] timers/hrtimers: Cleanups & Fixes

[PATCH V2 00/36] timers/hrtimers: Cleanups & Fixes

[Viresh Kumar]

Hi Thomas,

I know you are not going to look at these before end of this merge window and
you wanted to have a look at V1 before me posting these. But I am reposting them
now due to these reasons:
- Need to resend my cpu isolation (cpuset.quiesce) patches which are based of
  these
- Few patches are dropped/merged/fixed/updated and so all the patches from V1
  wouldn't have made sense
- There were some new patches as well which I wanted to send

These have gone through fair bit of testing via kbuild system maintained by
Fengguang Wu.


These are some minor cleanups and potential bug fixes in there. These are based
of tip/timers-core-for-linus ..

V1 of most of these patches (~28) were posted here:
https://lkml.org/lkml/2014/3/26/107
https://lkml.org/lkml/2014/3/28/148

V1->V2:
- few new patches:
  - patches around for_each_active_base()
  - hrtimer: call switch_hrtimer_base() after setting new expiry time
  - Some other minor cleanups
- few patches are dropped
- few are merged together as they covered same stuff
- rebased all patches and moved the patches removing parameters or return values
  at the bottom, so that others can be applied easily. Though as per my last
  mail, it doesn't look like they are making the 'text' segments any bigger.

Viresh Kumar (36):
  hrtimer: replace 'tab' with 'space' after 'comma'
  hrtimer: Fix comment mistake over hrtimer_force_reprogram()
  hrtimer: fix routine names in comments
  hrtimer: remove {} around a single liner 'for' loop in
    migrate_hrtimers()
  hrtimer: Coalesce format fragments in printk()
  hrtimer: remove dummy definition of hrtimer_force_reprogram()
  hrtimer: replace sizeof(struct hrtimer) with sizeof(*timer)
  hrtimer: move unlock_hrtimer_base() upwards
  hrtimer: call hrtimer_set_expires_range() from
    hrtimer_set_expires_range_ns()
  hrtimer: use base->index instead of basenum in switch_hrtimer_base()
  hrtimer: no need to rewrite '1' to hrtimer_hres_enabled
  hrtimer: use base->hres_active directly instead of
    hrtimer_hres_active()
  hrtimer: don't check state of base->hres_active in
    hrtimer_switch_to_hres()
  hrtimer: reorder code in __remove_hrtimer()
  hrtimer: don't emulate notifier call to initialize timer base
  hrtimer: Create hrtimer_get_monoexpires()
  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: clear active_bases as soon as the timer is removed
  hrtimer: create for_each_active_base()
  hrtimer: Use for_each_active_base() to iterate over active clock
    bases
  hrtimer: call hrtimer_get_softirq_time() only if
    cpu_base->active_bases is set
  hrtimer: take lock only once for a cpu_base in hrtimer_run_queues()
  hrtimer: call switch_hrtimer_base() after setting new expiry time
  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
  timer: simplify CPU_UP_PREPARE notifier code path
  timer: don't emulate notifier call to initialize timer base

 include/linux/hrtimer.h |  14 +-
 kernel/hrtimer.c        | 365 ++++++++++++++++++++++--------------------------
 kernel/timer.c          |  12 +-
 3 files changed, 179 insertions(+), 212 deletions(-)

-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 01/36] hrtimer: replace 'tab' with 'space' after 'comma'

[Viresh Kumar]

Currently we have a 'tab' here instead of 'space' after 'comma'. Replace it with
space.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index d55092c..a1120a0 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -128,7 +128,7 @@ static void hrtimer_get_softirq_time(struct hrtimer_cpu_base *base)
 	base->clock_base[HRTIMER_BASE_MONOTONIC].softirq_time = mono;
 	base->clock_base[HRTIMER_BASE_BOOTTIME].softirq_time = boot;
 	base->clock_base[HRTIMER_BASE_TAI].softirq_time =
-				ktime_add(xtim,	ktime_set(tai_offset, 0));
+				ktime_add(xtim, ktime_set(tai_offset, 0));
 }
 
 /*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 02/36] hrtimer: Fix comment mistake over hrtimer_force_reprogram()

[Viresh Kumar]

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index a1120a0..3c05140 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -530,8 +530,7 @@ static inline int hrtimer_hres_active(void)
 }
 
 /*
- * Reprogram the event source with checking both queues for the
- * next event
+ * Reprogram the event source with checking all queues for the next event.
  * Called with interrupts disabled and base->lock held
  */
 static void
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 03/36] hrtimer: fix routine names in comments

[Viresh Kumar]

As majority of this code came from existing kernel/timer.c, few comments still
had name of routines from timers framework. Fix them.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 3c05140..c87e896 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -142,7 +142,7 @@ static void hrtimer_get_softirq_time(struct hrtimer_cpu_base *base)
  * means that all timers which are tied to this base via timer->base are
  * locked, and the base itself is locked too.
  *
- * So __run_timers/migrate_timers can safely modify all timers which could
+ * So __run_hrtimer/migrate_hrtimers can safely modify all timers which could
  * be found on the lists/queues.
  *
  * When the timer's base is locked, and the timer removed from list, it is
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 04/36] hrtimer: remove {} around a single liner 'for' loop in migrate_hrtimers()

[Viresh Kumar]

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index c87e896..514b53a 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1712,10 +1712,9 @@ static void migrate_hrtimers(int scpu)
 	raw_spin_lock(&new_base->lock);
 	raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);
 
-	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) {
+	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++)
 		migrate_hrtimer_list(&old_base->clock_base[i],
 				     &new_base->clock_base[i]);
-	}
 
 	raw_spin_unlock(&old_base->lock);
 	raw_spin_unlock(&new_base->lock);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 05/36] hrtimer: Coalesce format fragments in printk()

[Viresh Kumar]

Breaking format fragments into multiple lines hits readability of code. Even if
it goes over 80 column width, its better to keep them together.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 514b53a..4843238 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -694,8 +694,8 @@ static int hrtimer_switch_to_hres(void)
 
 	if (tick_init_highres()) {
 		local_irq_restore(flags);
-		printk(KERN_WARNING "Could not switch to high resolution "
-				    "mode on CPU %d\n", cpu);
+		printk(KERN_WARNING "Could not switch to high resolution mode on CPU %d\n",
+			cpu);
 		return 0;
 	}
 	base->hres_active = 1;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 06/36] hrtimer: remove dummy definition of hrtimer_force_reprogram()

[Viresh Kumar]

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

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

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

[PATCH V2 07/36] hrtimer: replace sizeof(struct hrtimer) with sizeof(*timer)

[Viresh Kumar]

Linux coding guidelines says:

The preferred form for passing a size of a struct is the following:
	p = kmalloc(sizeof(*p), ...);

But __hrtimer_init() wasn't following that. Fix it.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index f6b1968..db580ab 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1166,7 +1166,7 @@ static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 	struct hrtimer_cpu_base *cpu_base;
 	int base;
 
-	memset(timer, 0, sizeof(struct hrtimer));
+	memset(timer, 0, sizeof(*timer));
 
 	cpu_base = &__raw_get_cpu_var(hrtimer_bases);
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 08/36] hrtimer: move unlock_hrtimer_base() upwards

[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 (Atleast in the current code) and so we can
move unlock_hrtimer_base() upwards, i.e. closer to its counterparts. This
improves readability of the code.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index db580ab..3b29023 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -254,6 +254,12 @@ lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags)
 
 #endif	/* !CONFIG_SMP */
 
+static inline
+void unlock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags)
+{
+	raw_spin_unlock_irqrestore(&timer->base->cpu_base->lock, *flags);
+}
+
 /*
  * Functions for the union type storage format of ktime_t which are
  * too large for inlining:
@@ -805,15 +811,6 @@ static inline void timer_stats_account_hrtimer(struct hrtimer *timer)
 #endif
 }
 
-/*
- * Counterpart to lock_hrtimer_base above:
- */
-static inline
-void unlock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags)
-{
-	raw_spin_unlock_irqrestore(&timer->base->cpu_base->lock, *flags);
-}
-
 /**
  * hrtimer_forward - forward the timer expiry
  * @timer:	hrtimer to forward
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 09/36] hrtimer: call hrtimer_set_expires_range() from hrtimer_set_expires_range_ns()

[Viresh Kumar]

hrtimer_set_expires_range() and hrtimer_set_expires_range_ns() have almost same
implementations and so we can actually call hrtimer_set_expires_range() from
hrtimer_set_expires_range_ns() internally instead of duplicating code.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/hrtimer.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index e7a8d3f..17c08ca 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -207,8 +207,7 @@ static inline void hrtimer_set_expires_range(struct hrtimer *timer, ktime_t time
 
 static inline void hrtimer_set_expires_range_ns(struct hrtimer *timer, ktime_t time, unsigned long delta)
 {
-	timer->_softexpires = time;
-	timer->node.expires = ktime_add_safe(time, ns_to_ktime(delta));
+	hrtimer_set_expires_range(timer, time, ns_to_ktime(delta));
 }
 
 static inline void hrtimer_set_expires_tv64(struct hrtimer *timer, s64 tv64)
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 10/36] hrtimer: use base->index instead of basenum in switch_hrtimer_base()

[Viresh Kumar]

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 3b29023..04e4c77 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -202,11 +202,10 @@ switch_hrtimer_base(struct hrtimer *timer, struct hrtimer_clock_base *base,
 	struct hrtimer_cpu_base *new_cpu_base;
 	int this_cpu = smp_processor_id();
 	int cpu = get_nohz_timer_target(pinned);
-	int basenum = base->index;
 
 again:
 	new_cpu_base = &per_cpu(hrtimer_bases, cpu);
-	new_base = &new_cpu_base->clock_base[basenum];
+	new_base = &new_cpu_base->clock_base[base->index];
 
 	if (base != new_base) {
 		/*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 11/36] hrtimer: no need to rewrite '1' to hrtimer_hres_enabled

[Viresh Kumar]

High Resolution feature can be enabled/disabled from bootargs if we have a
string 'highres=' followed by 'on' or 'off'. The default value of this variable
is '1'.  When 'on' is passed as bootarg, we don't have to overwrite this
variable by '1'.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 04e4c77..b0fbf12 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -509,9 +509,7 @@ static int __init setup_hrtimer_hres(char *str)
 {
 	if (!strcmp(str, "off"))
 		hrtimer_hres_enabled = 0;
-	else if (!strcmp(str, "on"))
-		hrtimer_hres_enabled = 1;
-	else
+	else if (strcmp(str, "on"))
 		return 0;
 	return 1;
 }
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 12/36] hrtimer: use base->hres_active directly instead of hrtimer_hres_active()

[Viresh Kumar]

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

	__this_cpu_read(hrtimer_bases.hres_active)

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

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index b0fbf12..ad5b7ba 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -672,7 +672,7 @@ static void retrigger_next_event(void *arg)
 {
 	struct hrtimer_cpu_base *base = &__get_cpu_var(hrtimer_bases);
 
-	if (!hrtimer_hres_active())
+	if (!base->hres_active)
 		return;
 
 	raw_spin_lock(&base->lock);
@@ -897,7 +897,7 @@ static void __remove_hrtimer(struct hrtimer *timer,
 	if (&timer->node == next_timer) {
 #ifdef CONFIG_HIGH_RES_TIMERS
 		/* Reprogram the clock event device. if enabled */
-		if (reprogram && hrtimer_hres_active()) {
+		if (reprogram && base->cpu_base->hres_active) {
 			ktime_t expires;
 
 			expires = ktime_sub(hrtimer_get_expires(timer),
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 13/36] hrtimer: don't check state of base->hres_active in hrtimer_switch_to_hres()

[Viresh Kumar]

Caller of hrtimer_switch_to_hres(), i.e. hrtimer_run_pending(), has already
verified this by calling hrtimer_hres_active() and so we don't need to do it
again in hrtimer_switch_to_hres().

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ad5b7ba..476ad5d 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -690,9 +690,6 @@ static int hrtimer_switch_to_hres(void)
 	struct hrtimer_cpu_base *base = &per_cpu(hrtimer_bases, cpu);
 	unsigned long flags;
 
-	if (base->hres_active)
-		return 1;
-
 	local_irq_save(flags);
 
 	if (tick_init_highres()) {
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 14/36] hrtimer: reorder code in __remove_hrtimer()

[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. So, move this check within the #ifdef/endif
  block.
- Validate 'reprogram' and hrtimer_hres_active() first as without these we don't
  need to check if 'timer' is the next one to fire.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 476ad5d..833db9f 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -891,19 +891,18 @@ static void __remove_hrtimer(struct hrtimer *timer,
 
 	next_timer = timerqueue_getnext(&base->active);
 	timerqueue_del(&base->active, &timer->node);
-	if (&timer->node == next_timer) {
+
 #ifdef CONFIG_HIGH_RES_TIMERS
-		/* Reprogram the clock event device. if enabled */
-		if (reprogram && base->cpu_base->hres_active) {
-			ktime_t expires;
-
-			expires = ktime_sub(hrtimer_get_expires(timer),
-					    base->offset);
-			if (base->cpu_base->expires_next.tv64 == expires.tv64)
-				hrtimer_force_reprogram(base->cpu_base, 1);
-		}
-#endif
+	/* Reprogram the clock event device. if enabled */
+	if (reprogram && base->cpu_base->hres_active &&
+	    &timer->node == next_timer) {
+		ktime_t expires;
+
+		expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
+		if (base->cpu_base->expires_next.tv64 == expires.tv64)
+			hrtimer_force_reprogram(base->cpu_base, 1);
 	}
+#endif
 	if (!timerqueue_getnext(&base->active))
 		base->cpu_base->active_bases &= ~(1 << base->index);
 out:
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 15/36] hrtimer: don't emulate notifier call to initialize timer base

[Viresh Kumar]

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

So, this patch calls init_hrtimers_cpu() directly from hrtimers_init().

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 833db9f..5b0cbe7 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1753,8 +1753,7 @@ static struct notifier_block hrtimers_nb = {
 
 void __init hrtimers_init(void)
 {
-	hrtimer_cpu_notify(&hrtimers_nb, (unsigned long)CPU_UP_PREPARE,
-			  (void *)(long)smp_processor_id());
+	init_hrtimers_cpu(smp_processor_id());
 	register_cpu_notifier(&hrtimers_nb);
 #ifdef CONFIG_HIGH_RES_TIMERS
 	open_softirq(HRTIMER_SOFTIRQ, run_hrtimer_softirq);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 16/36] hrtimer: Create hrtimer_get_monoexpires()

[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 17c08ca..d1836cb 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -233,6 +233,12 @@ static inline ktime_t hrtimer_get_expires(const struct hrtimer *timer)
 	return timer->node.expires;
 }
 
+static inline ktime_t hrtimer_get_monoexpires(const struct hrtimer *timer,
+		struct hrtimer_clock_base *base)
+{
+	return ktime_sub(hrtimer_get_expires(timer), base->offset);
+}
+
 static inline ktime_t hrtimer_get_softexpires(const struct hrtimer *timer)
 {
 	return timer->_softexpires;
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 5b0cbe7..1a1fdc0 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -184,7 +184,7 @@ hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base)
 	if (!new_base->cpu_base->hres_active)
 		return 0;
 
-	expires = ktime_sub(hrtimer_get_expires(timer), new_base->offset);
+	expires = hrtimer_get_monoexpires(timer, new_base);
 	return expires.tv64 <= new_base->cpu_base->expires_next.tv64;
 #else
 	return 0;
@@ -554,7 +554,7 @@ hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal)
 			continue;
 		timer = container_of(next, struct hrtimer, node);
 
-		expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
+		expires = hrtimer_get_monoexpires(timer, base);
 		/*
 		 * clock_was_set() has changed base->offset so the
 		 * result might be negative. Fix it up to prevent a
@@ -588,7 +588,7 @@ static int hrtimer_reprogram(struct hrtimer *timer,
 			     struct hrtimer_clock_base *base)
 {
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
-	ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
+	ktime_t expires = hrtimer_get_monoexpires(timer, base);
 	int res;
 
 	WARN_ON_ONCE(hrtimer_get_expires_tv64(timer) < 0);
@@ -898,7 +898,7 @@ static void __remove_hrtimer(struct hrtimer *timer,
 	    &timer->node == next_timer) {
 		ktime_t expires;
 
-		expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
+		expires = hrtimer_get_monoexpires(timer, base);
 		if (base->cpu_base->expires_next.tv64 == expires.tv64)
 			hrtimer_force_reprogram(base->cpu_base, 1);
 	}
@@ -1309,8 +1309,7 @@ retry:
 			if (basenow.tv64 < hrtimer_get_softexpires_tv64(timer)) {
 				ktime_t expires;
 
-				expires = ktime_sub(hrtimer_get_expires(timer),
-						    base->offset);
+				expires = hrtimer_get_monoexpires(timer, base);
 				if (expires.tv64 < 0)
 					expires.tv64 = KTIME_MAX;
 				if (expires.tv64 < expires_next.tv64)
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 17/36] hrtimer: don't check if timer is queued in __remove_hrtimer()

[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 | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 1a1fdc0..58b5e3f 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -886,8 +886,6 @@ static void __remove_hrtimer(struct hrtimer *timer,
 			     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);
@@ -903,10 +901,9 @@ static void __remove_hrtimer(struct hrtimer *timer,
 			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;
 }
 
 /*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 18/36] hrtimer: rewrite switch_hrtimer_base() to remove extra indentation level

[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 | 51 ++++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 58b5e3f..fe13dcf 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -207,33 +207,34 @@ again:
 	new_cpu_base = &per_cpu(hrtimer_bases, cpu);
 	new_base = &new_cpu_base->clock_base[base->index];
 
-	if (base != new_base) {
-		/*
-		 * We are trying to move timer to new_base.
-		 * However we can't change timer's base while it is running,
-		 * so we keep it on the same CPU. No hassle vs. reprogramming
-		 * the event source in the high resolution case. The softirq
-		 * code will take care of this when the timer function has
-		 * completed. There is no conflict as we hold the lock until
-		 * the timer is enqueued.
-		 */
-		if (unlikely(hrtimer_callback_running(timer)))
-			return base;
-
-		/* See the comment in lock_timer_base() */
-		timer->base = NULL;
-		raw_spin_unlock(&base->cpu_base->lock);
-		raw_spin_lock(&new_base->cpu_base->lock);
+	if (base == new_base)
+		return base;
 
-		if (cpu != this_cpu && hrtimer_check_target(timer, new_base)) {
-			cpu = this_cpu;
-			raw_spin_unlock(&new_base->cpu_base->lock);
-			raw_spin_lock(&base->cpu_base->lock);
-			timer->base = base;
-			goto again;
-		}
-		timer->base = new_base;
+	/*
+	 * We are trying to move timer to new_base. However we can't change
+	 * timer's base while it is running, so we keep it on the same CPU. No
+	 * hassle vs. reprogramming the event source in the high resolution
+	 * case. The softirq code will take care of this when the timer function
+	 * has completed. There is no conflict as we hold the lock until the
+	 * timer is enqueued.
+	 */
+	if (unlikely(hrtimer_callback_running(timer)))
+		return base;
+
+	/* See the comment in lock_timer_base() */
+	timer->base = NULL;
+	raw_spin_unlock(&base->cpu_base->lock);
+	raw_spin_lock(&new_base->cpu_base->lock);
+
+	if (cpu != this_cpu && hrtimer_check_target(timer, new_base)) {
+		cpu = this_cpu;
+		raw_spin_unlock(&new_base->cpu_base->lock);
+		raw_spin_lock(&base->cpu_base->lock);
+		timer->base = base;
+		goto again;
 	}
+
+	timer->base = new_base;
 	return new_base;
 }
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 19/36] hrtimer: rewrite remove_hrtimer() to remove extra indentation level

[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 | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index fe13dcf..2ac423d 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -913,31 +913,29 @@ static void __remove_hrtimer(struct hrtimer *timer,
 static inline int
 remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base)
 {
-	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 = 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);
-		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, base, state,
+			 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

[PATCH V2 20/36] hrtimer: replace base by new_base to get resolution: __hrtimer_start_range_ns()

[Viresh Kumar]

This code was added long back by following commit:

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

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

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

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

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

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

[PATCH V2 21/36] hrtimer: create base_on_this_cpu()

[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 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 458b952..2d5bb9d 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
@@ -933,8 +937,7 @@ remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base)
 	 * move the timer base in switch_hrtimer_base.
 	 */
 	state = timer->state & HRTIMER_STATE_CALLBACK;
-	__remove_hrtimer(timer, base, state,
-			 base->cpu_base == &__get_cpu_var(hrtimer_bases));
+	__remove_hrtimer(timer, base, state, base_on_this_cpu(base));
 	return 1;
 }
 
@@ -980,7 +983,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)
+	if (leftmost && base_on_this_cpu(new_base)
 		&& hrtimer_enqueue_reprogram(timer, new_base)) {
 		if (wakeup) {
 			/*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 22/36] hrtimer: clear active_bases as soon as the timer is removed

[Viresh Kumar]

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

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 2d5bb9d..379d21a 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -895,6 +895,9 @@ static void __remove_hrtimer(struct hrtimer *timer,
 	next_timer = timerqueue_getnext(&base->active);
 	timerqueue_del(&base->active, &timer->node);
 
+	if (!timerqueue_getnext(&base->active))
+		base->cpu_base->active_bases &= ~(1 << base->index);
+
 #ifdef CONFIG_HIGH_RES_TIMERS
 	/* Reprogram the clock event device. if enabled */
 	if (reprogram && base->cpu_base->hres_active &&
@@ -907,8 +910,6 @@ static void __remove_hrtimer(struct hrtimer *timer,
 	}
 #endif
 	timer->state = newstate;
-	if (!timerqueue_getnext(&base->active))
-		base->cpu_base->active_bases &= ~(1 << base->index);
 }
 
 /*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 23/36] hrtimer: create for_each_active_base()

[Viresh Kumar]

There are many places where we need to iterate over all the currently active
clock bases for a particular cpu_base. Create for_each_active_base() to simplify
code at those places.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 379d21a..ceadfa5 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -113,6 +113,19 @@ static inline bool base_on_this_cpu(struct hrtimer_clock_base *base)
 }
 
 /*
+ * for_each_active_base: iterate over all active clock bases
+ * @_index: 'int' variable for internal purpose
+ * @_base: holds pointer to a active clock base
+ * @_cpu_base: cpu base to iterate on
+ * @_active_bases: 'unsigned int' variable for internal purpose
+ */
+#define for_each_active_base(_index, _base, _cpu_base, _active_bases)	\
+	for ((_active_bases) = (_cpu_base)->active_bases;		\
+		(_index) = ffs(_active_bases),				\
+		(_base) = (_cpu_base)->clock_base + (_index) - 1, (_index); \
+		(_active_bases) &= ~(1 << ((_index) - 1)))
+
+/*
  * Get the coarse grained time at the softirq based on xtime and
  * wall_to_monotonic.
  */
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 24/36] hrtimer: Use for_each_active_base() to iterate over active clock bases

[Viresh Kumar]

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ceadfa5..b3ab19a 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -557,21 +557,17 @@ static inline int hrtimer_hres_active(void)
 static void
 hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal)
 {
-	int i;
-	struct hrtimer_clock_base *base = cpu_base->clock_base;
+	struct hrtimer_clock_base *base;
+	struct hrtimer *timer;
 	ktime_t expires, expires_next;
+	unsigned int active_bases;
+	int i;
 
 	expires_next.tv64 = KTIME_MAX;
 
-	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) {
-		struct hrtimer *timer;
-		struct timerqueue_node *next;
-
-		next = timerqueue_getnext(&base->active);
-		if (!next)
-			continue;
-		timer = container_of(next, struct hrtimer, node);
-
+	for_each_active_base(i, base, cpu_base, active_bases) {
+		timer = container_of(timerqueue_getnext(&base->active),
+				     struct hrtimer, node);
 		expires = hrtimer_get_monoexpires(timer, base);
 		/*
 		 * clock_was_set() has changed base->offset so the
@@ -1131,23 +1127,19 @@ EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
 ktime_t hrtimer_get_next_event(void)
 {
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
-	struct hrtimer_clock_base *base = cpu_base->clock_base;
+	struct hrtimer_clock_base *base;
 	ktime_t delta, mindelta = { .tv64 = KTIME_MAX };
+	struct hrtimer *timer;
+	unsigned int active_bases;
 	unsigned long flags;
 	int i;
 
 	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 
 	if (!hrtimer_hres_active()) {
-		for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) {
-			struct hrtimer *timer;
-			struct timerqueue_node *next;
-
-			next = timerqueue_getnext(&base->active);
-			if (!next)
-				continue;
-
-			timer = container_of(next, struct hrtimer, node);
+		for_each_active_base(i, base, cpu_base, active_bases) {
+			timer = container_of(timerqueue_getnext(&base->active),
+					     struct hrtimer, node);
 			delta.tv64 = hrtimer_get_expires_tv64(timer);
 			delta = ktime_sub(delta, base->get_time());
 			if (delta.tv64 < mindelta.tv64)
@@ -1270,7 +1262,9 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
 void hrtimer_interrupt(struct clock_event_device *dev)
 {
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
+	struct hrtimer_clock_base *base;
 	ktime_t expires_next, now, entry_time, delta;
+	unsigned int active_bases;
 	int i, retries = 0;
 
 	BUG_ON(!cpu_base->hres_active);
@@ -1290,15 +1284,10 @@ retry:
 	 */
 	cpu_base->expires_next.tv64 = KTIME_MAX;
 
-	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) {
-		struct hrtimer_clock_base *base;
+	for_each_active_base(i, base, cpu_base, active_bases) {
 		struct timerqueue_node *node;
 		ktime_t basenow;
 
-		if (!(cpu_base->active_bases & (1 << i)))
-			continue;
-
-		base = cpu_base->clock_base + i;
 		basenow = ktime_add(now, base->offset);
 
 		while ((node = timerqueue_getnext(&base->active))) {
@@ -1468,16 +1457,13 @@ void hrtimer_run_queues(void)
 	struct timerqueue_node *node;
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
 	struct hrtimer_clock_base *base;
+	unsigned int active_bases;
 	int index, gettime = 1;
 
 	if (hrtimer_hres_active())
 		return;
 
-	for (index = 0; index < HRTIMER_MAX_CLOCK_BASES; index++) {
-		base = &cpu_base->clock_base[index];
-		if (!timerqueue_getnext(&base->active))
-			continue;
-
+	for_each_active_base(index, base, cpu_base, active_bases) {
 		if (gettime) {
 			hrtimer_get_softirq_time(cpu_base);
 			gettime = 0;
@@ -1697,6 +1683,8 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
 static void migrate_hrtimers(int scpu)
 {
 	struct hrtimer_cpu_base *old_base, *new_base;
+	struct hrtimer_clock_base *clock_base;
+	unsigned int active_bases;
 	int i;
 
 	BUG_ON(cpu_online(scpu));
@@ -1712,9 +1700,9 @@ static void migrate_hrtimers(int scpu)
 	raw_spin_lock(&new_base->lock);
 	raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);
 
-	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++)
-		migrate_hrtimer_list(&old_base->clock_base[i],
-				     &new_base->clock_base[i]);
+	for_each_active_base(i, clock_base, old_base, active_bases)
+		migrate_hrtimer_list(clock_base,
+				     &new_base->clock_base[clock_base->index]);
 
 	raw_spin_unlock(&old_base->lock);
 	raw_spin_unlock(&new_base->lock);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 25/36] hrtimer: call hrtimer_get_softirq_time() only if cpu_base->active_bases is set

[Viresh Kumar]

We need to call hrtimer_get_softirq_time() only once for a cpu_base from
hrtimer_run_queues(). And it shouldn't be called if there are no timers queued
for that cpu_base.

Currently we are managing this with help of a variable: gettime. This part of
code can be simplified by using cpu_base->active_bases instead. With this we can
get rid of the 'if' block from the loop iterating over all clock bases.

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index b3ab19a..2d9a7e2 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1458,19 +1458,16 @@ void hrtimer_run_queues(void)
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
 	struct hrtimer_clock_base *base;
 	unsigned int active_bases;
-	int index, gettime = 1;
+	int index;
 
 	if (hrtimer_hres_active())
 		return;
 
-	for_each_active_base(index, base, cpu_base, active_bases) {
-		if (gettime) {
-			hrtimer_get_softirq_time(cpu_base);
-			gettime = 0;
-		}
+	if (cpu_base->active_bases)
+		hrtimer_get_softirq_time(cpu_base);
 
+	for_each_active_base(index, base, cpu_base, active_bases) {
 		raw_spin_lock(&cpu_base->lock);
-
 		while ((node = timerqueue_getnext(&base->active))) {
 			struct hrtimer *timer;
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 26/36] hrtimer: take lock only once for a cpu_base in hrtimer_run_queues()

[Viresh Kumar]

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

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 2d9a7e2..c712960 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1466,8 +1466,8 @@ void hrtimer_run_queues(void)
 	if (cpu_base->active_bases)
 		hrtimer_get_softirq_time(cpu_base);
 
+	raw_spin_lock(&cpu_base->lock);
 	for_each_active_base(index, base, cpu_base, active_bases) {
-		raw_spin_lock(&cpu_base->lock);
 		while ((node = timerqueue_getnext(&base->active))) {
 			struct hrtimer *timer;
 
@@ -1478,8 +1478,8 @@ void hrtimer_run_queues(void)
 
 			__run_hrtimer(timer, &base->softirq_time);
 		}
-		raw_spin_unlock(&cpu_base->lock);
 	}
+	raw_spin_unlock(&cpu_base->lock);
 }
 
 /*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 27/36] hrtimer: call switch_hrtimer_base() after setting new expiry time

[Viresh Kumar]

In switch_hrtimer_base() we are calling hrtimer_check_target() which guarantees
this:

/*
 * With HIGHRES=y we do not migrate the timer when it is expiring
 * before the next event on the target cpu because we cannot reprogram
 * the target cpu hardware and we would cause it to fire late.
 *
 * Called with cpu_base->lock of target cpu held.
 */

But switch_hrtimer_base() is only called from one place, i.e.
__hrtimer_start_range_ns() and at the point (where we call
switch_hrtimer_base()) expiration time is not yet known as we call this routine
later:

hrtimer_set_expires_range_ns()

To fix this, we need to find the updated expiry time before calling
switch_hrtimer_base() from hrtimer_set_expires_range_ns().

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

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index c712960..81e0251 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -964,11 +964,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	/* Remove an active timer from the queue: */
 	ret = remove_hrtimer(timer, base);
 
-	/* Switch the timer base, if necessary: */
-	new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
-
 	if (mode & HRTIMER_MODE_REL) {
-		tim = ktime_add_safe(tim, new_base->get_time());
+		tim = ktime_add_safe(tim, base->get_time());
 		/*
 		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
 		 * to signal that they simply return xtime in
@@ -977,12 +974,15 @@ 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, new_base->resolution);
+		tim = ktime_add_safe(tim, base->resolution);
 #endif
 	}
 
 	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
 
+	/* Switch the timer base, if necessary: */
+	new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
+
 	timer_stats_hrtimer_set_start_info(timer);
 
 	leftmost = enqueue_hrtimer(timer, new_base);
@@ -1000,7 +1000,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 			 * We need to drop cpu_base->lock to avoid a
 			 * lock ordering issue vs. rq->lock.
 			 */
-			raw_spin_unlock(&new_base->cpu_base->lock);
+			raw_spin_unlock(&timer->base->cpu_base->lock);
 			raise_softirq_irqoff(HRTIMER_SOFTIRQ);
 			local_irq_restore(flags);
 			return ret;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 28/36] hrtimer: remove 'base' parameter from remove_timer() and __remove_timer()

[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 | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 81e0251..a404436 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -895,10 +895,10 @@ static int enqueue_hrtimer(struct hrtimer *timer,
  * reprogram to zero. This is useful, when the context does a reprogramming
  * anyway (e.g. timer interrupt)
  */
-static void __remove_hrtimer(struct hrtimer *timer,
-			     struct hrtimer_clock_base *base,
-			     unsigned long newstate, int reprogram)
+static void __remove_hrtimer(struct hrtimer *timer, unsigned long newstate,
+			     int reprogram)
 {
+	struct hrtimer_clock_base *base = timer->base;
 	struct timerqueue_node *next_timer;
 
 	next_timer = timerqueue_getnext(&base->active);
@@ -921,11 +921,8 @@ static void __remove_hrtimer(struct hrtimer *timer,
 	timer->state = newstate;
 }
 
-/*
- * remove hrtimer, called with base lock held
- */
-static inline int
-remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base)
+/* remove hrtimer, called with base lock held */
+static inline int remove_hrtimer(struct hrtimer *timer)
 {
 	unsigned long state;
 
@@ -947,7 +944,7 @@ remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base)
 	 * move the timer base in switch_hrtimer_base.
 	 */
 	state = timer->state & HRTIMER_STATE_CALLBACK;
-	__remove_hrtimer(timer, base, state, base_on_this_cpu(base));
+	__remove_hrtimer(timer, state, base_on_this_cpu(timer->base));
 	return 1;
 }
 
@@ -962,7 +959,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	base = lock_hrtimer_base(timer, &flags);
 
 	/* Remove an active timer from the queue: */
-	ret = remove_hrtimer(timer, base);
+	ret = remove_hrtimer(timer);
 
 	if (mode & HRTIMER_MODE_REL) {
 		tim = ktime_add_safe(tim, base->get_time());
@@ -1064,14 +1061,13 @@ EXPORT_SYMBOL_GPL(hrtimer_start);
  */
 int hrtimer_try_to_cancel(struct hrtimer *timer)
 {
-	struct hrtimer_clock_base *base;
 	unsigned long flags;
 	int ret = -1;
 
-	base = lock_hrtimer_base(timer, &flags);
+	lock_hrtimer_base(timer, &flags);
 
 	if (!hrtimer_callback_running(timer))
-		ret = remove_hrtimer(timer, base);
+		ret = remove_hrtimer(timer);
 
 	unlock_hrtimer_base(timer, &flags);
 
@@ -1223,7 +1219,7 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
 	WARN_ON(!irqs_disabled());
 
 	debug_deactivate(timer);
-	__remove_hrtimer(timer, base, HRTIMER_STATE_CALLBACK, 0);
+	__remove_hrtimer(timer, HRTIMER_STATE_CALLBACK, 0);
 	timer_stats_account_hrtimer(timer);
 	fn = timer->function;
 
@@ -1660,7 +1656,7 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
 		 * timer could be seen as !active and just vanish away
 		 * under us on another CPU
 		 */
-		__remove_hrtimer(timer, old_base, HRTIMER_STATE_MIGRATE, 0);
+		__remove_hrtimer(timer, HRTIMER_STATE_MIGRATE, 0);
 		timer->base = new_base;
 		/*
 		 * Enqueue the timers on the new cpu. This does not
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 29/36] hrtimer: remove 'base' parameter from switch_hrtimer_base()

[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 a404436..c35dc36 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -212,10 +212,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);
@@ -267,7 +266,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 */
 
@@ -978,7 +977,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
 
 	/* Switch the timer base, if necessary: */
-	new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
+	new_base = switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED);
 
 	timer_stats_hrtimer_set_start_info(timer);
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 30/36] hrtimer: remove 'base' parameter from enqueue_hrtimer()

[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 c35dc36..abbf155 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -867,9 +867,10 @@ EXPORT_SYMBOL_GPL(hrtimer_forward);
  *
  * Returns 1 when the new timer is the leftmost timer in the tree.
  */
-static int enqueue_hrtimer(struct hrtimer *timer,
-			   struct hrtimer_clock_base *base)
+static int enqueue_hrtimer(struct hrtimer *timer)
 {
+	struct hrtimer_clock_base *base = timer->base;
+
 	debug_activate(timer);
 
 	timerqueue_add(&base->active, &timer->node);
@@ -981,7 +982,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 
 	timer_stats_hrtimer_set_start_info(timer);
 
-	leftmost = enqueue_hrtimer(timer, new_base);
+	leftmost = enqueue_hrtimer(timer);
 
 	/*
 	 * Only allow reprogramming if the new base is on this CPU.
@@ -1210,8 +1211,7 @@ EXPORT_SYMBOL_GPL(hrtimer_get_res);
 
 static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
 {
-	struct hrtimer_clock_base *base = timer->base;
-	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
+	struct hrtimer_cpu_base *cpu_base = timer->base->cpu_base;
 	enum hrtimer_restart (*fn)(struct hrtimer *);
 	int restart;
 
@@ -1240,7 +1240,7 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
 	 */
 	if (restart != HRTIMER_NORESTART) {
 		BUG_ON(timer->state != HRTIMER_STATE_CALLBACK);
-		enqueue_hrtimer(timer, base);
+		enqueue_hrtimer(timer);
 	}
 
 	WARN_ON_ONCE(!(timer->state & HRTIMER_STATE_CALLBACK));
@@ -1665,7 +1665,7 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
 		 * sort out already expired timers and reprogram the
 		 * event device.
 		 */
-		enqueue_hrtimer(timer, new_base);
+		enqueue_hrtimer(timer);
 
 		/* Clear the migration state bit */
 		timer->state &= ~HRTIMER_STATE_MIGRATE;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 31/36] hrtimer: remove 'base' parameter from hrtimer_{enqueue_}reprogram()

[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 abbf155..fcbabcf 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -597,11 +597,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);
@@ -661,10 +660,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)
@@ -743,8 +741,7 @@ 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;
 }
@@ -991,7 +988,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	 * XXX send_remote_softirq() ?
 	 */
 	if (leftmost && base_on_this_cpu(new_base)
-		&& 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

[PATCH V2 32/36] hrtimer: make switch_hrtimer_base() return void

[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 | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index fcbabcf..e581bba 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -211,8 +211,7 @@ hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base)
 /*
  * Switch the timer base to the current CPU when possible.
  */
-static inline struct hrtimer_clock_base *
-switch_hrtimer_base(struct hrtimer *timer, int pinned)
+static inline void switch_hrtimer_base(struct hrtimer *timer, int pinned)
 {
 	struct hrtimer_clock_base *new_base, *base = timer->base;
 	struct hrtimer_cpu_base *new_cpu_base;
@@ -224,7 +223,7 @@ again:
 	new_base = &new_cpu_base->clock_base[base->index];
 
 	if (base == new_base)
-		return base;
+		return;
 
 	/*
 	 * We are trying to move timer to new_base. However we can't change
@@ -235,7 +234,7 @@ again:
 	 * timer is enqueued.
 	 */
 	if (unlikely(hrtimer_callback_running(timer)))
-		return base;
+		return;
 
 	/* See the comment in lock_timer_base() */
 	timer->base = NULL;
@@ -251,7 +250,6 @@ again:
 	}
 
 	timer->base = new_base;
-	return new_base;
 }
 
 #else /* CONFIG_SMP */
@@ -266,7 +264,7 @@ lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags)
 	return base;
 }
 
-# define switch_hrtimer_base(t, p)	(t->base)
+static inline void switch_hrtimer_base(struct hrtimer *timer, int pinned) {}
 
 #endif	/* !CONFIG_SMP */
 
@@ -949,7 +947,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 		unsigned long delta_ns, const enum hrtimer_mode mode,
 		int wakeup)
 {
-	struct hrtimer_clock_base *base, *new_base;
+	struct hrtimer_clock_base *base;
 	unsigned long flags;
 	int ret, leftmost;
 
@@ -975,7 +973,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
 
 	/* Switch the timer base, if necessary: */
-	new_base = switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED);
+	switch_hrtimer_base(timer, mode & HRTIMER_MODE_PINNED);
 
 	timer_stats_hrtimer_set_start_info(timer);
 
@@ -987,7 +985,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	 *
 	 * XXX send_remote_softirq() ?
 	 */
-	if (leftmost && base_on_this_cpu(new_base)
+	if (leftmost && base_on_this_cpu(timer->base)
 		&& hrtimer_enqueue_reprogram(timer)) {
 		if (wakeup) {
 			/*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 33/36] hrtimer: make lock_hrtimer_base() return void

[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 e581bba..ea620e5 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -166,9 +166,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;
 
@@ -177,7 +175,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);
 		}
@@ -254,14 +252,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) {}
@@ -951,7 +945,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

[PATCH V2 34/36] hrtimer: make enqueue_hrtimer() return void

[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        | 10 ++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index d1836cb..435ac4c 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -263,6 +263,11 @@ static inline ktime_t hrtimer_expires_remaining(const struct hrtimer *timer)
 	return ktime_sub(timer->node.expires, timer->base->get_time());
 }
 
+static inline int hrtimer_is_leftmost(struct hrtimer *timer)
+{
+	return &timer->node == timer->base->active.next;
+}
+
 #ifdef CONFIG_HIGH_RES_TIMERS
 struct clock_event_device;
 
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ea620e5..d62fe32 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -856,7 +856,7 @@ EXPORT_SYMBOL_GPL(hrtimer_forward);
  *
  * Returns 1 when the new timer is the leftmost timer in the tree.
  */
-static int enqueue_hrtimer(struct hrtimer *timer)
+static void enqueue_hrtimer(struct hrtimer *timer)
 {
 	struct hrtimer_clock_base *base = timer->base;
 
@@ -870,8 +870,6 @@ static int enqueue_hrtimer(struct hrtimer *timer)
 	 * state of a possibly running callback.
 	 */
 	timer->state |= HRTIMER_STATE_ENQUEUED;
-
-	return (&timer->node == base->active.next);
 }
 
 /*
@@ -943,7 +941,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 {
 	struct hrtimer_clock_base *base;
 	unsigned long flags;
-	int ret, leftmost;
+	int ret;
 
 	lock_hrtimer_base(timer, &flags);
 	base = timer->base;
@@ -972,7 +970,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 
 	timer_stats_hrtimer_set_start_info(timer);
 
-	leftmost = enqueue_hrtimer(timer);
+	enqueue_hrtimer(timer);
 
 	/*
 	 * Only allow reprogramming if the new base is on this CPU.
@@ -980,7 +978,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	 *
 	 * XXX send_remote_softirq() ?
 	 */
-	if (leftmost && base_on_this_cpu(timer->base)
+	if (hrtimer_is_leftmost(timer) && base_on_this_cpu(timer->base)
 		&& hrtimer_enqueue_reprogram(timer)) {
 		if (wakeup) {
 			/*
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 35/36] timer: simplify CPU_UP_PREPARE notifier code path

[Viresh Kumar]

Currently we are returning notifier_from_errno() from CPU_UP_PREPARE notifier
when we detect there is an error while calling init_timers_cpu().
notifier_from_errno() already has enough checks within to do something similar.
And so we can call it directly without checking if there was an error or not.

Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/timer.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/timer.c b/kernel/timer.c
index 1d35dda..4360edc 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1646,9 +1646,7 @@ static int timer_cpu_notify(struct notifier_block *self,
 	case CPU_UP_PREPARE:
 	case CPU_UP_PREPARE_FROZEN:
 		err = init_timers_cpu(cpu);
-		if (err < 0)
-			return notifier_from_errno(err);
-		break;
+		return notifier_from_errno(err);
 #ifdef CONFIG_HOTPLUG_CPU
 	case CPU_DEAD:
 	case CPU_DEAD_FROZEN:
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 36/36] timer: don't emulate notifier call to initialize timer base

[Viresh Kumar]

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

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

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

diff --git a/kernel/timer.c b/kernel/timer.c
index 4360edc..d13eb56 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1666,15 +1666,9 @@ static struct notifier_block timers_nb = {
 
 void __init init_timers(void)
 {
-	int err;
-
 	/* ensure there are enough low bits for flags in timer->base pointer */
 	BUILD_BUG_ON(__alignof__(struct tvec_base) & TIMER_FLAG_MASK);
-
-	err = timer_cpu_notify(&timers_nb, (unsigned long)CPU_UP_PREPARE,
-			       (void *)(long)smp_processor_id());
-	BUG_ON(err != NOTIFY_OK);
-
+	BUG_ON(init_timers_cpu(smp_processor_id()));
 	init_timer_stats();
 	register_cpu_notifier(&timers_nb);
 	open_softirq(TIMER_SOFTIRQ, run_timer_softirq);
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V2 00/36] timers/hrtimers: Cleanups & Fixes

[Viresh Kumar]

On 4 April 2014 12:05, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> I know you are not going to look at these before end of this merge window and
> you wanted to have a look at V1 before me posting these. But I am reposting them
> now due to these reasons:
> - Need to resend my cpu isolation (cpuset.quiesce) patches which are based of
>   these
> - Few patches are dropped/merged/fixed/updated and so all the patches from V1
>   wouldn't have made sense
> - There were some new patches as well which I wanted to send
>
> These have gone through fair bit of testing via kbuild system maintained by
> Fengguang Wu.
>
>
> These are some minor cleanups and potential bug fixes in there. These are based
> of tip/timers-core-for-linus ..
>
> V1 of most of these patches (~28) were posted here:
> https://lkml.org/lkml/2014/3/26/107
> https://lkml.org/lkml/2014/3/28/148
>
> V1->V2:
> - few new patches:
>   - patches around for_each_active_base()
>   - hrtimer: call switch_hrtimer_base() after setting new expiry time
>   - Some other minor cleanups
> - few patches are dropped
> - few are merged together as they covered same stuff
> - rebased all patches and moved the patches removing parameters or return values
>   at the bottom, so that others can be applied easily. Though as per my last
>   mail, it doesn't look like they are making the 'text' segments any bigger.

Any inputs on these ?