linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] tick: A few more cleanups
@ 2014-01-16 15:41 Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 1/4] tick: Rename tick_check_idle() to tick_irq_enter() Frederic Weisbecker
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2014-01-16 15:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Kevin Hilman, Thomas Gleixner,
	Peter Zijlstra, Alex Shi, Steven Rostedt, Paul E. McKenney,
	John Stultz

Ingo,

Please pull the timers/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	timers/core

HEAD: 8fe8ff09ce3b5750e1f3e45a1f4a81d59c7ff1f1

----
Nothing very exiting, just a bunch of non-critical cleanups for the next merge window:

1) Make the IRQ tick APIs naming more symetric

2) Optimize a bit jiffies_lock code coverage

3) Whitespace fixes from Alex Shi

4) Fix overflow in scheduler tick max deferment calculation. Given the
current 1 second max limitation, this bug shouldn't happen in mainline.
It's rather to prepare for making this value tunable. Or simply in case
we change the current constant.

Thanks,
	Frederic
---

Frederic Weisbecker (2):
      tick: Rename tick_check_idle() to tick_irq_enter()
      nohz: Get timekeeping max deferment outside jiffies_lock

Alex Shi (1):
      nohz_full: fix code style issue of tick_nohz_full_stop_tick

Kevin Hilman (1):
      sched/nohz: Fix overflow error in scheduler_tick_max_deferment()


 include/linux/jiffies.h  |  6 ++++++
 include/linux/tick.h     |  6 +++---
 kernel/sched/core.c      |  2 +-
 kernel/softirq.c         |  2 +-
 kernel/time/tick-sched.c | 27 ++++++++++++++-------------
 5 files changed, 25 insertions(+), 18 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] tick: Rename tick_check_idle() to tick_irq_enter()
  2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
@ 2014-01-16 15:41 ` Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 2/4] nohz: Get timekeeping max deferment outside jiffies_lock Frederic Weisbecker
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2014-01-16 15:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Thomas Gleixner, Peter Zijlstra,
	Alex Shi, Steven Rostedt, Paul E. McKenney, John Stultz,
	Kevin Hilman

This makes the code more symetric against the existing tick functions
called on irq exit: tick_irq_exit() and tick_nohz_irq_exit().

These function are also symetric as they mirror each other's action:
we start to account idle time on irq exit and we stop this accounting
on irq entry. Also the tick is stopped on irq exit and timekeeping
catches up with the tickless time elapsed until we reach irq entry.

This rename was suggested by Peter Zijlstra a long while ago but it
got forgotten in the mass.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alex Shi <alex.shi@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Kevin Hilman <khilman@linaro.org>
Link: http://lkml.kernel.org/r/1387320692-28460-2-git-send-email-fweisbec@gmail.com
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/tick.h     | 6 +++---
 kernel/softirq.c         | 2 +-
 kernel/time/tick-sched.c | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 0175d86..b84773c 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -104,7 +104,7 @@ extern struct cpumask *tick_get_broadcast_oneshot_mask(void);
 extern void tick_clock_notify(void);
 extern int tick_check_oneshot_change(int allow_nohz);
 extern struct tick_sched *tick_get_tick_sched(int cpu);
-extern void tick_check_idle(void);
+extern void tick_irq_enter(void);
 extern int tick_oneshot_mode_active(void);
 #  ifndef arch_needs_cpu
 #   define arch_needs_cpu(cpu) (0)
@@ -112,7 +112,7 @@ extern int tick_oneshot_mode_active(void);
 # else
 static inline void tick_clock_notify(void) { }
 static inline int tick_check_oneshot_change(int allow_nohz) { return 0; }
-static inline void tick_check_idle(void) { }
+static inline void tick_irq_enter(void) { }
 static inline int tick_oneshot_mode_active(void) { return 0; }
 # endif
 
@@ -121,7 +121,7 @@ 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 void tick_check_idle(void) { }
+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/softirq.c b/kernel/softirq.c
index 11348de..ca9cb35 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -318,7 +318,7 @@ void irq_enter(void)
 		 * here, as softirq will be serviced on return from interrupt.
 		 */
 		local_bh_disable();
-		tick_check_idle();
+		tick_irq_enter();
 		_local_bh_enable();
 	}
 
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 0ddd020..e4d0f09 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1023,7 +1023,7 @@ static void tick_nohz_kick_tick(struct tick_sched *ts, ktime_t now)
 #endif
 }
 
-static inline void tick_check_nohz_this_cpu(void)
+static inline void tick_nohz_irq_enter(void)
 {
 	struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
 	ktime_t now;
@@ -1042,17 +1042,17 @@ static inline void tick_check_nohz_this_cpu(void)
 #else
 
 static inline void tick_nohz_switch_to_nohz(void) { }
-static inline void tick_check_nohz_this_cpu(void) { }
+static inline void tick_nohz_irq_enter(void) { }
 
 #endif /* CONFIG_NO_HZ_COMMON */
 
 /*
  * Called from irq_enter to notify about the possible interruption of idle()
  */
-void tick_check_idle(void)
+void tick_irq_enter(void)
 {
 	tick_check_oneshot_broadcast_this_cpu();
-	tick_check_nohz_this_cpu();
+	tick_nohz_irq_enter();
 }
 
 /*
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] nohz: Get timekeeping max deferment outside jiffies_lock
  2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 1/4] tick: Rename tick_check_idle() to tick_irq_enter() Frederic Weisbecker
@ 2014-01-16 15:41 ` Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 3/4] nohz_full: fix code style issue of tick_nohz_full_stop_tick Frederic Weisbecker
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2014-01-16 15:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Thomas Gleixner, Peter Zijlstra,
	Alex Shi, Steven Rostedt, Paul E. McKenney, John Stultz,
	Kevin Hilman

