[PATCH V2 00/19] tick: cleanups (Shouldn't change code behavior)

[PATCH V2 00/19] tick: cleanups (Shouldn't change code behavior)

[Viresh Kumar]

As suggested by you (https://lkml.org/lkml/2014/4/14/797), this is the second
lot of changes I have. I have divided the earlier patchset into three parts:
- Bugfixes, already merged
- Code cleanups which shouldn't have any functional change
- Code cleanups which may have any functional change

This patchset is targeting the second part now. Its just about moving the code
around to make it more readable and obvious. Not removing any code at all, that
will be addressed in next series.

V1->V2: Actually V1 was never reviewed and so it is mostly a resend of V1. Some
rearrangement of patches is done though.

Viresh Kumar (19):
  tick: trivial cleanups
  tick: update doc style comments for 'struct tick_sched'
  tick: rearrange members of 'struct tick_sched'
  tick: move declaration of 'tick_cpu_device' to tick.h
  tick: move definition of tick_get_device() to tick.h
  tick: create tick_get_cpu_device() to get tick_cpu_device on this cpu
  tick: initialize variables during their definitions
  tick-oneshot: move tick_is_oneshot_available() to tick-oneshot.c
  tick-oneshot: remove tick_resume_oneshot()
  tick-common: call tick_check_percpu() from tick_check_preferred()
  tick-common: remove tick_check_replacement()
  tick-common: don't pass 'cpu' & 'cpumask' to tick_setup_device()
  tick-common: remove local variable 'broadcast' from tick_resume()
  tick-sched: add comment about 'idle_active' in tick_nohz_idle_exit()
  tick-sched: define 'delta' inside 'if' block in
    update_ts_time_stats()
  tick-sched: remove parameters to {__}tick_nohz_task_switch() routines
  tick-sched: remove local variable 'now' from tick_setup_sched_timer()
  tick-sched: invert parameter of tick_check_oneshot_change()
  tick-sched: rearrange code in tick_do_update_jiffies64()

 include/linux/hrtimer.h      |  3 --
 include/linux/tick.h         | 62 +++++++++++++++++++-----------
 kernel/hrtimer.c             |  4 +-
 kernel/sched/core.c          |  2 +-
 kernel/time/clockevents.c    | 12 +++---
 kernel/time/clocksource.c    | 14 +++----
 kernel/time/tick-broadcast.c | 48 +++++++++--------------
 kernel/time/tick-common.c    | 90 ++++++++++++++------------------------------
 kernel/time/tick-internal.h  | 15 ++++----
 kernel/time/tick-oneshot.c   | 34 +++++++++--------
 kernel/time/tick-sched.c     | 80 +++++++++++++++++++++++----------------
 kernel/time/timekeeping.c    | 10 ++---
 12 files changed, 177 insertions(+), 197 deletions(-)

-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 01/19] tick: trivial cleanups

[Viresh Kumar]

This does some trivial fixups:
- break lines longer than 80 columns
- merge few lines together
- don't break print messages even if they cross 80 columns
- remove extra whitespaces and blank lines
- replace printk() with pr_*()

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h         |  3 ++-
 kernel/time/clockevents.c    |  4 ++--
 kernel/time/clocksource.c    | 14 +++++---------
 kernel/time/tick-broadcast.c | 16 ++++++----------
 kernel/time/tick-internal.h  |  5 ++++-
 kernel/time/tick-oneshot.c   |  3 +--
 kernel/time/tick-sched.c     | 29 ++++++++++++++++-------------
 kernel/time/timekeeping.c    | 10 ++++------
 8 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index b84773c..8c865fb 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -47,7 +47,8 @@ enum tick_nohz_mode {
  * @idle_waketime:	Time when the idle was interrupted
  * @idle_exittime:	Time when the idle state was left
  * @idle_sleeptime:	Sum of the time slept in idle with sched tick stopped
- * @iowait_sleeptime:	Sum of the time slept in idle with sched tick stopped, with IO outstanding
+ * @iowait_sleeptime:	Sum of the time slept in idle with sched tick stopped,
+ *			with IO outstanding
  * @sleep_length:	Duration of the current idle sleep
  * @do_timer_lst:	CPU was the last one doing do_timer before going idle
  */
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index ad362c2..2ba812bc 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -146,7 +146,7 @@ static int clockevents_increase_min_delta(struct clock_event_device *dev)
 {
 	/* Nothing to do if we already reached the limit */
 	if (dev->min_delta_ns >= MIN_DELTA_LIMIT) {
-		printk(KERN_WARNING "CE: Reprogramming failure. Giving up\n");
+		pr_warn("CE: Reprogramming failure. Giving up\n");
 		dev->next_event.tv64 = KTIME_MAX;
 		return -ETIME;
 	}
@@ -159,7 +159,7 @@ static int clockevents_increase_min_delta(struct clock_event_device *dev)
 	if (dev->min_delta_ns > MIN_DELTA_LIMIT)
 		dev->min_delta_ns = MIN_DELTA_LIMIT;
 
-	printk(KERN_WARNING "CE: %s increased min_delta_ns to %llu nsec\n",
+	pr_warn("CE: %s increased min_delta_ns to %llu nsec\n",
 	       dev->name ? dev->name : "?",
 	       (unsigned long long) dev->min_delta_ns);
 	return 0;
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index ba3e502..6ef39af 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -219,8 +219,7 @@ static void __clocksource_unstable(struct clocksource *cs)
 
 static void clocksource_unstable(struct clocksource *cs, int64_t delta)
 {
-	printk(KERN_WARNING "Clocksource %s unstable (delta = %Ld ns)\n",
-	       cs->name, delta);
+	pr_warn("Clocksource %s unstable (delta = %Ld ns)\n", cs->name, delta);
 	__clocksource_unstable(cs);
 }
 
@@ -643,9 +642,8 @@ static void __clocksource_select(bool skipcur)
 		 */
 		if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
 			/* Override clocksource cannot be used. */
-			printk(KERN_WARNING "Override clocksource %s is not "
-			       "HRT compatible. Cannot switch while in "
-			       "HRT/NOHZ mode\n", cs->name);
+			pr_warn("Override clocksource %s is not HRT compatible. Cannot switch while in HRT/NOHZ mode\n",
+				cs->name);
 			override_name[0] = 0;
 		} else
 			/* Override clocksource can be used. */
@@ -1095,12 +1093,10 @@ __setup("clocksource=", boot_override_clocksource);
 static int __init boot_override_clock(char* str)
 {
 	if (!strcmp(str, "pmtmr")) {
-		printk("Warning: clock=pmtmr is deprecated. "
-			"Use clocksource=acpi_pm.\n");
+		printk("Warning: clock=pmtmr is deprecated. Use clocksource=acpi_pm.\n");
 		return boot_override_clocksource("acpi_pm");
 	}
-	printk("Warning! clock= boot option is deprecated. "
-		"Use clocksource=xyz\n");
+	printk("Warning! clock= boot option is deprecated. Use clocksource=xyz\n");
 	return boot_override_clocksource(str);
 }
 
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 64c5990..b1c7b21 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -132,7 +132,6 @@ int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq)
 	return ret;
 }
 
-
 static void err_broadcast(const struct cpumask *mask)
 {
 	pr_crit_once("Failed to broadcast timer tick. Some CPUs may be unresponsive.\n");
@@ -358,8 +357,7 @@ static void tick_do_broadcast_on_off(unsigned long *reason)
 	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
 		cpumask_set_cpu(cpu, tick_broadcast_on);
 		if (!cpumask_test_and_set_cpu(cpu, tick_broadcast_mask)) {
-			if (tick_broadcast_device.mode ==
-			    TICKDEV_MODE_PERIODIC)
+			if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC)
 				clockevents_shutdown(dev);
 		}
 		if (*reason == CLOCK_EVT_NOTIFY_BROADCAST_FORCE)
@@ -372,8 +370,7 @@ static void tick_do_broadcast_on_off(unsigned long *reason)
 		if (!tick_device_is_functional(dev))
 			break;
 		if (cpumask_test_and_clear_cpu(cpu, tick_broadcast_mask)) {
-			if (tick_broadcast_device.mode ==
-			    TICKDEV_MODE_PERIODIC)
+			if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC)
 				tick_setup_periodic(dev, 0);
 		}
 		break;
@@ -399,8 +396,8 @@ out:
 void tick_broadcast_on_off(unsigned long reason, int *oncpu)
 {
 	if (!cpumask_test_cpu(*oncpu, cpu_online_mask))
-		printk(KERN_ERR "tick-broadcast: ignoring broadcast for "
-		       "offline CPU #%d\n", *oncpu);
+		pr_err("tick-broadcast: ignoring broadcast for offline CPU #%d\n",
+		       *oncpu);
 	else
 		tick_do_broadcast_on_off(&reason);
 }
@@ -484,7 +481,6 @@ int tick_resume_broadcast(void)
 	return broadcast;
 }
 
-
 #ifdef CONFIG_TICK_ONESHOT
 
 static cpumask_var_t tick_broadcast_oneshot_mask;
@@ -727,7 +723,8 @@ int tick_broadcast_oneshot_control(unsigned long reason)
 			 */
 			if (!cpumask_test_cpu(cpu, tick_broadcast_force_mask) &&
 			    dev->next_event.tv64 < bc->next_event.tv64)
-				tick_broadcast_set_event(bc, cpu, dev->next_event, 1);
+				tick_broadcast_set_event(bc, cpu,
+							 dev->next_event, 1);
 		}
 		/*
 		 * If the current CPU owns the hrtimer broadcast
@@ -894,7 +891,6 @@ void tick_broadcast_switch_to_oneshot(void)
 	raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
 }
 
-
 /*
  * Remove a dead CPU from broadcasting
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 7ab92b1..855c513 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -87,7 +87,10 @@ static inline void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 {
 	BUG();
 }
-static inline int tick_broadcast_oneshot_control(unsigned long reason) { return 0; }
+static inline int tick_broadcast_oneshot_control(unsigned long reason)
+{
+	return 0;
+}
 static inline void tick_shutdown_broadcast_oneshot(unsigned int *cpup) { }
 static inline int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
 {
diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
index 8241090..5fe86a7 100644
--- a/kernel/time/tick-oneshot.c
+++ b/kernel/time/tick-oneshot.c
@@ -65,8 +65,7 @@ int tick_switch_to_oneshot(void (*handler)(struct clock_event_device *))
 	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT) ||
 		    !tick_device_is_functional(dev)) {
 
-		printk(KERN_INFO "Clockevents: "
-		       "could not switch to one-shot mode:");
+		pr_info("Clockevents: could not switch to one-shot mode:");
 		if (!dev) {
 			printk(" no tick device\n");
 		} else {
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 6558b7a..9e9ddba 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -108,7 +108,6 @@ static ktime_t tick_init_jiffy_update(void)
 	return period;
 }
 
-
 static void tick_sched_do_timer(ktime_t now)
 {
 	int cpu = smp_processor_id();
@@ -248,8 +247,8 @@ void tick_nohz_full_kick_all(void)
 		return;
 
 	preempt_disable();
-	smp_call_function_many(tick_nohz_full_mask,
-			       nohz_full_kick_ipi, NULL, false);
+	smp_call_function_many(tick_nohz_full_mask, nohz_full_kick_ipi, NULL,
+			       false);
 	tick_nohz_full_kick();
 	preempt_enable();
 }
@@ -288,7 +287,8 @@ static int __init tick_nohz_full_setup(char *str)
 
 	cpu = smp_processor_id();
 	if (cpumask_test_cpu(cpu, tick_nohz_full_mask)) {
-		pr_warning("NO_HZ: Clearing %d from nohz_full range for timekeeping\n", cpu);
+		pr_warning("NO_HZ: Clearing %d from nohz_full range for timekeeping\n",
+			   cpu);
 		cpumask_clear_cpu(cpu, tick_nohz_full_mask);
 	}
 	tick_nohz_full_running = true;
@@ -298,8 +298,7 @@ static int __init tick_nohz_full_setup(char *str)
 __setup("nohz_full=", tick_nohz_full_setup);
 
 static int tick_nohz_cpu_down_callback(struct notifier_block *nfb,
-						 unsigned long action,
-						 void *hcpu)
+				       unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
 
@@ -353,7 +352,8 @@ void __init tick_nohz_init(void)
 		context_tracking_cpu_set(cpu);
 
 	cpu_notifier(tick_nohz_cpu_down_callback, 0);
-	cpulist_scnprintf(nohz_full_buf, sizeof(nohz_full_buf), tick_nohz_full_mask);
+	cpulist_scnprintf(nohz_full_buf, sizeof(nohz_full_buf),
+			  tick_nohz_full_mask);
 	pr_info("NO_HZ: Full dynticks CPUs: %s.\n", nohz_full_buf);
 }
 #endif
@@ -365,8 +365,8 @@ void __init tick_nohz_init(void)
 /*
  * NO HZ enabled ?
  */
-static int tick_nohz_enabled __read_mostly  = 1;
-int tick_nohz_active  __read_mostly;
+static int tick_nohz_enabled __read_mostly = 1;
+int tick_nohz_active __read_mostly;
 /*
  * Enable / Disable tickless mode
  */
@@ -410,16 +410,19 @@ static void tick_nohz_update_jiffies(ktime_t now)
  * Updates the per cpu time idle statistics counters
  */
 static void