We don't need to fetch the timekeeping max deferment under the
jiffies_lock seqlock.

If the clocksource is updated concurrently while we stop the tick,
stop machine is called and the tick will be reevaluated again along with
uptodate jiffies and its related values.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alex Shi <alex.shi@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Kevin Hilman <khilman@linaro.org>
Link: http://lkml.kernel.org/r/1387320692-28460-9-git-send-email-fweisbec@gmail.com
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/time/tick-sched.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index e4d0f09..68331d1 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -533,12 +533,13 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
 	struct clock_event_device *dev = __get_cpu_var(tick_cpu_device).evtdev;
 	u64 time_delta;
 
+	time_delta = timekeeping_max_deferment();
+
 	/* Read jiffies and the time when jiffies were updated last */
 	do {
 		seq = read_seqbegin(&jiffies_lock);
 		last_update = last_jiffies_update;
 		last_jiffies = jiffies;
-		time_delta = timekeeping_max_deferment();
 	} while (read_seqretry(&jiffies_lock, seq));
 
 	if (rcu_needs_cpu(cpu, &rcu_delta_jiffies) ||
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] nohz_full: fix code style issue of tick_nohz_full_stop_tick
  2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 1/4] tick: Rename tick_check_idle() to tick_irq_enter() Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 2/4] nohz: Get timekeeping max deferment outside jiffies_lock Frederic Weisbecker
@ 2014-01-16 15:41 ` Frederic Weisbecker
  2014-01-16 15:41 ` [PATCH 4/4] sched/nohz: Fix overflow error in scheduler_tick_max_deferment() Frederic Weisbecker
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2014-01-16 15:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Alex Shi, Thomas Gleixner, Peter Zijlstra, Steven Rostedt,
	Paul E. McKenney, John Stultz, Kevin Hilman, Frederic Weisbecker

From: Alex Shi <alex.shi@linaro.org>

Code usually starts with 'tab' instead of 7 'space' in kernel