-update_ts_time_stats(int cpu, struct tick_sched *ts, ktime_t now, u64 *last_update_time)
+update_ts_time_stats(int cpu, struct tick_sched *ts, ktime_t now,
+		     u64 *last_update_time)
 {
 	ktime_t delta;
 
 	if (ts->idle_active) {
 		delta = ktime_sub(now, ts->idle_entrytime);
 		if (nr_iowait_cpu(cpu) > 0)
-			ts->iowait_sleeptime = ktime_add(ts->iowait_sleeptime, delta);
+			ts->iowait_sleeptime = ktime_add(ts->iowait_sleeptime,
+							 delta);
 		else
-			ts->idle_sleeptime = ktime_add(ts->idle_sleeptime, delta);
+			ts->idle_sleeptime = ktime_add(ts->idle_sleeptime,
+						       delta);
 		ts->idle_entrytime = now;
 	}
 
@@ -876,7 +879,7 @@ static void tick_nohz_restart_sched_tick(struct tick_sched *ts, ktime_t now)
 	/*
 	 * Cancel the scheduled timer and restore the tick
 	 */
-	ts->tick_stopped  = 0;
+	ts->tick_stopped = 0;
 	ts->idle_exittime = now;
 
 	tick_nohz_restart(ts, now);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index f7df8ea..8e547b5 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -852,8 +852,7 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
 							struct timespec *delta)
 {
 	if (!timespec_valid_strict(delta)) {
-		printk(KERN_WARNING "__timekeeping_inject_sleeptime: Invalid "
-					"sleep delta value!\n");
+		pr_warn("__timekeeping_inject_sleeptime: Invalid sleep delta value!\n");
 		return;
 	}
 	tk_xtime_add(tk, delta);
@@ -1157,10 +1156,9 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 
 	if (unlikely(tk->clock->maxadj &&
 		(tk->mult + adj > tk->clock->mult + tk->clock->maxadj))) {
-		printk_once(KERN_WARNING
-			"Adjusting %s more than 11%% (%ld vs %ld)\n",
-			tk->clock->name, (long)tk->mult + adj,
-			(long)tk->clock->mult + tk->clock->maxadj);
+		pr_warn_once("Adjusting %s more than 11%% (%ld vs %ld)\n",
+			     tk->clock->name, (long)tk->mult + adj,
+			     (long)tk->clock->mult + tk->clock->maxadj);
 	}
 	/*
 	 * So the following can be confusing.
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V2 01/19] tick: trivial cleanups

[Frederic Weisbecker]

On Mon, Apr 21, 2014 at 03:24:57PM +0530, Viresh Kumar wrote:
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 6558b7a..9e9ddba 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -108,7 +108,6 @@ static ktime_t tick_init_jiffy_update(void)
>  	return period;
>  }
>  
> -
>  static void tick_sched_do_timer(ktime_t now)
>  {
>  	int cpu = smp_processor_id();
> @@ -248,8 +247,8 @@ void tick_nohz_full_kick_all(void)
>  		return;
>  
>  	preempt_disable();
> -	smp_call_function_many(tick_nohz_full_mask,
> -			       nohz_full_kick_ipi, NULL, false);
> +	smp_call_function_many(tick_nohz_full_mask, nohz_full_kick_ipi, NULL,
> +			       false);

Breaking < 80 char lines is arguable although I'm not sure it still matters in 2014.

But I don't see much the point of the above change. I usually prefer when line contents
are a bit balanced. It may be a matter of taste I guess.

Re: [PATCH V2 01/19] tick: trivial cleanups

[Viresh Kumar]

On 23 April 2014 02:53, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Mon, Apr 21, 2014 at 03:24:57PM +0530, Viresh Kumar wrote:
>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>> index 6558b7a..9e9ddba 100644
>> --- a/kernel/time/tick-sched.c
>> +++ b/kernel/time/tick-sched.c
>> @@ -108,7 +108,6 @@ static ktime_t tick_init_jiffy_update(void)
>>       return period;
>>  }
>>
>> -
>>  static void tick_sched_do_timer(ktime_t now)
>>  {
>>       int cpu = smp_processor_id();
>> @@ -248,8 +247,8 @@ void tick_nohz_full_kick_all(void)
>>               return;
>>
>>       preempt_disable();
>> -     smp_call_function_many(tick_nohz_full_mask,
>> -                            nohz_full_kick_ipi, NULL, false);
>> +     smp_call_function_many(tick_nohz_full_mask, nohz_full_kick_ipi, NULL,
>> +                            false);
>
> Breaking < 80 char lines is arguable although I'm not sure it still matters in 2014.

I agree. In case we don't care anymore, checkpatch.pl must be fixed..

> But I don't see much the point of the above change. I usually prefer when line contents
> are a bit balanced. It may be a matter of taste I guess.

When I tried doing it, I though it might come in a single line, but
then it didn't.
The way I wrap things normally is I let 'vim' do it after 80 columns. And it
tries to fit max in a single line.. So this happened.

I can drop it if you want.. :)

[PATCH V2 02/19] tick: update doc style comments for 'struct tick_sched'

[Viresh Kumar]

Some fields of 'struct tick_sched' didn't have a description in the kernel doc
style comment present above its declaration. Add them.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 8c865fb..8cc804c 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -35,14 +35,19 @@ enum tick_nohz_mode {
  * struct tick_sched - sched tick emulation and no idle tick control/stats
  * @sched_timer:	hrtimer to schedule the periodic tick in high
  *			resolution mode
+ * @check_clocks:	tracks if clockevent device is recently changed
+ * @nohz_mode:		Current NOHZ mode
  * @last_tick:		Store the last tick expiry time when the tick
  *			timer is modified for nohz sleeps. This is necessary
  *			to resume the tick timer operation in the timeline
  *			when the CPU returns from nohz sleep.
+ * @inidle:		CPU is currently executing from within the idle loop
  * @tick_stopped:	Indicator that the idle tick has been stopped
  * @idle_jiffies:	jiffies at the entry to idle for idle time accounting
  * @idle_calls:		Total number of idle calls
  * @idle_sleeps:	Number of idle calls, where the sched tick was stopped
+ * @idle_active:	similar to inidle, but is reset when we get an interrupt
+ *			while being in idle
  * @idle_entrytime:	Time when the idle call was entered
  * @idle_waketime:	Time when the idle was interrupted
  * @idle_exittime:	Time when the idle state was left
@@ -50,7 +55,10 @@ enum tick_nohz_mode {
  * @iowait_sleeptime:	Sum of the time slept in idle with sched tick stopped,
  *			with IO outstanding
  * @sleep_length:	Duration of the current idle sleep
- * @do_timer_lst:	CPU was the last one doing do_timer before going idle
+ * @last_jiffies:	Last updated value of jiffies
+ * @next_jiffies:	Next jiffie for which timer is requested
+ * @idle_expires:	Time when we should come out of idle or next timer event
+ * @do_timer_last:	CPU was the last one doing do_timer before going idle
  */
 struct tick_sched {
 	struct hrtimer			sched_timer;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 03/19] tick: rearrange members of 'struct tick_sched'

[Viresh Kumar]

Rearrange members of 'struct tick_sched' to make it more readable. i.e. By
keeping all ktime_t members together, similarly 'unsigned long' and 'int' as
well. I couldn't figure out breaking any logical blocks here which we wanted to
keep together.

Change in footprints of this structure:
For x86: size reduced to 232 from 240 bytes
For ARM: no change

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 8cc804c..edabc7d 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -35,19 +35,17 @@ enum tick_nohz_mode {
  * struct tick_sched - sched tick emulation and no idle tick control/stats
  * @sched_timer:	hrtimer to schedule the periodic tick in high
  *			resolution mode
- * @check_clocks:	tracks if clockevent device is recently changed
  * @nohz_mode:		Current NOHZ mode
+ * @check_clocks:	tracks if clockevent device is recently changed
+ * @idle_jiffies:	jiffies at the entry to idle for idle time accounting
+ * @idle_calls:		Total number of idle calls
+ * @idle_sleeps:	Number of idle calls, where the sched tick was stopped
+ * @last_jiffies:	Last updated value of jiffies
+ * @next_jiffies:	Next jiffie for which timer is requested
  * @last_tick:		Store the last tick expiry time when the tick
  *			timer is modified for nohz sleeps. This is necessary
  *			to resume the tick timer operation in the timeline
  *			when the CPU returns from nohz sleep.
- * @inidle:		CPU is currently executing from within the idle loop
- * @tick_stopped:	Indicator that the idle tick has been stopped
- * @idle_jiffies:	jiffies at the entry to idle for idle time accounting
- * @idle_calls:		Total number of idle calls
- * @idle_sleeps:	Number of idle calls, where the sched tick was stopped
- * @idle_active:	similar to inidle, but is reset when we get an interrupt
- *			while being in idle
  * @idle_entrytime:	Time when the idle call was entered
  * @idle_waketime:	Time when the idle was interrupted
  * @idle_exittime:	Time when the idle state was left
@@ -55,31 +53,33 @@ enum tick_nohz_mode {
  * @iowait_sleeptime:	Sum of the time slept in idle with sched tick stopped,
  *			with IO outstanding
  * @sleep_length:	Duration of the current idle sleep
- * @last_jiffies:	Last updated value of jiffies
- * @next_jiffies:	Next jiffie for which timer is requested
  * @idle_expires:	Time when we should come out of idle or next timer event
+ * @inidle:		CPU is currently executing from within the idle loop
+ * @idle_active:	similar to inidle, but is reset when we get an interrupt
+ *			while being in idle
+ * @tick_stopped:	Indicator that the idle tick has been stopped
  * @do_timer_last:	CPU was the last one doing do_timer before going idle
  */
 struct tick_sched {
 	struct hrtimer			sched_timer;
-	unsigned long			check_clocks;
 	enum tick_nohz_mode		nohz_mode;
-	ktime_t				last_tick;
-	int				inidle;
-	int				tick_stopped;
+	unsigned long			check_clocks;
 	unsigned long			idle_jiffies;
 	unsigned long			idle_calls;
 	unsigned long			idle_sleeps;
-	int				idle_active;
+	unsigned long			last_jiffies;
+	unsigned long			next_jiffies;
+	ktime_t				last_tick;
 	ktime_t				idle_entrytime;
 	ktime_t				idle_waketime;
 	ktime_t				idle_exittime;
 	ktime_t				idle_sleeptime;
 	ktime_t				iowait_sleeptime;
 	ktime_t				sleep_length;
-	unsigned long			last_jiffies;
-	unsigned long			next_jiffies;
 	ktime_t				idle_expires;
+	int				inidle;
+	int				idle_active;
+	int				tick_stopped;
 	int				do_timer_last;
 };
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 04/19] tick: move declaration of 'tick_cpu_device' to tick.h

[Viresh Kumar]

'tick_cpu_device' isn't local to kernel/time/ directory as it is declared in
hrtimer.h as well. Move its declaration to tick.h.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/hrtimer.h     | 3 ---
 include/linux/tick.h        | 2 ++
 kernel/time/tick-internal.h | 1 -
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index e7a8d3f..a31f83e 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -332,9 +332,6 @@ extern ktime_t ktime_get_clocktai(void);
 extern ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot,
 					 ktime_t *offs_tai);
 
-DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
-
-
 /* Exported timer functions: */
 
 /* Initialize timers: */
diff --git a/include/linux/tick.h b/include/linux/tick.h
index edabc7d..1a9908a 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -85,6 +85,8 @@ struct tick_sched {
 
 extern void __init tick_init(void);
 extern int tick_is_oneshot_available(void);
+
+DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 extern struct tick_device *tick_get_device(int cpu);
 
 # ifdef CONFIG_HIGH_RES_TIMERS
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 855c513..57c1a76 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -13,7 +13,6 @@ extern seqlock_t jiffies_lock;
 #define TICK_DO_TIMER_NONE	-1
 #define TICK_DO_TIMER_BOOT	-2
 
-DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 extern ktime_t tick_next_period;
 extern ktime_t tick_period;
 extern int tick_do_timer_cpu __read_mostly;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 05/19] tick: move definition of tick_get_device() to tick.h

[Viresh Kumar]

There are multiple users of tick_get_device() which are currently using
&per_cpu(tick_cpu_device, cpu) directly. Would be better if we use
tick_get_device() consistently. Move definition of tick_get_device() to tick.h
and update others to use it.

This change reduced size of bzImage for x86 by 96 bytes.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h         |  5 ++++-
 kernel/time/clockevents.c    |  6 +++---
 kernel/time/tick-broadcast.c | 12 ++++++------
 kernel/time/tick-common.c    |  9 ++-------
 4 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 1a9908a..45e1331 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -87,7 +87,10 @@ extern void __init tick_init(void);
 extern int tick_is_oneshot_available(void);
 
 DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
-extern struct tick_device *tick_get_device(int cpu);
+static inline struct tick_device *tick_get_device(int cpu)
+{
+	return &per_cpu(tick_cpu_device, cpu);
+}
 
 # ifdef CONFIG_HIGH_RES_TIMERS
 extern int tick_init_highres(void);
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 2ba812bc..a3e9333 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -326,7 +326,7 @@ static int __clockevents_try_unbind(struct clock_event_device *ced, int cpu)
 		return 0;
 	}
 
-	return ced == per_cpu(tick_cpu_device, cpu).evtdev ? -EAGAIN : -EBUSY;
+	return ced == tick_get_device(cpu)->evtdev ? -EAGAIN : -EBUSY;
 }
 
 /*
@@ -675,7 +675,7 @@ static struct device tick_bc_dev = {
 static struct tick_device *tick_get_tick_dev(struct device *dev)
 {
 	return dev == &tick_bc_dev ? tick_get_broadcast_device() :
-		&per_cpu(tick_cpu_device, dev->id);
+		tick_get_device(dev->id);
 }
 
 static __init int tick_broadcast_init_sysfs(void)
@@ -689,7 +689,7 @@ static __init int tick_broadcast_init_sysfs(void)
 #else
 static struct tick_device *tick_get_tick_dev(struct device *dev)
 {
-	return &per_cpu(tick_cpu_device, dev->id);
+	return tick_get_device(dev->id);
 }
 static inline int tick_broadcast_init_sysfs(void) { return 0; }
 #endif
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index b1c7b21..6289680 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -262,7 +262,7 @@ static void tick_do_broadcast(struct cpumask *mask)
 	 */
 	if (cpumask_test_cpu(cpu, mask)) {
 		cpumask_clear_cpu(cpu, mask);
-		td = &per_cpu(tick_cpu_device, cpu);
+		td = tick_get_device(cpu);
 		td->evtdev->event_handler(td->evtdev);
 	}
 
@@ -273,7 +273,7 @@ static void tick_do_broadcast(struct cpumask *mask)
 		 * one of the first device. This works as long as we have this
 		 * misfeature only on x86 (lapic)
 		 */
-		td = &per_cpu(tick_cpu_device, cpumask_first(mask));
+		td = tick_get_device(cpumask_first(mask));
 		td->evtdev->broadcast(mask);
 	}
 }
@@ -337,7 +337,7 @@ static void tick_do_broadcast_on_off(unsigned long *reason)
 	raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
 
 	cpu = smp_processor_id();
-	td = &per_cpu(tick_cpu_device, cpu);
+	td = tick_get_device(cpu);
 	dev = td->evtdev;
 	bc = tick_broadcast_device.evtdev;
 