Signed-off-by: Alex Shi <alex.shi@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alex Shi <alex.shi@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Kevin Hilman <khilman@linaro.org>
Link: http://lkml.kernel.org/r/1386074112-30754-2-git-send-email-alex.shi@linaro.org
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/time/tick-sched.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 68331d1..d603bad 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -679,18 +679,18 @@ out:
 static void tick_nohz_full_stop_tick(struct tick_sched *ts)
 {
 #ifdef CONFIG_NO_HZ_FULL
-       int cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
-       if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
-               return;
+	if (!tick_nohz_full_cpu(cpu) || is_idle_task(current))
+		return;
 
-       if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
-	       return;
+	if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
+		return;
 
-       if (!can_stop_full_tick())
-               return;
+	if (!can_stop_full_tick())
+		return;
 
-       tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+	tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
 #endif
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] sched/nohz: Fix overflow error in scheduler_tick_max_deferment()
  2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
                   ` (2 preceding siblings ...)
  2014-01-16 15:41 ` [PATCH 3/4] nohz_full: fix code style issue of tick_nohz_full_stop_tick Frederic Weisbecker
@ 2014-01-16 15:41 ` Frederic Weisbecker
  2014-01-23  0:56 ` [GIT PULL] tick: A few more cleanups Frederic Weisbecker
  2014-01-25  7:28 ` Ingo Molnar
  5 siblings, 0 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2014-01-16 15:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Kevin Hilman, Thomas Gleixner, Peter Zijlstra, Alex Shi,
	Steven Rostedt, Paul E. McKenney, John Stultz,
	Frederic Weisbecker

From: Kevin Hilman <khilman@linaro.org>

While calculating the scheduler tick max deferment, the delta is
converted from microseconds to nanoseconds through a multiplication
against NSEC_PER_USEC.

But this microseconds operand is an unsigned int, thus the result may
likely overflow. The result is cast to u64 but only once the operation
is completed, which is too late to avoid overflown result.

This is currently not a problem because the scheduler tick max deferment
is 1 second. But this may become an issue as we plan to make this
value tunable.

So lets fix this by casting the usecs value to u64 before multiplying by
NSECS_PER_USEC.

Also to prevent from this kind of mistake to happen again, move this
ad-hoc jiffies -> nsecs conversion to a new helper.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alex Shi <alex.shi@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Kevin Hilman <khilman@linaro.org>
Link: http://lkml.kernel.org/r/1387315388-31676-2-git-send-email-khilman@linaro.org
[move ad-hoc conversion to jiffies_to_nsecs helper]
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/jiffies.h | 6 ++++++
 kernel/sched/core.c     | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index d235e88..1f44466 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -294,6 +294,12 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+
+static inline u64 jiffies_to_nsecs(const unsigned long j)
+{
+	return (u64)jiffies_to_usecs(j) * NSEC_PER_USEC;
+}
+
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a88f4a4..61e601f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2325,7 +2325,7 @@ u64 scheduler_tick_max_deferment(void)
 	if (time_before_eq(next, now))
 		return 0;
 
-	return jiffies_to_usecs(next - now) * NSEC_PER_USEC;
+	return jiffies_to_nsecs(next - now);
 }
 #endif
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] tick: A few more cleanups
  2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
                   ` (3 preceding siblings ...)
  2014-01-16 15:41 ` [PATCH 4/4] sched/nohz: Fix overflow error in scheduler_tick_max_deferment() Frederic Weisbecker
@ 2014-01-23  0:56 ` Frederic Weisbecker
  2014-01-25  7:28 ` Ingo Molnar
  5 siblings, 0 replies; 7+ messages in thread
From: Frederic Weisbecker @ 2014-01-23  0:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Kevin Hilman, Thomas Gleixner, Peter Zijlstra, Alex Shi,
	Steven Rostedt, Paul E. McKenney, John Stultz

On Thu, Jan 16, 2014 at 04:41:48PM +0100, Frederic Weisbecker wrote:
> Ingo,
> 
> Please pull the timers/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	timers/core
> 
> HEAD: 8fe8ff09ce3b5750e1f3e45a1f4a81d59c7ff1f1
> 
> ----
> Nothing very exiting, just a bunch of non-critical cleanups for the next merge window:
> 
> 1) Make the IRQ tick APIs naming more symetric
> 
> 2) Optimize a bit jiffies_lock code coverage
> 
> 3) Whitespace fixes from Alex Shi
> 
> 4) Fix overflow in scheduler tick max deferment calculation. Given the
> current 1 second max limitation, this bug shouldn't happen in mainline.
> It's rather to prepare for making this value tunable. Or simply in case
> we change the current constant.
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (2):
>       tick: Rename tick_check_idle() to tick_irq_enter()
>       nohz: Get timekeeping max deferment outside jiffies_lock
> 
> Alex Shi (1):
>       nohz_full: fix code style issue of tick_nohz_full_stop_tick
> 
> Kevin Hilman (1):
>       sched/nohz: Fix overflow error in scheduler_tick_max_deferment()
> 

Ping.

> 
>  include/linux/jiffies.h  |  6 ++++++
>  include/linux/tick.h     |  6 +++---
>  kernel/sched/core.c      |  2 +-
>  kernel/softirq.c         |  2 +-
>  kernel/time/tick-sched.c | 27 ++++++++++++++-------------
>  5 files changed, 25 insertions(+), 18 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PULL] tick: A few more cleanups
  2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
                   ` (4 preceding siblings ...)
  2014-01-23  0:56 ` [GIT PULL] tick: A few more cleanups Frederic Weisbecker
@ 2014-01-25  7:28 ` Ingo Molnar
  5 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2014-01-25  7:28 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Kevin Hilman, Thomas Gleixner, Peter Zijlstra, Alex Shi,
	Steven Rostedt, Paul E. McKenney, John Stultz


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> Please pull the timers/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	timers/core
> 
> HEAD: 8fe8ff09ce3b5750e1f3e45a1f4a81d59c7ff1f1
> 
> ----
> Nothing very exiting, just a bunch of non-critical cleanups for the next merge window:
> 
> 1) Make the IRQ tick APIs naming more symetric
> 
> 2) Optimize a bit jiffies_lock code coverage
> 
> 3) Whitespace fixes from Alex Shi
> 
> 4) Fix overflow in scheduler tick max deferment calculation. Given the
> current 1 second max limitation, this bug shouldn't happen in mainline.
> It's rather to prepare for making this value tunable. Or simply in case
> we change the current constant.
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (2):
>       tick: Rename tick_check_idle() to tick_irq_enter()
>       nohz: Get timekeeping max deferment outside jiffies_lock
> 
> Alex Shi (1):
>       nohz_full: fix code style issue of tick_nohz_full_stop_tick
> 
> Kevin Hilman (1):
>       sched/nohz: Fix overflow error in scheduler_tick_max_deferment()
> 
> 
>  include/linux/jiffies.h  |  6 ++++++
>  include/linux/tick.h     |  6 +++---
>  kernel/sched/core.c      |  2 +-
>  kernel/softirq.c         |  2 +-
>  kernel/time/tick-sched.c | 27 ++++++++++++++-------------
>  5 files changed, 25 insertions(+), 18 deletions(-)

Pulled, thanks Frederic!

	Ingo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-01-25  7:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-16 15:41 [GIT PULL] tick: A few more cleanups Frederic Weisbecker
2014-01-16 15:41 ` [PATCH 1/4] tick: Rename tick_check_idle() to tick_irq_enter() Frederic Weisbecker
2014-01-16 15:41 ` [PATCH 2/4] nohz: Get timekeeping max deferment outside jiffies_lock Frederic Weisbecker
2014-01-16 15:41 ` [PATCH 3/4] nohz_full: fix code style issue of tick_nohz_full_stop_tick Frederic Weisbecker
2014-01-16 15:41 ` [PATCH 4/4] sched/nohz: Fix overflow error in scheduler_tick_max_deferment() Frederic Weisbecker
2014-01-23  0:56 ` [GIT PULL] tick: A few more cleanups Frederic Weisbecker
2014-01-25  7:28 ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).