@@ -581,7 +581,7 @@ again:
 	now = ktime_get();
 	/* Find all expired events */
 	for_each_cpu(cpu, tick_broadcast_oneshot_mask) {
-		td = &per_cpu(tick_cpu_device, cpu);
+		td = tick_get_device(cpu);
 		if (td->evtdev->next_event.tv64 <= now.tv64) {
 			cpumask_set_cpu(cpu, tmpmask);
 			/*
@@ -700,7 +700,7 @@ int tick_broadcast_oneshot_control(unsigned long reason)
 	 * idle code, so we can't be moved away.
 	 */
 	cpu = smp_processor_id();
-	td = &per_cpu(tick_cpu_device, cpu);
+	td = tick_get_device(cpu);
 	dev = td->evtdev;
 
 	if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
@@ -824,7 +824,7 @@ static void tick_broadcast_init_next_event(struct cpumask *mask,
 	int cpu;
 
 	for_each_cpu(cpu, mask) {
-		td = &per_cpu(tick_cpu_device, cpu);
+		td = tick_get_device(cpu);
 		if (td->evtdev)
 			td->evtdev->next_event = expires;
 	}
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 0a0608e..3f3aa86 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -53,11 +53,6 @@ int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT;
 /*
  * Debugging: see timer_list.c
  */
-struct tick_device *tick_get_device(int cpu)
-{
-	return &per_cpu(tick_cpu_device, cpu);
-}
-
 /**
  * tick_is_oneshot_available - check for a oneshot capable event device
  */
@@ -296,7 +291,7 @@ void tick_check_new_device(struct clock_event_device *newdev)
 	if (!cpumask_test_cpu(cpu, newdev->cpumask))
 		goto out_bc;
 
-	td = &per_cpu(tick_cpu_device, cpu);
+	td = tick_get_device(cpu);
 	curdev = td->evtdev;
 
 	/* cpu local device ? */
@@ -356,7 +351,7 @@ void tick_handover_do_timer(int *cpup)
  */
 void tick_shutdown(unsigned int *cpup)
 {
-	struct tick_device *td = &per_cpu(tick_cpu_device, *cpup);
+	struct tick_device *td = tick_get_device(*cpup);
 	struct clock_event_device *dev = td->evtdev;
 
 	td->mode = TICKDEV_MODE_PERIODIC;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 06/19] tick: create tick_get_cpu_device() to get tick_cpu_device on this cpu

[Viresh Kumar]

We are using &__get_cpu_var(tick_cpu_device) at several places and would be
better if we create tick_get_cpu_device() and use it instead.

This also converts tick_get_device(smp_processor_id()) to use
tick_get_cpu_device().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h         |  5 +++++
 kernel/hrtimer.c             |  2 +-
 kernel/time/tick-broadcast.c |  8 ++++----
 kernel/time/tick-common.c    | 10 +++++-----
 kernel/time/tick-oneshot.c   |  8 ++++----
 kernel/time/tick-sched.c     |  2 +-
 6 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 45e1331..98065e5 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -92,6 +92,11 @@ static inline struct tick_device *tick_get_device(int cpu)
 	return &per_cpu(tick_cpu_device, cpu);
 }
 
+static inline struct tick_device *tick_get_cpu_device(void)
+{
+	return &__get_cpu_var(tick_cpu_device);
+}
+
 # ifdef CONFIG_HIGH_RES_TIMERS
 extern int tick_init_highres(void);
 extern int tick_program_event(ktime_t expires, int force);
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index d55092c..393f422 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1403,7 +1403,7 @@ static void __hrtimer_peek_ahead_timers(void)
 	if (!hrtimer_hres_active())
 		return;
 
-	td = &__get_cpu_var(tick_cpu_device);
+	td = tick_get_cpu_device();
 	if (td && td->evtdev)
 		hrtimer_interrupt(td->evtdev);
 }
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 6289680..bf289cd 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -235,7 +235,7 @@ int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu)
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 int tick_receive_broadcast(void)
 {
-	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
+	struct tick_device *td = tick_get_cpu_device();
 	struct clock_event_device *evt = td->evtdev;
 
 	if (!evt)
@@ -337,7 +337,7 @@ static void tick_do_broadcast_on_off(unsigned long *reason)
 	raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
 
 	cpu = smp_processor_id();
-	td = tick_get_device(cpu);
+	td = tick_get_cpu_device();
 	dev = td->evtdev;
 	bc = tick_broadcast_device.evtdev;
 
@@ -550,7 +550,7 @@ int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
 void tick_check_oneshot_broadcast_this_cpu(void)
 {
 	if (cpumask_test_cpu(smp_processor_id(), tick_broadcast_oneshot_mask)) {
-		struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+		struct tick_device *td = tick_get_cpu_device();
 
 		/*
 		 * We might be in the middle of switching over from
@@ -700,7 +700,7 @@ int tick_broadcast_oneshot_control(unsigned long reason)
 	 * idle code, so we can't be moved away.
 	 */
 	cpu = smp_processor_id();
-	td = tick_get_device(cpu);
+	td = tick_get_cpu_device();
 	dev = td->evtdev;
 
 	if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 3f3aa86..1e2c96e 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -58,7 +58,7 @@ int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT;
  */
 int tick_is_oneshot_available(void)
 {
-	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
+	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
 
 	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT))
 		return 0;
@@ -219,7 +219,7 @@ static void tick_setup_device(struct tick_device *td,
 
 void tick_install_replacement(struct clock_event_device *newdev)
 {
-	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	struct tick_device *td = tick_get_cpu_device();
 	int cpu = smp_processor_id();
 
 	clockevents_exchange_device(td->evtdev, newdev);
@@ -291,7 +291,7 @@ void tick_check_new_device(struct clock_event_device *newdev)
 	if (!cpumask_test_cpu(cpu, newdev->cpumask))
 		goto out_bc;
 
-	td = tick_get_device(cpu);
+	td = tick_get_cpu_device();
 	curdev = td->evtdev;
 
 	/* cpu local device ? */
@@ -369,14 +369,14 @@ void tick_shutdown(unsigned int *cpup)
 
 void tick_suspend(void)
 {
-	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	struct tick_device *td = tick_get_cpu_device();
 
 	clockevents_shutdown(td->evtdev);
 }
 
 void tick_resume(void)
 {
-	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	struct tick_device *td = tick_get_cpu_device();
 	int broadcast = tick_resume_broadcast();
 
 	clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_RESUME);
diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
index 5fe86a7..7089dea 100644
--- a/kernel/time/tick-oneshot.c
+++ b/kernel/time/tick-oneshot.c
@@ -26,7 +26,7 @@
  */
 int tick_program_event(ktime_t expires, int force)
 {
-	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
+	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
 
 	return clockevents_program_event(dev, expires, force);
 }
@@ -36,7 +36,7 @@ int tick_program_event(ktime_t expires, int force)
  */
 void tick_resume_oneshot(void)
 {
-	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
+	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
 
 	clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT);
 	clockevents_program_event(dev, ktime_get(), true);
@@ -59,7 +59,7 @@ void tick_setup_oneshot(struct clock_event_device *newdev,
  */
 int tick_switch_to_oneshot(void (*handler)(struct clock_event_device *))
 {
-	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	struct tick_device *td = tick_get_cpu_device();
 	struct clock_event_device *dev = td->evtdev;
 
 	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT) ||
@@ -96,7 +96,7 @@ int tick_oneshot_mode_active(void)
 	int ret;
 
 	local_irq_save(flags);
-	ret = __this_cpu_read(tick_cpu_device.mode) == TICKDEV_MODE_ONESHOT;
+	ret = tick_get_cpu_device()->mode == TICKDEV_MODE_ONESHOT;
 	local_irq_restore(flags);
 
 	return ret;
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 9e9ddba..5f27c71 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -536,7 +536,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
 	unsigned long seq, last_jiffies, next_jiffies, delta_jiffies;
 	ktime_t last_update, expires, ret = { .tv64 = 0 };
 	unsigned long rcu_delta_jiffies;
-	struct clock_event_device *dev = __get_cpu_var(tick_cpu_device).evtdev;
+	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
 	u64 time_delta;
 
 	time_delta = timekeeping_max_deferment();
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 07/19] tick: initialize variables during their definitions

[Viresh Kumar]

We don't have to assign values to variables in separate lines when we can do
that during their initialization. Move assignments of few variables to their
definitions. This makes code smaller and more readable.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/tick-broadcast.c | 20 ++++++--------------
 kernel/time/tick-common.c    | 10 +++-------
 kernel/time/tick-sched.c     |  6 ++----
 3 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index bf289cd..7802020 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -329,16 +329,12 @@ unlock:
  */
 static void tick_do_broadcast_on_off(unsigned long *reason)
 {
-	struct clock_event_device *bc, *dev;
-	struct tick_device *td;
+	struct tick_device *td = tick_get_cpu_device();
+	struct clock_event_device *bc, *dev = td->evtdev;
+	int cpu = smp_processor_id(), bc_stopped;
 	unsigned long flags;
-	int cpu, bc_stopped;
 
 	raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
-
-	cpu = smp_processor_id();
-	td = tick_get_cpu_device();
-	dev = td->evtdev;
 	bc = tick_broadcast_device.evtdev;
 
 	/*
@@ -682,11 +678,11 @@ static void broadcast_move_bc(int deadcpu)
  */
 int tick_broadcast_oneshot_control(unsigned long reason)
 {
-	struct clock_event_device *bc, *dev;
-	struct tick_device *td;
+	struct tick_device *td = tick_get_cpu_device();
+	struct clock_event_device *bc, *dev = td->evtdev;
+	int cpu = smp_processor_id(), ret = 0;
 	unsigned long flags;
 	ktime_t now;
-	int cpu, ret = 0;
 
 	/*
 	 * Periodic mode does not care about the enter/exit of power
@@ -699,10 +695,6 @@ int tick_broadcast_oneshot_control(unsigned long reason)
 	 * We are called with preemtion disabled from the depth of the
 	 * idle code, so we can't be moved away.
 	 */
-	cpu = smp_processor_id();
-	td = tick_get_cpu_device();
-	dev = td->evtdev;
-
 	if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
 		return 0;
 
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 1e2c96e..4d45e08 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -283,17 +283,13 @@ bool tick_check_replacement(struct clock_event_device *curdev,
  */
 void tick_check_new_device(struct clock_event_device *newdev)
 {
-	struct clock_event_device *curdev;
-	struct tick_device *td;
-	int cpu;
+	struct tick_device *td = tick_get_cpu_device();
+	struct clock_event_device *curdev = td->evtdev;
+	int cpu = smp_processor_id();
 
-	cpu = smp_processor_id();
 	if (!cpumask_test_cpu(cpu, newdev->cpumask))
 		goto out_bc;
 
-	td = tick_get_cpu_device();
-	curdev = td->evtdev;
-
 	/* cpu local device ? */
 	if (!tick_check_percpu(curdev, newdev, cpu))
 		goto out_bc;
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 5f27c71..71f64ee 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -277,7 +277,7 @@ out:
 /* Parse the boot-time nohz CPU list from the kernel parameters. */
 static int __init tick_nohz_full_setup(char *str)
 {
-	int cpu;
+	int cpu = smp_processor_id();
 
 	alloc_bootmem_cpumask_var(&tick_nohz_full_mask);
 	if (cpulist_parse(str, tick_nohz_full_mask) < 0) {
@@ -285,7 +285,6 @@ static int __init tick_nohz_full_setup(char *str)
 		return 1;
 	}
 
-	cpu = smp_processor_id();
 	if (cpumask_test_cpu(cpu, tick_nohz_full_mask)) {
 		pr_warning("NO_HZ: Clearing %d from nohz_full range for timekeeping\n",
 			   cpu);
@@ -790,7 +789,7 @@ static void __tick_nohz_idle_enter(struct tick_sched *ts)
  */
 void tick_nohz_idle_enter(void)
 {
-	struct tick_sched *ts;
+	struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
 
 	WARN_ON_ONCE(irqs_disabled());
 
@@ -804,7 +803,6 @@ void tick_nohz_idle_enter(void)
 
 	local_irq_disable();
 
-	ts = &__get_cpu_var(tick_cpu_sched);
 	ts->inidle = 1;
 	__tick_nohz_idle_enter(ts);
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 08/19] tick-oneshot: move tick_is_oneshot_available() to tick-oneshot.c

[Viresh Kumar]

This is tick-oneshot specific routine and hence must be defined in
tick-oneshot.c.

Also, as it isn't used outside kernel/time/, move its declaration to
tick-internal.h.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h        |  1 -
 kernel/time/tick-common.c   | 14 --------------
 kernel/time/tick-internal.h |  2 ++
 kernel/time/tick-oneshot.c  | 14 ++++++++++++++
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 98065e5..b8ee6f4 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -84,7 +84,6 @@ struct tick_sched {
 };
 
 extern void __init tick_init(void);
-extern int tick_is_oneshot_available(void);
 
 DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 static inline struct tick_device *tick_get_device(int cpu)
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 4d45e08..64897d3 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -53,20 +53,6 @@ int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT;
 /*
  * Debugging: see timer_list.c
  */
-/**
- * tick_is_oneshot_available - check for a oneshot capable event device
- */
-int tick_is_oneshot_available(void)
-{
-	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
-
-	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT))
-		return 0;
-	if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
-		return 1;
-	return tick_broadcast_oneshot_available();
-}
-
 /*
  * Periodic tick
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 57c1a76..09f4307 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -42,6 +42,7 @@ extern void tick_setup_oneshot(struct clock_event_device *newdev,
 extern int tick_program_event(ktime_t expires, int force);
 extern void tick_oneshot_notify(void);
 extern int tick_switch_to_oneshot(void (*handler)(struct clock_event_device *));
+extern int tick_is_oneshot_available(void);
 extern void tick_resume_oneshot(void);
 # ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
@@ -96,6 +97,7 @@ static inline int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
 	return 0;
 }
 static inline int tick_broadcast_oneshot_active(void) { return 0; }
+static inline int tick_is_oneshot_available(void) { return 0; };
 static inline bool tick_broadcast_oneshot_available(void) { return false; }
 #endif /* !TICK_ONESHOT */
 
diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
index 7089dea..aaf60a9 100644
--- a/kernel/time/tick-oneshot.c
+++ b/kernel/time/tick-oneshot.c
@@ -102,6 +102,20 @@ int tick_oneshot_mode_active(void)
 	return ret;
 }
 
+/**
+ * tick_is_oneshot_available - check for a oneshot capable event device
+ */
+int tick_is_oneshot_available(void)
+{
+	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
+
+	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT))
+		return 0;
+	if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
+		return 1;
+	return tick_broadcast_oneshot_available();
+}
+
 #ifdef CONFIG_HIGH_RES_TIMERS
 /**
  * tick_init_highres - switch to high resolution mode
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 09/19] tick-oneshot: remove tick_resume_oneshot()

[Viresh Kumar]

tick_resume_oneshot() and tick_setup_oneshot() have almost same implementation.
So, we can remove tick_resume_oneshot() and use tick_setup_oneshot() in its
place.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/tick-common.c   |  8 +++++---
 kernel/time/tick-internal.h |  5 -----
 kernel/time/tick-oneshot.c  | 11 -----------
 3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 64897d3..965e9c3 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -359,15 +359,17 @@ void tick_suspend(void)
 void tick_resume(void)
 {
 	struct tick_device *td = tick_get_cpu_device();
+	struct clock_event_device *dev = td->evtdev;
 	int broadcast = tick_resume_broadcast();
 
-	clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_RESUME);
+	clockevents_set_mode(dev, CLOCK_EVT_MODE_RESUME);
 
 	if (!broadcast) {
 		if (td->mode == TICKDEV_MODE_PERIODIC)
-			tick_setup_periodic(td->evtdev, 0);
+			tick_setup_periodic(dev, 0);
 		else
-			tick_resume_oneshot();
+			tick_setup_oneshot(dev, dev->event_handler,
+					   ktime_get());
 	}
 }
 
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 09f4307..65f080a 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -43,7 +43,6 @@ extern int tick_program_event(ktime_t expires, int force);
 extern void tick_oneshot_notify(void);
 extern int tick_switch_to_oneshot(void (*handler)(struct clock_event_device *));
 extern int tick_is_oneshot_available(void);
-extern void tick_resume_oneshot(void);
 # ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
 extern int tick_broadcast_oneshot_control(unsigned long reason);
@@ -74,10 +73,6 @@ void tick_setup_oneshot(struct clock_event_device *newdev,
 {
 	BUG();
 }
-static inline void tick_resume_oneshot(void)
-{
-	BUG();
-}
 static inline int tick_program_event(ktime_t expires, int force)
 {
 	return 0;
diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
index aaf60a9..a07fc3d 100644
--- a/kernel/time/tick-oneshot.c
+++ b/kernel/time/tick-oneshot.c
@@ -32,17 +32,6 @@ int tick_program_event(ktime_t expires, int force)
 }
 
 /**
- * tick_resume_onshot - resume oneshot mode
- */
-void tick_resume_oneshot(void)
-{
-	struct clock_event_device *dev = tick_get_cpu_device()->evtdev;
-
-	clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT);
-	clockevents_program_event(dev, ktime_get(), true);
-}
-
-/**
  * tick_setup_oneshot - setup the event device for oneshot mode (hres or nohz)
  */
 void tick_setup_oneshot(struct clock_event_device *newdev,
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 10/19] tick-common: call tick_check_percpu() from tick_check_preferred()

[Viresh Kumar]

tick_check_percpu() and tick_check_preferred() are called from two places and in
exactly same order. So, would make sense to call tick_check_percpu() from
tick_check_preferred() instead, so that their caller can just call
tick_check_preferred().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/tick-common.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 965e9c3..9d3106b 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -233,6 +233,9 @@ static bool tick_check_percpu(struct clock_event_device *curdev,
 static bool tick_check_preferred(struct clock_event_device *curdev,
 				 struct clock_event_device *newdev)
 {
+	if (!tick_check_percpu(curdev, newdev, smp_processor_id()))
+		return false;
+
 	/* Prefer oneshot capable device */
 	if (!(newdev->features & CLOCK_EVT_FEAT_ONESHOT)) {
 		if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT))
@@ -257,9 +260,6 @@ static bool tick_check_preferred(struct clock_event_device *curdev,
 bool tick_check_replacement(struct clock_event_device *curdev,
 			    struct clock_event_device *newdev)
 {
-	if (!tick_check_percpu(curdev, newdev, smp_processor_id()))
-		return false;
-
 	return tick_check_preferred(curdev, newdev);
 }
 
@@ -276,10 +276,6 @@ void tick_check_new_device(struct clock_event_device *newdev)
 	if (!cpumask_test_cpu(cpu, newdev->cpumask))
 		goto out_bc;
 
-	/* cpu local device ? */
-	if (!tick_check_percpu(curdev, newdev, cpu))
-		goto out_bc;
-
 	/* Preference decision */
 	if (!tick_check_preferred(curdev, newdev))
 		goto out_bc;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 11/19] tick-common: remove tick_check_replacement()

[Viresh Kumar]

tick_check_replacement() is now nothing more than a dummy wrapper over
tick_check_preferred() and can be removed.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/clockevents.c   |  2 +-
 kernel/time/tick-common.c   | 16 +++++-----------
 kernel/time/tick-internal.h |  2 +-
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index a3e9333..15ff71d 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -298,7 +298,7 @@ static int clockevents_replace(struct clock_event_device *ced)
 		if (dev == ced || dev->mode != CLOCK_EVT_MODE_UNUSED)
 			continue;
 
-		if (!tick_check_replacement(newdev, dev))
+		if (!tick_check_preferred(newdev, dev))
 			continue;
 
 		if (!try_module_get(dev->owner))
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 9d3106b..896c992 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -230,7 +230,11 @@ static bool tick_check_percpu(struct clock_event_device *curdev,
 	return true;
 }
 
-static bool tick_check_preferred(struct clock_event_device *curdev,
+/*
+ * Check whether the new device is a better fit than curdev. curdev
+ * can be NULL !
+ */
+bool tick_check_preferred(struct clock_event_device *curdev,
 				 struct clock_event_device *newdev)
 {
 	if (!tick_check_percpu(curdev, newdev, smp_processor_id()))
@@ -254,16 +258,6 @@ static bool tick_check_preferred(struct clock_event_device *curdev,
 }
 
 /*
- * Check whether the new device is a better fit than curdev. curdev
- * can be NULL !
- */
-bool tick_check_replacement(struct clock_event_device *curdev,
-			    struct clock_event_device *newdev)
-{
-	return tick_check_preferred(curdev, newdev);
-}
-
-/*
  * Check, if the new registered device should be used. Called with
  * clockevents_lock held and interrupts disabled.
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 65f080a..8c6e85d 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -24,7 +24,7 @@ extern void tick_handover_do_timer(int *cpup);
 extern void tick_shutdown(unsigned int *cpup);
 extern void tick_suspend(void);
 extern void tick_resume(void);
-extern bool tick_check_replacement(struct clock_event_device *curdev,
+extern bool tick_check_preferred(struct clock_event_device *curdev,
 				   struct clock_event_device *newdev);
 extern void tick_install_replacement(struct clock_event_device *dev);
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 12/19] tick-common: don't pass 'cpu' & 'cpumask' to tick_setup_device()

[Viresh Kumar]

tick_setup_device() is always passed cpumask of 'cpu' and 'cpu' is always
obtained from smp_processor_id(). Remove these two parameters and obtain them in
tick_setup_device() instead.

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

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 896c992..85ee9fd 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -145,9 +145,10 @@ void tick_setup_periodic(struct clock_event_device *dev, int broadcast)
  * Setup the tick device
  */
 static void tick_setup_device(struct tick_device *td,
-			      struct clock_event_device *newdev, int cpu,
-			      const struct cpumask *cpumask)
+			      struct clock_event_device *newdev)
 {
+	int cpu = smp_processor_id();
+	const struct cpumask *cpumask = cpumask_of(cpu);
 	ktime_t next_event;
 	void (*handler)(struct clock_event_device *) = NULL;
 
@@ -206,10 +207,9 @@ static void tick_setup_device(struct tick_device *td,
 void tick_install_replacement(struct clock_event_device *newdev)
 {
 	struct tick_device *td = tick_get_cpu_device();
-	int cpu = smp_processor_id();
 
 	clockevents_exchange_device(td->evtdev, newdev);
-	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
+	tick_setup_device(td, newdev);
 	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
 		tick_oneshot_notify();
 }
@@ -287,7 +287,7 @@ void tick_check_new_device(struct clock_event_device *newdev)
 		curdev = NULL;
 	}
 	clockevents_exchange_device(curdev, newdev);
-	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
+	tick_setup_device(td, newdev);
 	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
 		tick_oneshot_notify();
 	return;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 13/19] tick-common: remove local variable 'broadcast' from tick_resume()

[Viresh Kumar]

'broadcast' is used just once and we can use tick_resume_broadcast() directly
instead. Also it changes the code a bit to get rid of extra indentation level
which was forcing us to break function calls into two lines.

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

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 85ee9fd..2dc6822 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -350,17 +350,16 @@ void tick_resume(void)
 {
 	struct tick_device *td = tick_get_cpu_device();
 	struct clock_event_device *dev = td->evtdev;
-	int broadcast = tick_resume_broadcast();
 
 	clockevents_set_mode(dev, CLOCK_EVT_MODE_RESUME);
 
-	if (!broadcast) {
-		if (td->mode == TICKDEV_MODE_PERIODIC)
-			tick_setup_periodic(dev, 0);
-		else
-			tick_setup_oneshot(dev, dev->event_handler,
-					   ktime_get());
-	}
+	if (tick_resume_broadcast())
+		return;
+
+	if (td->mode == TICKDEV_MODE_PERIODIC)
+		tick_setup_periodic(dev, 0);
+	else
+		tick_setup_oneshot(dev, dev->event_handler, ktime_get());
 }
 
 /**
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 14/19] tick-sched: add comment about 'idle_active' in tick_nohz_idle_exit()

[Viresh Kumar]

The sequence of calls for dynticks CPUs is a bit confusing. Add a comment in
tick_nohz_idle_exit() to mention it clearly. All information required is in
commit and this conversation with Frederic.

https://lkml.org/lkml/2014/4/10/355

Suggested-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/tick-sched.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 71f64ee..c3aed50 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -922,6 +922,22 @@ void tick_nohz_idle_exit(void)
 
 	ts->inidle = 0;
 
+	/*
+	 * Can idle_active be false here?
+	 * Ideally this would be the sequence of calls:
+	 * - tick_nohz_idle_enter(), i.e. idle_active = true;
+	 * - local_irq_disable()
+	 * - IDLE
+	 * - wake up due to IPI or other interrupt
+	 * - local_irq_enable()
+	 * - tick_nohz_irq_enter(), i.e. idle_active = false;
+	 * - tick_nohz_irq_exit(), i.e. idle_active = true; This is not called
+	 *   in case of IPI's as need_resched() will prevent that in
+	 *   tick_irq_exit(), as we don't need to account any more for idle time
+	 *   or try to enter dyntics mode (We are going to exit idle state).
+	 *
+	 * - tick_nohz_idle_exit()
+	 */
 	if (ts->idle_active || ts->tick_stopped)
 		now = ktime_get();
 
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V2 14/19] tick-sched: add comment about 'idle_active' in tick_nohz_idle_exit()

[Frederic Weisbecker]

On Mon, Apr 21, 2014 at 03:25:10PM +0530, Viresh Kumar wrote:
> The sequence of calls for dynticks CPUs is a bit confusing. Add a comment in
> tick_nohz_idle_exit() to mention it clearly. All information required is in
> commit and this conversation with Frederic.
> 
> https://lkml.org/lkml/2014/4/10/355
> 
> Suggested-by: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  kernel/time/tick-sched.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 71f64ee..c3aed50 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -922,6 +922,22 @@ void tick_nohz_idle_exit(void)
>  
>  	ts->inidle = 0;
>  
> +	/*
> +	 * Can idle_active be false here?
> +	 * Ideally this would be the sequence of calls:
> +	 * - tick_nohz_idle_enter(), i.e. idle_active = true;
> +	 * - local_irq_disable()
> +	 * - IDLE
> +	 * - wake up due to IPI or other interrupt
> +	 * - local_irq_enable()
> +	 * - tick_nohz_irq_enter(), i.e. idle_active = false;
> +	 * - tick_nohz_irq_exit(), i.e. idle_active = true; This is not called
> +	 *   in case of IPI's as need_resched() will prevent that in
> +	 *   tick_irq_exit(), as we don't need to account any more for idle time
> +	 *   or try to enter dyntics mode (We are going to exit idle state).
> +	 *
> +	 * - tick_nohz_idle_exit()
> +	 */
>  	if (ts->idle_active || ts->tick_stopped)
>  		now = ktime_get();

It's still over-detailed. Much of the above is easily deduced after common review. OTOH
I proposed to summarize there: https://lkml.org/lkml/2014/4/11/334
The below disambiguates it a bit further.

Now it's eventually getting as big as your comment ;-)


          /*
           * ts->idle_active drives the idle time which typically elapses in the idle loop
           * but breaks on IRQs interrupting idle loop.
           *
           * Hence ts->idle_active can be 1 here if we exit the idle loop without the help of
           * an IRQ. OTOH it can be 0 on idle exit if a wake up IPI pulled the CPU out of
           * the idle loop. Since we know that we'll be exiting the idle task after the wake
           * up IPI, all the pending idle sleep time is flushed on irq entry and no more is
           * accounted further thanks to the need_resched() check on irq_exit().
           */

Thanks.

Re: [PATCH V2 14/19] tick-sched: add comment about 'idle_active' in tick_nohz_idle_exit()

[viresh kumar]

On Tuesday 22 April 2014 04:50 AM, Frederic Weisbecker wrote:
> It's still over-detailed. Much of the above is easily deduced after common review. OTOH
> I proposed to summarize there: https://lkml.org/lkml/2014/4/11/334
> The below disambiguates it a bit further.

Hmm.. Something broke for sure in my repo. I do remember updating this patch with your
comments and something went wrong while playing with patches.

Sorry for that. Fixed my repo now.

> Now it's eventually getting as big as your comment ;-)
> 
> 
>           /*
>            * ts->idle_active drives the idle time which typically elapses in the idle loop
>            * but breaks on IRQs interrupting idle loop.
>            *
>            * Hence ts->idle_active can be 1 here if we exit the idle loop without the help of
>            * an IRQ. OTOH it can be 0 on idle exit if a wake up IPI pulled the CPU out of
>            * the idle loop. Since we know that we'll be exiting the idle task after the wake
>            * up IPI, all the pending idle sleep time is flushed on irq entry and no more is
>            * accounted further thanks to the need_resched() check on irq_exit().
>            */

@Thomas: Please consider this patch instead:

Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Mon Apr 21 15:25:10 2014 +0530

    tick-sched: add comment about 'idle_active' in tick_nohz_idle_exit()

    The sequence of calls for dynticks CPUs is a bit confusing. Add a comment in
    tick_nohz_idle_exit() to mention it clearly. All information required is in
    commit and this conversation with Frederic.

    https://lkml.org/lkml/2014/4/10/355

    Suggested-by: Frederic Weisbecker <fweisbec@gmail.com>
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/tick-sched.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 71f64ee..b2f024f 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -922,6 +922,17 @@ void tick_nohz_idle_exit(void)

        ts->inidle = 0;

+       /*
+        * ts->idle_active drives the idle time which typically elapses in the
+        * idle loop but breaks on IRQs interrupting idle loop.
+        *
+        * Hence ts->idle_active can be 1 here if we exit the idle loop without
+        * the help of an IRQ. OTOH it can be 0 on idle exit if a wake up IPI
+        * pulled the CPU out of the idle loop. Since we know that we'll be
+        * exiting the idle task after the wake up IPI, all the pending idle
+        * sleep time is flushed on irq entry and no more is accounted further
+        * thanks to the need_resched() check on irq_exit().
+        */
        if (ts->idle_active || ts->tick_stopped)
                now = ktime_get();

[PATCH V2 15/19] tick-sched: define 'delta' inside 'if' block in update_ts_time_stats()

[Viresh Kumar]

'delta' is used only inside the 'if' block and must be accessible within that.
So, move its definition inside the 'if' block.

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

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index c3aed50..8e75c13 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -412,10 +412,9 @@ static void
 update_ts_time_stats(int cpu, struct tick_sched *ts, ktime_t now,
 		     u64 *last_update_time)
 {
-	ktime_t delta;
-
 	if (ts->idle_active) {
-		delta = ktime_sub(now, ts->idle_entrytime);
+		ktime_t delta = ktime_sub(now, ts->idle_entrytime);
+
 		if (nr_iowait_cpu(cpu) > 0)
 			ts->iowait_sleeptime = ktime_add(ts->iowait_sleeptime,
 							 delta);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 16/19] tick-sched: remove parameters to {__}tick_nohz_task_switch() routines

[Viresh Kumar]

tick_nohz_task_switch() and __tick_nohz_task_switch() routines get task_struct
passed to them (always for the 'current' task), but they never use it. Remove
it.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/tick.h     | 8 ++++----
 kernel/sched/core.c      | 2 +-
 kernel/time/tick-sched.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index b8ee6f4..f538a4d 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -201,7 +201,7 @@ extern void tick_nohz_init(void);
 extern void __tick_nohz_full_check(void);
 extern void tick_nohz_full_kick(void);
 extern void tick_nohz_full_kick_all(void);
-extern void __tick_nohz_task_switch(struct task_struct *tsk);
+extern void __tick_nohz_task_switch(void);
 #else
 static inline void tick_nohz_init(void) { }
 static inline bool tick_nohz_full_enabled(void) { return false; }
@@ -209,7 +209,7 @@ static inline bool tick_nohz_full_cpu(int cpu) { return false; }
 static inline void __tick_nohz_full_check(void) { }
 static inline void tick_nohz_full_kick(void) { }
 static inline void tick_nohz_full_kick_all(void) { }
-static inline void __tick_nohz_task_switch(struct task_struct *tsk) { }
+static inline void __tick_nohz_task_switch(void) { }
 #endif
 
 static inline void tick_nohz_full_check(void)
@@ -218,10 +218,10 @@ static inline void tick_nohz_full_check(void)
 		__tick_nohz_full_check();
 }
 
-static inline void tick_nohz_task_switch(struct task_struct *tsk)
+static inline void tick_nohz_task_switch(void)
 {
 	if (tick_nohz_full_enabled())
-		__tick_nohz_task_switch(tsk);
+		__tick_nohz_task_switch();
 }
 
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3a4bb63..7cfe9d5a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2157,7 +2157,7 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
 		put_task_struct(prev);
 	}
 
-	tick_nohz_task_switch(current);
+	tick_nohz_task_switch();
 }
 
 #ifdef CONFIG_SMP
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 8e75c13..2360e7a 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -258,7 +258,7 @@ void tick_nohz_full_kick_all(void)
  * It might need the tick due to per task/process properties:
  * perf events, posix cpu timers, ...
  */
-void __tick_nohz_task_switch(struct task_struct *tsk)
+void __tick_nohz_task_switch(void)
 {
 	unsigned long flags;
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 17/19] tick-sched: remove local variable 'now' from tick_setup_sched_timer()

[Viresh Kumar]

Local variable 'now' is used at only one place and so can be replaced by
ktime_get() instead.

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

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 2360e7a..4a94412 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1120,7 +1120,6 @@ early_param("skew_tick", skew_tick);
 void tick_setup_sched_timer(void)
 {
 	struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
-	ktime_t now = ktime_get();
 
 	/*
 	 * Emulate tick processing via per-CPU hrtimers:
@@ -1140,13 +1139,12 @@ void tick_setup_sched_timer(void)
 	}
 
 	for (;;) {
-		hrtimer_forward(&ts->sched_timer, now, tick_period);
+		hrtimer_forward(&ts->sched_timer, ktime_get(), tick_period);
 		hrtimer_start_expires(&ts->sched_timer,
 				      HRTIMER_MODE_ABS_PINNED);
 		/* Check, if the timer was already in the past */
 		if (hrtimer_active(&ts->sched_timer))
 			break;
-		now = ktime_get();
 	}
 
 #ifdef CONFIG_NO_HZ_COMMON
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 18/19] tick-sched: invert parameter of tick_check_oneshot_change()

[Viresh Kumar]

There is only one caller of tick_check_oneshot_change(), i.e.
hrtimer_run_pending(). hrtimer_run_pending() calls this routine after doing a
logical NOT (!) of its parameter and then tick_check_oneshot_change() also uses
its parameter after doing logical NOT (!) of it.

It would be more efficient and readable if we can just invert the meaning of
this parameter. It is called hres_enabled instead of allow_nohz now.

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

diff --git a/include/linux/tick.h b/include/linux/tick.h
index f538a4d..1065a51 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -120,7 +120,7 @@ extern struct cpumask *tick_get_broadcast_oneshot_mask(void);
 
 # ifdef CONFIG_TICK_ONESHOT
 extern void tick_clock_notify(void);
-extern int tick_check_oneshot_change(int allow_nohz);
+extern int tick_check_oneshot_change(int hres_enabled);
 extern struct tick_sched *tick_get_tick_sched(int cpu);
 extern void tick_irq_enter(void);
 extern int tick_oneshot_mode_active(void);
@@ -129,7 +129,7 @@ extern int tick_oneshot_mode_active(void);
 #  endif
 # else
 static inline void tick_clock_notify(void) { }
-static inline int tick_check_oneshot_change(int allow_nohz) { return 0; }
+static inline int tick_check_oneshot_change(int hres_enabled) { return 0; }
 static inline void tick_irq_enter(void) { }
 static inline int tick_oneshot_mode_active(void) { return 0; }
 # endif
@@ -138,7 +138,7 @@ static inline int tick_oneshot_mode_active(void) { return 0; }
 static inline void tick_init(void) { }
 static inline void tick_cancel_sched_timer(int cpu) { }
 static inline void tick_clock_notify(void) { }
-static inline int tick_check_oneshot_change(int allow_nohz) { return 0; }
+static inline int tick_check_oneshot_change(int hres_enabled) { return 0; }
 static inline void tick_irq_enter(void) { }
 static inline int tick_oneshot_mode_active(void) { return 0; }
 #endif /* !CONFIG_GENERIC_CLOCKEVENTS */
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 393f422..bfa7811 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1457,7 +1457,7 @@ void hrtimer_run_pending(void)
 	 * check bit in the tick_oneshot code, otherwise we might
 	 * deadlock vs. xtime_lock.
 	 */
-	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled()))
+	if (tick_check_oneshot_change(hrtimer_is_hres_enabled()))
 		hrtimer_switch_to_hres();
 }
 
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 4a94412..77084fa 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1195,11 +1195,11 @@ void tick_oneshot_notify(void)
  * Check, if a change happened, which makes oneshot possible.
  *
  * Called cyclic from the hrtimer softirq (driven by the timer
- * softirq) allow_nohz signals, that we can switch into low-res nohz
- * mode, because high resolution timers are disabled (either compile
- * or runtime).
+ * softirq). If hres_enabled is non zero, it means we can't switch into low-res
+ * nohz mode, because high resolution timers are enabled(either compile or
+ * runtime).
  */
-int tick_check_oneshot_change(int allow_nohz)
+int tick_check_oneshot_change(int hres_enabled)
 {
 	struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
 
@@ -1212,7 +1212,7 @@ int tick_check_oneshot_change(int allow_nohz)
 	if (!timekeeping_valid_for_hres() || !tick_is_oneshot_available())
 		return 0;
 
-	if (!allow_nohz)
+	if (hres_enabled)
 		return 1;
 
 	tick_nohz_switch_to_nohz();
-- 
1.7.12.rc2.18.g61b472e

[PATCH V2 19/19] tick-sched: rearrange code in tick_do_update_jiffies64()

[Viresh Kumar]

The last two lines of tick_do_update_jiffies64() are only executed when
'delta.tv64 >= tick_period.tv64'. Move them to the 'if' block to make it more
clear.

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

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 77084fa..0647589 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -84,12 +84,12 @@ static void tick_do_update_jiffies64(ktime_t now)
 
 		/* Keep the tick_next_period variable up to date */
 		tick_next_period = ktime_add(last_jiffies_update, tick_period);
+
+		write_sequnlock(&jiffies_lock);
+		update_wall_time();
 	} else {
 		write_sequnlock(&jiffies_lock);
-		return;
 	}
-	write_sequnlock(&jiffies_lock);
-	update_wall_time();
 }
 
 /*
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V2 00/19] tick: cleanups (Shouldn't change code behavior)

[Viresh Kumar]

On 21 April 2014 15:24, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> As suggested by you (https://lkml.org/lkml/2014/4/14/797), this is the second
> lot of changes I have. I have divided the earlier patchset into three parts:
> - Bugfixes, already merged
> - Code cleanups which shouldn't have any functional change
> - Code cleanups which may have any functional change
>
> This patchset is targeting the second part now. Its just about moving the code
> around to make it more readable and obvious. Not removing any code at all, that
> will be addressed in next series.
>
> V1->V2: Actually V1 was never reviewed and so it is mostly a resend of V1. Some
> rearrangement of patches is done though.
>
> Viresh Kumar (19):

Hi Thomas,

Any inputs on these ?