timers: Footprint diet and NOHZ overhead mitigation

---
 kernel/time/timer.c |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

kernel/time/timer.c +15 ~~-11~~

Index: tip/kernel/time/timer.c
===================================================================
@@ -356,7 +356,7 @@ EXPORT_SYMBOL_GPL(set_timer_slack);
  * The caller must hold the tvec_base lock.  Returns true if the list
  * was empty and therefore ->timer_jiffies was updated.
  */
-static bool catchup_timer_jiffies(struct tvec_base *base)
+static inline bool catchup_timer_jiffies(struct tvec_base *base)
 {
 	if (!base->all_timers) {
 		base->timer_jiffies = jiffies;

@@ -411,7 +411,10 @@ __internal_add_timer(struct tvec_base *b
 
 static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 {
-	(void)catchup_timer_jiffies(base);
+	/* Advance base->jiffies, if the base is empty */
+	if (!base->all_timers++)
+		base->timer_jiffies = jiffies;
+
 	__internal_add_timer(base, timer);
 	/*
 	 * Update base->active_timers and base->next_timer

@@ -421,7 +424,6 @@ static void internal_add_timer(struct tv
 		    time_before(timer->expires, base->next_timer))
 			base->next_timer = timer->expires;
 	}
-	base->all_timers++;
 
 	/*
 	 * Check whether the other CPU is in dynticks mode and needs

@@ -719,7 +721,6 @@ detach_expired_timer(struct timer_list *
 	if (!tbase_get_deferrable(timer->base))
 		base->active_timers--;
 	base->all_timers--;
-	(void)catchup_timer_jiffies(base);
 }
 
 static int detach_if_pending(struct timer_list *timer, struct tvec_base *base,

@@ -734,8 +735,9 @@ static int detach_if_pending(struct time
 		if (timer->expires == base->next_timer)
 			base->next_timer = base->timer_jiffies;
 	}
-	base->all_timers--;
-	(void)catchup_timer_jiffies(base);
+	/* If this was the last timer, advance base->jiffies */
+	if (!--base->all_timers)
+		base->timer_jiffies = jiffies;
 	return 1;
 }

@@ -1185,14 +1187,16 @@ static inline void __run_timers(struct t
 	struct timer_list *timer;
 
 	spin_lock_irq(&base->lock);
-	if (catchup_timer_jiffies(base)) {
-		spin_unlock_irq(&base->lock);
-		return;
-	}
+
 	while (time_after_eq(jiffies, base->timer_jiffies)) {
 		struct list_head work_list;
 		struct list_head *head = &work_list;
-		int index = base->timer_jiffies & TVR_MASK;
+		int index;
+
+		if (catchup_timer_jiffies(base))
+			break;
+
+		index = base->timer_jiffies & TVR_MASK;
 
 		/*
 		 * Cascade timers:

Viresh Kumar

2015-05-27 05:39 UTC #2 lore raw

Re: [patch 1/7] timers: Sanitize catchup_timer_jiffies() usage

Quoted 7 lines · Thomas Gleixner

3) __run_timers()
We only check on entry, which is silly, because base->timer_jiffies can be behind - especially on NOHZ kernels - and if there is a single deferrable timer somewhere between base->timer_jiffies and jiffies we expire it and then loop until base->timer_jiffies == jiffies.

This may be incorrect. Once we expire that single deferrable timer, we call detach_expired_timer(), which calls catchup_timer_jiffies() at its end. And so the following loop should end right away, isn't it ?

        while (time_after_eq(jiffies, base->timer_jiffies))

kernel/time/timer.c EXPORT_SYMBOL_GPL(set_timer_slack); · line 359

Quoted 3 lines

+++ tip/kernel/time/timer.c

kernel/time/timer.c +1 -1

-static bool catchup_timer_jiffies(struct tvec_base *base)
+static inline bool catchup_timer_jiffies(struct tvec_base *base)

There is only one user left for this routine now, i.e. __run_timers(). Should we drop this routine ?

Signature · 1 line

viresh

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:24 UTC #3 lore raw

[tip:timers/core] timers: Sanitize catchup_timer_jiffies() usage

timers: Sanitize catchup_timer_jiffies() usage

Commit-ID:  3bb475a3446facd0425d3f2fe7e85bf03c5c6c05
Gitweb:     http://git.kernel.org/tip/3bb475a3446facd0425d3f2fe7e85bf03c5c6c05
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:24 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:27 +0200

timers: Sanitize catchup_timer_jiffies() usage

catchup_timer_jiffies() has been applied blindly to several functions without looking for possible better ways to do it.

1) internal_add_timer()

Move the update to base->all_timers before we actually insert the timer into the wheel.

2) detach_if_pending()

Again the update to base->all_timers allows us to explicitely do the timer_jiffies update in place, if this was the last timer which got removed.

3) __run_timers()

Move it into the loop.

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 kernel/time/timer.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

kernel/time/timer.c +16 ~~-24~~

index 7775d45..d5e0179 100644
@@ -351,20 +351,6 @@ void set_timer_slack(struct timer_list *timer, int slack_hz)
 }
 EXPORT_SYMBOL_GPL(set_timer_slack);
 
-/*
- * If the list is empty, catch up ->timer_jiffies to the current time.
- * The caller must hold the tvec_base lock.  Returns true if the list
- * was empty and therefore ->timer_jiffies was updated.
- */
-static bool catchup_timer_jiffies(struct tvec_base *base)
-{
-	if (!base->all_timers) {
-		base->timer_jiffies = jiffies;
-		return true;
-	}
-	return false;
-}
-
 static void
 __internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 {

@@ -411,7 +397,10 @@ __internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 
 static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 {
-	(void)catchup_timer_jiffies(base);
+	/* Advance base->jiffies, if the base is empty */
+	if (!base->all_timers++)
+		base->timer_jiffies = jiffies;
+
 	__internal_add_timer(base, timer);
 	/*
 	 * Update base->active_timers and base->next_timer

@@ -421,7 +410,6 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 		    time_before(timer->expires, base->next_timer))
 			base->next_timer = timer->expires;
 	}
-	base->all_timers++;
 
 	/*
 	 * Check whether the other CPU is in dynticks mode and needs

@@ -718,7 +706,6 @@ detach_expired_timer(struct timer_list *timer, struct tvec_base *base)
 	if (!tbase_get_deferrable(timer->base))
 		base->active_timers--;
 	base->all_timers--;
-	(void)catchup_timer_jiffies(base);
 }
 
 static int detach_if_pending(struct timer_list *timer, struct tvec_base *base,

@@ -733,8 +720,9 @@ static int detach_if_pending(struct timer_list *timer, struct tvec_base *base,
 		if (timer->expires == base->next_timer)
 			base->next_timer = base->timer_jiffies;
 	}
-	base->all_timers--;
-	(void)catchup_timer_jiffies(base);
+	/* If this was the last timer, advance base->jiffies */
+	if (!--base->all_timers)
+		base->timer_jiffies = jiffies;
 	return 1;
 }

@@ -1184,14 +1172,18 @@ static inline void __run_timers(struct tvec_base *base)
 	struct timer_list *timer;
 
 	spin_lock_irq(&base->lock);
-	if (catchup_timer_jiffies(base)) {
-		spin_unlock_irq(&base->lock);
-		return;
-	}
+
 	while (time_after_eq(jiffies, base->timer_jiffies)) {
 		struct list_head work_list;
 		struct list_head *head = &work_list;
-		int index = base->timer_jiffies & TVR_MASK;
+		int index;
+
+		if (!base->all_timers) {
+			base->timer_jiffies = jiffies;
+			break;
+		}
+
+		index = base->timer_jiffies & TVR_MASK;
 
 		/*
 		 * Cascade timers:

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Thomas Gleixner

author

2015-05-26 22:50 UTC #4 lore raw

PATCH 2/7 timer: Remove FIFO guarantee

The FIFO guarantee has been violated by the introduction of timer slack already. Remove it.

This is a preparatory patch for converting the timer wheel to hlist which reduces the memory foot print of the wheel by 50%. It's a seperate patch so any (unlikely to happen) regression caused by this can be identified clearly.

---
 kernel/time/timer.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

kernel/time/timer.c +2 -4

Index: tip/kernel/time/timer.c
===================================================================
@@ -403,10 +403,8 @@ __internal_add_timer(struct tvec_base *b
 		i = (expires >> (TVR_BITS + 3 * TVN_BITS)) & TVN_MASK;
 		vec = base->tv5.vec + i;
 	}
-	/*
-	 * Timers are FIFO:
-	 */
-	list_add_tail(&timer->entry, vec);
+
+	list_add(&timer->entry, vec);
 }
 
 static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)

Viresh Kumar

Reviewed-by on v1 2/7

2015-05-27 09:12 UTC #5 lore raw

Re: [patch 2/7] timer: Remove FIFO guarantee

Quoted 18 lines · Thomas Gleixner

The FIFO guarantee has been violated by the introduction of timer slack already. Remove it.
This is a preparatory patch for converting the timer wheel to hlist which reduces the memory foot print of the wheel by 50%. It's a seperate patch so any (unlikely to happen) regression caused by this can be identified clearly.
Signed-off-by Thomas Gleixner <[email protected]>
---
 kernel/time/timer.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
Index: tip/kernel/time/timer.c
===================================================================

kernel/time/timer.c __internal_add_timer(struct tvec_base *b · line 408

Quoted 10 lines

kernel/time/timer.c +2 -4

@@ -403,10 +403,8 @@ __internal_add_timer(struct tvec_base *b
 		i = (expires >> (TVR_BITS + 3 * TVN_BITS)) & TVN_MASK;
 		vec = base->tv5.vec + i;
 	}
-	/*
-	 * Timers are FIFO:
-	 */
-	list_add_tail(&timer->entry, vec);
+
+	list_add(&timer->entry, vec);
 }

Signature · 1 line

viresh

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:23 UTC #6 lore raw

[tip:timers/core] timer: Remove FIFO "guarantee"

timer: Remove FIFO "guarantee"

Commit-ID:  1bd04bf6f68d65f5422b2b85c495d65d49587a54
Gitweb:     http://git.kernel.org/tip/1bd04bf6f68d65f5422b2b85c495d65d49587a54
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:26 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:27 +0200

timer: Remove FIFO "guarantee"

The FIFO guarantee is only there if two timers are queued into the same bucket at the same jiffie on the same cpu:

- The slack value depends on the delta between expiry and enqueue time, so the resulting expiry time can be different for timers which are queued in different jiffies.

- Timers which are queued into the secondary array end up after a later queued timer which was queued into the primary array due to cascading.

- Timers can end up on different cpus due to the NOHZ target moving around. Obviously there is no guarantee of expiry ordering between cpus.

So anything which relies on FIFO behaviour of the timer wheel is broken already.

This is a preparatory patch for converting the timer wheel to hlist which reduces the memory foot print of the wheel by 50%.

It's a seperate patch so any (unlikely to happen) regression caused by this can be identified clearly.

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Cc George Spelvin <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 kernel/time/timer.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

kernel/time/timer.c +2 -4

index d5e0179..e212df2 100644
@@ -389,10 +389,8 @@ __internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 		i = (expires >> (TVR_BITS + 3 * TVN_BITS)) & TVN_MASK;
 		vec = base->tv5.vec + i;
 	}
-	/*
-	 * Timers are FIFO:
-	 */
-	list_add_tail(&timer->entry, vec);
+
+	list_add(&timer->entry, vec);
 }
 
 static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Thomas Gleixner

author

2015-05-26 22:50 UTC #7 lore raw

PATCH 3/7 timer: Use hlist for the timer wheel hash buckets

This reduces the size of struct tvec_base by 50% and results in slightly smaller code as well.

Before:
struct tvec_base: size: 8256, cachelines: 129

text data bss dec hex filename
17698 13297 8256 39251 9953 ../build/kernel/time/timer.o

After:
struct tvec_base: 4160, cachelines: 65, members: 12 */

text data bss dec hex filename
17491 9201 4160 30852 7884 ../build/kernel/time/timer.o

---
 include/linux/timer.h |    6 ++--
 kernel/time/timer.c   |   64 +++++++++++++++++++++-----------------------------
 2 files changed, 30 insertions(+), 40 deletions(-)

include/linux/timer.h +3 -3

Index: tip/include/linux/timer.h
===================================================================
@@ -14,7 +14,7 @@ struct timer_list {
 	 * All fields that change during normal runtime grouped to the
 	 * same cacheline
 	 */
-	struct list_head entry;
+	struct hlist_node entry;
 	unsigned long expires;
 	struct tvec_base *base;

@@ -71,7 +71,7 @@ extern struct tvec_base boot_tvec_bases;
 #define TIMER_FLAG_MASK			0x3LU
 
 #define __TIMER_INITIALIZER(_function, _expires, _data, _flags) { \
-		.entry = { .prev = TIMER_ENTRY_STATIC },	\
+		.entry = { .next = TIMER_ENTRY_STATIC },	\
 		.function = (_function),			\
 		.expires = (_expires),				\
 		.data = (_data),				\

@@ -168,7 +168,7 @@ static inline void init_timer_on_stack_k
  */
 static inline int timer_pending(const struct timer_list * timer)
 {
-	return timer->entry.next != NULL;
+	return timer->entry.pprev != NULL;
 }
 
 extern void add_timer_on(struct timer_list *timer, int cpu);

kernel/time/timer.c +27 ~~-37~~

174 lines not on this page · raw message

Viresh Kumar

Reviewed-by on v1 3/7

2015-05-27 09:13 UTC #8 lore raw

Re: [patch 3/7] timer: Use hlist for the timer wheel hash buckets

Quoted 12 lines · Thomas Gleixner

This reduces the size of struct tvec_base by 50% and results in slightly smaller code as well.
Before:
struct tvec_base: size: 8256, cachelines: 129
text data bss dec hex filename
17698 13297 8256 39251 9953 ../build/kernel/time/timer.o
After:
struct tvec_base: 4160, cachelines: 65, members: 12 */

Quoted 8 lines

text data bss dec hex filename
17491 9201 4160 30852 7884 ../build/kernel/time/timer.o
Signed-off-by Thomas Gleixner <[email protected]>
---
 include/linux/timer.h |    6 ++--
 kernel/time/timer.c   |   64 +++++++++++++++++++++-----------------------------
 2 files changed, 30 insertions(+), 40 deletions(-)

Signature · 1 line

viresh

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:39 UTC #9 lore raw

[tip:timers/core] timer: Use hlist for the timer wheel hash buckets

timer: Use hlist for the timer wheel hash buckets

Commit-ID:  1dabbcec2c0a36fe43509d06499b9e512e70a028
Gitweb:     http://git.kernel.org/tip/1dabbcec2c0a36fe43509d06499b9e512e70a028
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:28 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:27 +0200

timer: Use hlist for the timer wheel hash buckets

This reduces the size of struct tvec_base by 50% and results in slightly smaller code as well.

Before:
struct tvec_base: size: 8256, cachelines: 129

text data bss dec hex filename
17698 13297 8256 39251 9953 ../build/kernel/time/timer.o

After:
struct tvec_base: 4160, cachelines: 65

text data bss dec hex filename
17491 9201 4160 30852 7884 ../build/kernel/time/timer.o

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 include/linux/timer.h |  6 ++---
 kernel/time/timer.c   | 64 ++++++++++++++++++++++-----------------------------
 2 files changed, 30 insertions(+), 40 deletions(-)

include/linux/timer.h +3 -3

index fbb80e0..064ee24 100644
@@ -14,7 +14,7 @@ struct timer_list {
 	 * All fields that change during normal runtime grouped to the
 	 * same cacheline
 	 */
-	struct list_head entry;
+	struct hlist_node entry;
 	unsigned long expires;
 	struct tvec_base *base;

@@ -71,7 +71,7 @@ extern struct tvec_base boot_tvec_bases;
 #define TIMER_FLAG_MASK			0x3LU
 
 #define __TIMER_INITIALIZER(_function, _expires, _data, _flags) { \
-		.entry = { .prev = TIMER_ENTRY_STATIC },	\
+		.entry = { .next = TIMER_ENTRY_STATIC },	\
 		.function = (_function),			\
 		.expires = (_expires),				\
 		.data = (_data),				\

@@ -168,7 +168,7 @@ static inline void init_timer_on_stack_key(struct timer_list *timer,
  */
 static inline int timer_pending(const struct timer_list * timer)
 {
-	return timer->entry.next != NULL;
+	return timer->entry.pprev != NULL;
 }
 
 extern void add_timer_on(struct timer_list *timer, int cpu);

kernel/time/timer.c +27 ~~-37~~

174 lines not on this page · raw message

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Thomas Gleixner

author

2015-05-26 22:51 UTC #10 lore raw

PATCH 4/7 timer: Replace timer base by a cpu index

Instead of storing a pointer to the per cpu tvec_base we can simply cache a CPU index in the timer_list and use that to get hold of the correct per cpu tvec_base. This is only used in lock_timer_base() and the slightly larger code is peanuts versus the spinlock operation and the d-cache foot print of the timer wheel.

Aside of that this allows to get rid of following nuisances:

- boot_tvec_base

That statically allocated 4k bss data is just kept around so the timer has a home when it gets statically initialized. It serves no other purpose.

With the CPU index we assign the timer to CPU0 at static initialization time and therefor can avoid the whole boot_tvec_base dance. That also simplifies the init code, which just can use the per cpu base.

Before:
text data bss dec hex filename
17491 9201 4160 30852 7884 ../build/kernel/time/timer.o
After:
text data bss dec hex filename
17440 9193 0 26633 6809 ../build/kernel/time/timer.o

- Overloading the base pointer with various flags

The CPU index has enough space to hold the flags (deferrable, irqsafe) so we can get rid of the extra masking and bit fiddling with the base pointer.

As a benefit we reduce the size of struct timer_list on 64 bit machines. 4 - 8 bytes, a size reduction up to 15% per struct timer_list, which is a real win as we have tons of them embedded in other structs.

This changes also the newly added deferrable printout of the timer start trace point to capture and print all timer->flags, which allows us to decode the target cpu of the timer as well.

Cc Steven Rostedt <[email protected]>

Cc Badhri Jagan Sridharan <[email protected]>

---
 include/linux/timer.h        |   38 +++++-------
 include/trace/events/timer.h |   13 ++--
 kernel/time/timer.c          |  127 ++++++++++++-------------------------------
 3 files changed, 58 insertions(+), 120 deletions(-)

include/linux/timer.h +16 ~~-22~~

Index: tip/include/linux/timer.h
===================================================================
@@ -14,27 +14,23 @@ struct timer_list {
 	 * All fields that change during normal runtime grouped to the
 	 * same cacheline
 	 */
-	struct hlist_node entry;
-	unsigned long expires;
-	struct tvec_base *base;
-
-	void (*function)(unsigned long);
-	unsigned long data;
-
-	int slack;
+	struct hlist_node	entry;
+	unsigned long		expires;
+	void			(*function)(unsigned long);
+	unsigned long		data;
+	u32			flags;
+	int			slack;
 
 #ifdef CONFIG_TIMER_STATS
-	int start_pid;
-	void *start_site;
-	char start_comm[16];
+	int			start_pid;
+	void			*start_site;
+	char			start_comm[16];
 #endif
 #ifdef CONFIG_LOCKDEP
-	struct lockdep_map lockdep_map;
+	struct lockdep_map	lockdep_map;
 #endif
 };
 
-extern struct tvec_base boot_tvec_bases;
-
 #ifdef CONFIG_LOCKDEP
 /*
  * NB: because we have to copy the lockdep_map, setting the lockdep_map key

@@ -49,9 +45,6 @@ extern struct tvec_base boot_tvec_bases;
 #endif
 
 /*
- * Note that all tvec_bases are at least 4 byte aligned and lower two bits
- * of base in timer_list is guaranteed to be zero. Use them for flags.
- *
  * A deferrable timer will work normally when the system is busy, but
  * will not cause a CPU to come out of idle just to service it; instead,
  * the timer will be serviced when the CPU eventually wakes up with a

@@ -65,17 +58,18 @@ extern struct tvec_base boot_tvec_bases;
  * workqueue locking issues. It's not meant for executing random crap
  * with interrupts disabled. Abuse is monitored!
  */
-#define TIMER_DEFERRABLE		0x1LU
-#define TIMER_IRQSAFE			0x2LU
-
-#define TIMER_FLAG_MASK			0x3LU
+#define TIMER_CPUMASK		0x0007FFFF
+#define TIMER_MIGRATING		0x00080000
+#define TIMER_BASEMASK		(TIMER_CPUMASK | TIMER_MIGRATING)
+#define TIMER_DEFERRABLE	0x00100000
+#define TIMER_IRQSAFE		0x00200000
 
 #define __TIMER_INITIALIZER(_function, _expires, _data, _flags) { \
 		.entry = { .next = TIMER_ENTRY_STATIC },	\
 		.function = (_function),			\
 		.expires = (_expires),				\
 		.data = (_data),				\
-		.base = (void *)((unsigned long)&boot_tvec_bases + (_flags)), \
+		.flags = (_flags),				\
 		.slack = -1,					\
 		__TIMER_LOCKDEP_MAP_INITIALIZER(		\
 			__FILE__ ":" __stringify(__LINE__))	\

include/trace/events/timer.h +6 -7

Index: tip/include/trace/events/timer.h
===================================================================
@@ -45,16 +45,16 @@ TRACE_EVENT(timer_start,
 
 	TP_PROTO(struct timer_list *timer,
 		unsigned long expires,
-		unsigned int deferrable),
+		unsigned int flags),
 
-	TP_ARGS(timer, expires, deferrable),
+	TP_ARGS(timer, expires, flags),
 
 	TP_STRUCT__entry(
 		__field( void *,	timer		)
 		__field( void *,	function	)
 		__field( unsigned long,	expires		)
 		__field( unsigned long,	now		)
-		__field( unsigned int,	deferrable	)
+		__field( unsigned int,	flags		)
 	),
 
 	TP_fast_assign(

@@ -62,13 +62,12 @@ TRACE_EVENT(timer_start,
 		__entry->function	= timer->function;
 		__entry->expires	= expires;
 		__entry->now		= jiffies;
-		__entry->deferrable     = deferrable;
+		__entry->flags		= flags;
 	),
 
-	TP_printk("timer=%p function=%pf expires=%lu [timeout=%ld] defer=%c",
+	TP_printk("timer=%p function=%pf expires=%lu [timeout=%ld] flags=0x%08x",
 		  __entry->timer, __entry->function, __entry->expires,
-		  (long)__entry->expires - __entry->now,
-		  __entry->deferrable > 0 ? 'y':'n')
+		  (long)__entry->expires - __entry->now, __entry->flags)
 );
 
 /**

kernel/time/timer.c +36 ~~-91~~

309 lines not on this page · raw message

Viresh Kumar

Reviewed-by on v1 4/7

2015-05-27 09:22 UTC #11 lore raw

Re: [patch 4/7] timer: Replace timer base by a cpu index

Quoted 10 lines · Thomas Gleixner

static struct tvec_base *lock_timer_base(struct timer_list *timer,

kernel/time/timer.c +0 -5

 					unsigned long *flags)
 	__acquires(timer->base->lock)
 {
-	struct tvec_base *base;
-
 	for (;;) {
-		struct tvec_base *prelock_base = timer->base;
-		base = tbase_get_base(prelock_base);
-		if (likely(base != NULL)) {

kernel/time/timer.c static int detach_if_pending(struct time · line 766

Quoted 10 lines

kernel/time/timer.c +6 -2

+		u32 tf = timer->flags;
+		struct tvec_base *base;
+
+		if (!(tf & TIMER_MIGRATING)) {
+			base = per_cpu_ptr(&tvec_bases, tf & TIMER_CPUMASK);
 			spin_lock_irqsave(&base->lock, *flags);
-			if (likely(prelock_base == timer->base))
+			if (timer->flags == tf)
 				return base;
-			/* The timer has migrated to another CPU */

Maybe we should retain this comment. Its helpful for people who aren't very familiar with timer core.

kernel/time/timer.c EXPORT_SYMBOL(schedule_timeout_uninterru · line 1514

Quoted 11 lines

static void migrate_timer_list(struct tvec_base *new_base, struct hlist_head *head) {

kernel/time/timer.c +2 -1

 	struct timer_list *timer;
+	int cpu = new_base->cpu;
 
 	while (!hlist_empty(head)) {
 		timer = hlist_entry(head->first, struct timer_list, entry);
 		/* We ignore the accounting on the dying cpu */
 		detach_timer(timer, false);
-		timer_set_base(timer, new_base);
+		timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;

Because 'cpu' is used only once in this routine, maybe we can use 'new_base->cpu' here and the line will still be exactly 80 columns long.

Not sure, but maybe we can create a inline helper for this operation as it is repeated at multiple places.

Look good otherwise:

Signature · 1 line

viresh

Peter Zijlstra

maintainer

2015-05-27 12:09 UTC #12 lore raw

Re: [patch 4/7] timer: Replace timer base by a cpu index

Quoted 8 lines · Viresh Kumar

Review
Quoted 1 line
(quoted diff) +1 -0
+		timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;
Because 'cpu' is used only once in this routine, maybe we can use 'new_base->cpu' here and the line will still be exactly 80 columns long.
Not sure, but maybe we can create a inline helper for this operation as it is repeated at multiple places.

I suggested bitfields at some point I think. That gets the compiler to generate those helpers for you.

Thomas Gleixner

author

2015-06-02 13:59 UTC #13 lore raw

Re: [patch 4/7] timer: Replace timer base by a cpu index

Quoted 7 lines · Peter Zijlstra

Quoted 6 lines · Viresh Kumar
Review
Quoted 1 line
(quoted diff) +1 -0
+		timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;
Because 'cpu' is used only once in this routine, maybe we can use 'new_base->cpu' here and the line will still be exactly 80 columns long.

Quoted 5 lines

Quoted 2 lines
Not sure, but maybe we can create a inline helper for this operation as it is repeated at multiple places.
I suggested bitfields at some point I think. That gets the compiler to generate those helpers for you.

I tried, but they are a PITA for the static initializiers and I could not be bothered to deal with the endianess mess. Maybe I should try again :)

Thanks,

tglx

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:28 UTC #14 lore raw

[tip:timers/core] timer: Replace timer base by a cpu index

timer: Replace timer base by a cpu index

Commit-ID:  0eeda71bc30d74f66f8231f45621d5ace3419186
Gitweb:     http://git.kernel.org/tip/0eeda71bc30d74f66f8231f45621d5ace3419186
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:29 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:27 +0200

timer: Replace timer base by a cpu index

Aside of that this allows to get rid of following nuisances:

- boot_tvec_base

That statically allocated 4k bss data is just kept around so the timer has a home when it gets statically initialized. It serves no other purpose.

Before:
text data bss dec hex filename
17491 9201 4160 30852 7884 ../build/kernel/time/timer.o
After:
text data bss dec hex filename
17440 9193 0 26633 6809 ../build/kernel/time/timer.o

- Overloading the base pointer with various flags

The CPU index has enough space to hold the flags (deferrable, irqsafe) so we can get rid of the extra masking and bit fiddling with the base pointer.

This changes also the newly added deferrable printout of the timer start trace point to capture and print all timer->flags, which allows us to decode the target cpu of the timer as well.

We might have used bitfields for this, but that would change the static initializers and the init function for no value to accomodate big endian bitfields.

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Badhri Jagan Sridharan <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 include/linux/timer.h        |  38 ++++++-------
 include/trace/events/timer.h |  13 ++---
 kernel/time/timer.c          | 127 ++++++++++++-------------------------------
 3 files changed, 58 insertions(+), 120 deletions(-)

include/linux/timer.h +16 ~~-22~~

index 064ee24..4a0d52b 100644
@@ -14,27 +14,23 @@ struct timer_list {
 	 * All fields that change during normal runtime grouped to the
 	 * same cacheline
 	 */
-	struct hlist_node entry;
-	unsigned long expires;
-	struct tvec_base *base;
-
-	void (*function)(unsigned long);
-	unsigned long data;
-
-	int slack;
+	struct hlist_node	entry;
+	unsigned long		expires;
+	void			(*function)(unsigned long);
+	unsigned long		data;
+	u32			flags;
+	int			slack;
 
 #ifdef CONFIG_TIMER_STATS
-	int start_pid;
-	void *start_site;
-	char start_comm[16];
+	int			start_pid;
+	void			*start_site;
+	char			start_comm[16];
 #endif
 #ifdef CONFIG_LOCKDEP
-	struct lockdep_map lockdep_map;
+	struct lockdep_map	lockdep_map;
 #endif
 };
 
-extern struct tvec_base boot_tvec_bases;
-
 #ifdef CONFIG_LOCKDEP
 /*
  * NB: because we have to copy the lockdep_map, setting the lockdep_map key

@@ -49,9 +45,6 @@ extern struct tvec_base boot_tvec_bases;
 #endif
 
 /*
- * Note that all tvec_bases are at least 4 byte aligned and lower two bits
- * of base in timer_list is guaranteed to be zero. Use them for flags.
- *
  * A deferrable timer will work normally when the system is busy, but
  * will not cause a CPU to come out of idle just to service it; instead,
  * the timer will be serviced when the CPU eventually wakes up with a

@@ -65,17 +58,18 @@ extern struct tvec_base boot_tvec_bases;
  * workqueue locking issues. It's not meant for executing random crap
  * with interrupts disabled. Abuse is monitored!
  */
-#define TIMER_DEFERRABLE		0x1LU
-#define TIMER_IRQSAFE			0x2LU
-
-#define TIMER_FLAG_MASK			0x3LU
+#define TIMER_CPUMASK		0x0007FFFF
+#define TIMER_MIGRATING		0x00080000
+#define TIMER_BASEMASK		(TIMER_CPUMASK | TIMER_MIGRATING)
+#define TIMER_DEFERRABLE	0x00100000
+#define TIMER_IRQSAFE		0x00200000
 
 #define __TIMER_INITIALIZER(_function, _expires, _data, _flags) { \
 		.entry = { .next = TIMER_ENTRY_STATIC },	\
 		.function = (_function),			\
 		.expires = (_expires),				\
 		.data = (_data),				\
-		.base = (void *)((unsigned long)&boot_tvec_bases + (_flags)), \
+		.flags = (_flags),				\
 		.slack = -1,					\
 		__TIMER_LOCKDEP_MAP_INITIALIZER(		\
 			__FILE__ ":" __stringify(__LINE__))	\

include/trace/events/timer.h +6 -7

index d7abef1..073b9ac 100644
@@ -45,16 +45,16 @@ TRACE_EVENT(timer_start,
 
 	TP_PROTO(struct timer_list *timer,
 		unsigned long expires,
-		unsigned int deferrable),
+		unsigned int flags),
 
-	TP_ARGS(timer, expires, deferrable),
+	TP_ARGS(timer, expires, flags),
 
 	TP_STRUCT__entry(
 		__field( void *,	timer		)
 		__field( void *,	function	)
 		__field( unsigned long,	expires		)
 		__field( unsigned long,	now		)
-		__field( unsigned int,	deferrable	)
+		__field( unsigned int,	flags		)
 	),
 
 	TP_fast_assign(

@@ -62,13 +62,12 @@ TRACE_EVENT(timer_start,
 		__entry->function	= timer->function;
 		__entry->expires	= expires;
 		__entry->now		= jiffies;
-		__entry->deferrable     = deferrable;
+		__entry->flags		= flags;
 	),
 
-	TP_printk("timer=%p function=%pf expires=%lu [timeout=%ld] defer=%c",
+	TP_printk("timer=%p function=%pf expires=%lu [timeout=%ld] flags=0x%08x",
 		  __entry->timer, __entry->function, __entry->expires,
-		  (long)__entry->expires - __entry->now,
-		  __entry->deferrable > 0 ? 'y':'n')
+		  (long)__entry->expires - __entry->now, __entry->flags)
 );
 
 /**

kernel/time/timer.c +36 ~~-91~~

309 lines not on this page · raw message

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Jan Kiszka

2015-06-27 09:56 UTC #15 lore raw

PATCH 1/1 timer: Fix unsafe cpu variable access in migrate_timers

http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/urgent&id=24bfcb100959c8641a627b5604d967243f8f240c

From: Jan Kiszka <[email protected]>

migrate_timers is invoked with preemption enabled. Therefore we have to get/put the cpu-local variable tvec_bases like before commit 0eeda71bc3.

This fixes

BUG: using smp_processor_id() in preemptible [00000000] code: bash/4917
caller is debug_smp_processor_id+0x17/0x19
CPU: 0 PID: 4917 Comm: bash Not tainted 4.1.0-dbg+ #97
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.8.1-0-g4adadbd-20150316_085822-nilsson.home.kraxel.org 04/01/2014
 ffff880038674040 ffff880002e6fb98 ffffffff815356a0 0000000000000002
 0000000000000000 ffff880002e6fbc8 ffffffff8130c16f ffff88003fd8d500
 000000000000d500 0000000000000000 0000000000000003 ffff880002e6fbd8
Call Trace:
 [<ffffffff815356a0>] dump_stack+0x4f/0x7b
 [<ffffffff8130c16f>] check_preemption_disabled+0xdd/0xef
 [<ffffffff8130c198>] debug_smp_processor_id+0x17/0x19
 [<ffffffff810a9b37>] timer_cpu_notify+0x4f/0x112
 [<ffffffff8106718b>] notifier_call_chain+0x4c/0x71
 [<ffffffff810671be>] __raw_notifier_call_chain+0xe/0x10
 [<ffffffff810484b6>] __cpu_notify+0x20/0x37
 [<ffffffff810484e0>] cpu_notify+0x13/0x15
 [<ffffffff81048591>] cpu_notify_nofail+0xe/0x16
 [<ffffffff8152efb2>] _cpu_down+0x178/0x268
 [<ffffffff8108bc3a>] ? trace_hardirqs_on+0xd/0xf
 [<ffffffff8152f0ca>] cpu_down+0x28/0x3c
 [<ffffffff813cbdb9>] cpu_subsys_offline+0x14/0x16
 [<ffffffff813c779d>] device_offline+0x7d/0xb1
 [<ffffffff813c78a2>] online_store+0x48/0x68
 [<ffffffff813c5544>] dev_attr_store+0x18/0x22
 [<ffffffff811dac6c>] sysfs_kf_write+0x49/0x51
 [<ffffffff811da139>] kernfs_fop_write+0x105/0x158
 [<ffffffff8116c54f>] __vfs_write+0x28/0xbd
 [<ffffffff812ab014>] ? security_file_permission+0x23/0x90
 [<ffffffff8116ccb2>] vfs_write+0xb2/0x169
 [<ffffffff8116d717>] SyS_write+0x4a/0x91
 [<ffffffff8153d12e>] entry_SYSCALL_64_fastpath+0x12/0x76

triggered when offlining a CPU, e.g. via sysfs.

Signed-off-by Jan Kiszka <[email protected]>

---
 kernel/time/timer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

kernel/time/timer.c +2 -1

index 520499d..c826178 100644
@@ -1566,7 +1566,7 @@ static void migrate_timers(int cpu)
 
 	BUG_ON(cpu_online(cpu));
 	old_base = per_cpu_ptr(&tvec_bases, cpu);
-	new_base = this_cpu_ptr(&tvec_bases);
+	new_base = &get_cpu_var(tvec_bases);
 	/*
 	 * The caller is globally serialized and nobody else
 	 * takes two locks at once, deadlock is not possible.

@@ -1590,6 +1590,7 @@ static void migrate_timers(int cpu)
 
 	spin_unlock(&old_base->lock);
 	spin_unlock_irq(&new_base->lock);
+	put_cpu_var(tvec_bases);
 }
 
 static int timer_cpu_notify(struct notifier_block *self,

Borislav Petkov

2015-06-27 11:01 UTC #16 lore raw

Re: [PATCH] timer: Fix unsafe cpu variable access in migrate_timers

Quoted 51 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

migrate_timers is invoked with preemption enabled. Therefore we have to get/put the cpu-local variable tvec_bases like before commit 0eeda71bc3.

This fixes

BUG: using smp_processor_id() in preemptible [00000000] code: bash/4917
caller is debug_smp_processor_id+0x17/0x19
CPU: 0 PID: 4917 Comm: bash Not tainted 4.1.0-dbg+ #97
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.8.1-0-g4adadbd-20150316_085822-nilsson.home.kraxel.org 04/01/2014
 ffff880038674040 ffff880002e6fb98 ffffffff815356a0 0000000000000002
 0000000000000000 ffff880002e6fbc8 ffffffff8130c16f ffff88003fd8d500
 000000000000d500 0000000000000000 0000000000000003 ffff880002e6fbd8
Call Trace:
 [<ffffffff815356a0>] dump_stack+0x4f/0x7b
 [<ffffffff8130c16f>] check_preemption_disabled+0xdd/0xef
 [<ffffffff8130c198>] debug_smp_processor_id+0x17/0x19
 [<ffffffff810a9b37>] timer_cpu_notify+0x4f/0x112
 [<ffffffff8106718b>] notifier_call_chain+0x4c/0x71
 [<ffffffff810671be>] __raw_notifier_call_chain+0xe/0x10
 [<ffffffff810484b6>] __cpu_notify+0x20/0x37
 [<ffffffff810484e0>] cpu_notify+0x13/0x15
 [<ffffffff81048591>] cpu_notify_nofail+0xe/0x16
 [<ffffffff8152efb2>] _cpu_down+0x178/0x268
 [<ffffffff8108bc3a>] ? trace_hardirqs_on+0xd/0xf
 [<ffffffff8152f0ca>] cpu_down+0x28/0x3c
 [<ffffffff813cbdb9>] cpu_subsys_offline+0x14/0x16
 [<ffffffff813c779d>] device_offline+0x7d/0xb1
 [<ffffffff813c78a2>] online_store+0x48/0x68
 [<ffffffff813c5544>] dev_attr_store+0x18/0x22
 [<ffffffff811dac6c>] sysfs_kf_write+0x49/0x51
 [<ffffffff811da139>] kernfs_fop_write+0x105/0x158
 [<ffffffff8116c54f>] __vfs_write+0x28/0xbd
 [<ffffffff812ab014>] ? security_file_permission+0x23/0x90
 [<ffffffff8116ccb2>] vfs_write+0xb2/0x169
 [<ffffffff8116d717>] SyS_write+0x4a/0x91
 [<ffffffff8153d12e>] entry_SYSCALL_64_fastpath+0x12/0x76

triggered when offlining a CPU, e.g. via sysfs.

Signed-off-by Jan Kiszka <[email protected]>

---
 kernel/time/timer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

kernel/time/timer.c +0 -0

index 520499d..c826178 100644

kernel/time/timer.c static void migrate_timers(int cpu) · line 1569

Quoted 5 lines

kernel/time/timer.c +1 -1

@@ -1566,7 +1566,7 @@ static void migrate_timers(int cpu)
 
 	BUG_ON(cpu_online(cpu));
 	old_base = per_cpu_ptr(&tvec_bases, cpu);
-	new_base = this_cpu_ptr(&tvec_bases);
+	new_base = &get_cpu_var(tvec_bases);

Hmm, tglx's version doesn't disable preemtion around it:

Signature · 5 lines

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Jan Kiszka

2015-06-27 11:20 UTC #17 lore raw

Re: [PATCH] timer: Fix unsafe cpu variable access in migrate_timers

Quoted 53 lines · Borislav Petkov

Quoted 52 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

migrate_timers is invoked with preemption enabled. Therefore we have to get/put the cpu-local variable tvec_bases like before commit 0eeda71bc3.

This fixes

BUG: using smp_processor_id() in preemptible [00000000] code: bash/4917
caller is debug_smp_processor_id+0x17/0x19
CPU: 0 PID: 4917 Comm: bash Not tainted 4.1.0-dbg+ #97
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.8.1-0-g4adadbd-20150316_085822-nilsson.home.kraxel.org 04/01/2014
 ffff880038674040 ffff880002e6fb98 ffffffff815356a0 0000000000000002
 0000000000000000 ffff880002e6fbc8 ffffffff8130c16f ffff88003fd8d500
 000000000000d500 0000000000000000 0000000000000003 ffff880002e6fbd8
Call Trace:
 [<ffffffff815356a0>] dump_stack+0x4f/0x7b
 [<ffffffff8130c16f>] check_preemption_disabled+0xdd/0xef
 [<ffffffff8130c198>] debug_smp_processor_id+0x17/0x19
 [<ffffffff810a9b37>] timer_cpu_notify+0x4f/0x112
 [<ffffffff8106718b>] notifier_call_chain+0x4c/0x71
 [<ffffffff810671be>] __raw_notifier_call_chain+0xe/0x10
 [<ffffffff810484b6>] __cpu_notify+0x20/0x37
 [<ffffffff810484e0>] cpu_notify+0x13/0x15
 [<ffffffff81048591>] cpu_notify_nofail+0xe/0x16
 [<ffffffff8152efb2>] _cpu_down+0x178/0x268
 [<ffffffff8108bc3a>] ? trace_hardirqs_on+0xd/0xf
 [<ffffffff8152f0ca>] cpu_down+0x28/0x3c
 [<ffffffff813cbdb9>] cpu_subsys_offline+0x14/0x16
 [<ffffffff813c779d>] device_offline+0x7d/0xb1
 [<ffffffff813c78a2>] online_store+0x48/0x68
 [<ffffffff813c5544>] dev_attr_store+0x18/0x22
 [<ffffffff811dac6c>] sysfs_kf_write+0x49/0x51
 [<ffffffff811da139>] kernfs_fop_write+0x105/0x158
 [<ffffffff8116c54f>] __vfs_write+0x28/0xbd
 [<ffffffff812ab014>] ? security_file_permission+0x23/0x90
 [<ffffffff8116ccb2>] vfs_write+0xb2/0x169
 [<ffffffff8116d717>] SyS_write+0x4a/0x91
 [<ffffffff8153d12e>] entry_SYSCALL_64_fastpath+0x12/0x76

triggered when offlining a CPU, e.g. via sysfs.

Signed-off-by Jan Kiszka <[email protected]>

---
 kernel/time/timer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

kernel/time/timer.c +0 -0

index 520499d..c826178 100644
@@ -1566,7 +1566,7 @@ static void migrate_timers(int cpu)

Quoted 9 lines

kernel/time/timer.c static void migrate_timers(int cpu) · line 1569
Quoted 4 lines
kernel/time/timer.c +1 -1
 	BUG_ON(cpu_online(cpu));
 	old_base = per_cpu_ptr(&tvec_bases, cpu);
-	new_base = this_cpu_ptr(&tvec_bases);
+	new_base = &get_cpu_var(tvec_bases);
Hmm, tglx's version doesn't disable preemtion around it:
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/urgent&id=24bfcb100959c8641a627b5604d967243f8f240c

Oh, there is a fix already. It's just the same: both implicitly disable preemption via get_cpu_ptr/var, look at the macros.

Jan

Borislav Petkov

2015-06-27 11:25 UTC #18 lore raw

Re: [PATCH] timer: Fix unsafe cpu variable access in migrate_timers

Quoted 2 lines · Jan Kiszka

Oh, there is a fix already. It's just the same: both implicitly disable preemption via get_cpu_ptr/var, look at the macros.

Ah, yes.

/me goes to the coffee machine to make eyes open.

Signature · 5 lines

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Thomas Gleixner

author

2015-05-26 22:51 UTC #19 lore raw

PATCH 5/7 timer: stats: Simplify the flags handling

Simplify the handling of the flag storage for the timer statistics. No intermediate storage anymore. Just hand over the flags field.

I left the printout of 'deferrable' for now because changing this would be an ABI update and I have no idea how strong people feel about that. OTOH, I wonder whether we should kill the whole timer stats stuff because all of that information can be retrieved via ftrace/perf as well.

---
 include/linux/timer.h     |    5 +----
 kernel/time/timer.c       |    7 ++-----
 kernel/time/timer_stats.c |   10 +++++-----
 3 files changed, 8 insertions(+), 14 deletions(-)

include/linux/timer.h +1 -4

Index: tip/include/linux/timer.h
===================================================================
@@ -188,13 +188,10 @@ extern void set_timer_slack(struct timer
 
 extern int timer_stats_active;
 
-#define TIMER_STATS_FLAG_DEFERRABLE	0x1
-
 extern void init_timer_stats(void);
 
 extern void timer_stats_update_stats(void *timer, pid_t pid, void *startf,
-				     void *timerf, char *comm,
-				     unsigned int timer_flag);
+				     void *timerf, char *comm, u32 flags);
 
 extern void __timer_stats_timer_set_start_info(struct timer_list *timer,
 					       void *addr);

kernel/time/timer.c +2 -5

Index: tip/kernel/time/timer.c
===================================================================
@@ -418,15 +418,12 @@ void __timer_stats_timer_set_start_info(
 
 static void timer_stats_account_timer(struct timer_list *timer)
 {
-	unsigned int flag = 0;
-
 	if (likely(!timer->start_site))
 		return;
-	if (unlikely(timer->flags & TIMER_DEFERRABLE))
-		flag |= TIMER_STATS_FLAG_DEFERRABLE;
 
 	timer_stats_update_stats(timer, timer->start_pid, timer->start_site,
-				 timer->function, timer->start_comm, flag);
+				 timer->function, timer->start_comm,
+				 timer->flags);
 }
 
 #else

kernel/time/timer_stats.c +5 -5

Index: tip/kernel/time/timer_stats.c
===================================================================
@@ -68,7 +68,7 @@ struct entry {
 	 * Number of timeout events:
 	 */
 	unsigned long		count;
-	unsigned int		timer_flag;
+	u32			flags;
 
 	/*
 	 * We save the command-line string to preserve

@@ -227,13 +227,13 @@ static struct entry *tstat_lookup(struct
  * @startf:	pointer to the function which did the timer setup
  * @timerf:	pointer to the timer callback function of the timer
  * @comm:	name of the process which set up the timer
+ * @tflags:	The flags field of the timer
  *
  * When the timer is already registered, then the event counter is
  * incremented. Otherwise the timer is registered in a free slot.
  */
 void timer_stats_update_stats(void *timer, pid_t pid, void *startf,
-			      void *timerf, char *comm,
-			      unsigned int timer_flag)
+			      void *timerf, char *comm, u32 tflags)
 {
 	/*
 	 * It doesn't matter which lock we take:

@@ -251,7 +251,7 @@ void timer_stats_update_stats(void *time
 	input.start_func = startf;
 	input.expire_func = timerf;
 	input.pid = pid;
-	input.timer_flag = timer_flag;
+	input.flags = tflags;
 
 	raw_spin_lock_irqsave(lock, flags);
 	if (!timer_stats_active)

@@ -306,7 +306,7 @@ static int tstats_show(struct seq_file *
 
 	for (i = 0; i < nr_entries; i++) {
 		entry = entries + i;
-		if (entry->timer_flag & TIMER_STATS_FLAG_DEFERRABLE) {
+		if (entry->flags & TIMER_DEFERRABLE) {
 			seq_printf(m, "%4luD, %5d %-16s ",
 				entry->count, entry->pid, entry->comm);
 		} else {

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:24 UTC #20 lore raw

[tip:timers/core] timer: Stats: Simplify the flags handling

timer: Stats: Simplify the flags handling

Commit-ID:  c74441a17eb975b604e339ca6c11b9ab9aaca11f
Gitweb:     http://git.kernel.org/tip/c74441a17eb975b604e339ca6c11b9ab9aaca11f
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:31 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:27 +0200

timer: Stats: Simplify the flags handling

Simplify the handling of the flag storage for the timer statistics. No intermediate storage anymore. Just hand over the flags field.

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 include/linux/timer.h     |  5 +----
 kernel/time/timer.c       |  7 ++-----
 kernel/time/timer_stats.c | 10 +++++-----
 3 files changed, 8 insertions(+), 14 deletions(-)

include/linux/timer.h +1 -4

index 4a0d52b..ff0689b 100644
@@ -188,13 +188,10 @@ extern void set_timer_slack(struct timer_list *time, int slack_hz);
 
 extern int timer_stats_active;
 
-#define TIMER_STATS_FLAG_DEFERRABLE	0x1
-
 extern void init_timer_stats(void);
 
 extern void timer_stats_update_stats(void *timer, pid_t pid, void *startf,
-				     void *timerf, char *comm,
-				     unsigned int timer_flag);
+				     void *timerf, char *comm, u32 flags);
 
 extern void __timer_stats_timer_set_start_info(struct timer_list *timer,
 					       void *addr);

kernel/time/timer.c +2 -5

index 1540af9..3398d93 100644
@@ -404,15 +404,12 @@ void __timer_stats_timer_set_start_info(struct timer_list *timer, void *addr)
 
 static void timer_stats_account_timer(struct timer_list *timer)
 {
-	unsigned int flag = 0;
-
 	if (likely(!timer->start_site))
 		return;
-	if (unlikely(timer->flags & TIMER_DEFERRABLE))
-		flag |= TIMER_STATS_FLAG_DEFERRABLE;
 
 	timer_stats_update_stats(timer, timer->start_pid, timer->start_site,
-				 timer->function, timer->start_comm, flag);
+				 timer->function, timer->start_comm,
+				 timer->flags);
 }
 
 #else

kernel/time/timer_stats.c +5 -5

index 1fb08f2..1adecb4 100644
@@ -68,7 +68,7 @@ struct entry {
 	 * Number of timeout events:
 	 */
 	unsigned long		count;
-	unsigned int		timer_flag;
+	u32			flags;
 
 	/*
 	 * We save the command-line string to preserve

@@ -227,13 +227,13 @@ static struct entry *tstat_lookup(struct entry *entry, char *comm)
  * @startf:	pointer to the function which did the timer setup
  * @timerf:	pointer to the timer callback function of the timer
  * @comm:	name of the process which set up the timer
+ * @tflags:	The flags field of the timer
  *
  * When the timer is already registered, then the event counter is
  * incremented. Otherwise the timer is registered in a free slot.
  */
 void timer_stats_update_stats(void *timer, pid_t pid, void *startf,
-			      void *timerf, char *comm,
-			      unsigned int timer_flag)
+			      void *timerf, char *comm, u32 tflags)
 {
 	/*
 	 * It doesn't matter which lock we take:

@@ -251,7 +251,7 @@ void timer_stats_update_stats(void *timer, pid_t pid, void *startf,
 	input.start_func = startf;
 	input.expire_func = timerf;
 	input.pid = pid;
-	input.timer_flag = timer_flag;
+	input.flags = tflags;
 
 	raw_spin_lock_irqsave(lock, flags);
 	if (!timer_stats_active)

@@ -306,7 +306,7 @@ static int tstats_show(struct seq_file *m, void *v)
 
 	for (i = 0; i < nr_entries; i++) {
 		entry = entries + i;
-		if (entry->timer_flag & TIMER_STATS_FLAG_DEFERRABLE) {
+		if (entry->flags & TIMER_DEFERRABLE) {
 			seq_printf(m, "%4luD, %5d %-16s ",
 				entry->count, entry->pid, entry->comm);
 		} else {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Thomas Gleixner

author

2015-05-26 22:50 UTC #21 lore raw

PATCH 6/7 timer: Reduce timer migration overhead if disabled

Eric reported that the timer_migration sysctl is not really nice performance wise as it needs to check at every timer insertion whether the feature is enabled or not. Further the check does not live in the timer code, so we have an extra function call which checks an extra cache line to figure out that it is disabled.

We can do better and store that information in the per cpu (hr)timer bases. I pondered to use a static key, but that's a nightmare to update from the nohz code and the timer base cache line is hot anyway when we select a timer base.

The old logic enabled the timer migration unconditionally if CONFIG_NO_HZ was set even if nohz was disabled on the kernel command line.

With this modification, we start off with migration disabled. The user visible sysctl is still set to enabled. If the kernel switches to NOHZ migration is enabled, if the user did not disable it via the sysctl prior to the switch. If nohz=off is on the kernel command line, migration stays disabled no matter what.

Before:

  47.76%  hog       [.] main
  14.84%  [kernel]  [k] _raw_spin_lock_irqsave
   9.55%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.71%  [kernel]  [k] mod_timer
   6.24%  [kernel]  [k] lock_timer_base.isra.38
   3.76%  [kernel]  [k] detach_if_pending
   3.71%  [kernel]  [k] del_timer
   2.50%  [kernel]  [k] internal_add_timer
   1.51%  [kernel]  [k] get_nohz_timer_target
   1.28%  [kernel]  [k] __internal_add_timer
   0.78%  [kernel]  [k] timerfn
   0.48%  [kernel]  [k] wake_up_nohz_cpu

After:

  48.10%  hog       [.] main
  15.25%  [kernel]  [k] _raw_spin_lock_irqsave
   9.76%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.50%  [kernel]  [k] mod_timer
   6.44%  [kernel]  [k] lock_timer_base.isra.38
   3.87%  [kernel]  [k] detach_if_pending
   3.80%  [kernel]  [k] del_timer
   2.67%  [kernel]  [k] internal_add_timer
   1.33%  [kernel]  [k] __internal_add_timer
   0.73%  [kernel]  [k] timerfn
   0.54%  [kernel]  [k] wake_up_nohz_cpu

Reported-by Eric Dumazet <[email protected]>

---
 include/linux/hrtimer.h      |    2 +
 include/linux/sched.h        |    6 ----
 include/linux/sched/sysctl.h |   12 --------
 include/linux/timer.h        |    9 ++++++
 kernel/rcu/tree_plugin.h     |    2 -
 kernel/sched/core.c          |    9 ++----
 kernel/sysctl.c              |   18 ++++++-------
 kernel/time/hrtimer.c        |   35 +++++++++++++++++++------
 kernel/time/tick-internal.h  |   14 ++++++++++
 kernel/time/tick-sched.c     |   25 ++++++++++--------
 kernel/time/timer.c          |   59 +++++++++++++++++++++++++++++++++++++++----
 kernel/time/timer_list.c     |    2 -
 12 files changed, 133 insertions(+), 60 deletions(-)

include/linux/hrtimer.h +2 -0

Index: tip/include/linux/hrtimer.h
===================================================================
@@ -170,6 +170,7 @@ enum  hrtimer_base_type {
  * @cpu:		cpu number
  * @active_bases:	Bitfield to mark bases with active timers
  * @clock_was_set_seq:	Sequence counter of clock was set events
+ * @migration_enabled:	The migration of hrtimers to other cpus is enabled
  * @expires_next:	absolute time of the next event which was scheduled
  *			via clock_set_next_event()
  * @next_timer:		Pointer to the first expiring timer

@@ -191,6 +192,7 @@ struct hrtimer_cpu_base {
 	unsigned int			cpu;
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
+	bool				migration_enabled;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hres_active	: 1,

include/linux/sched.h +1 -5

Index: tip/include/linux/sched.h
===================================================================
@@ -335,14 +335,10 @@ extern int runqueue_is_locked(int cpu);
 #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
 extern void nohz_balance_enter_idle(int cpu);
 extern void set_cpu_sd_state_idle(void);
-extern int get_nohz_timer_target(int pinned);
+extern int get_nohz_timer_target(void);
 #else
 static inline void nohz_balance_enter_idle(int cpu) { }
 static inline void set_cpu_sd_state_idle(void) { }
-static inline int get_nohz_timer_target(int pinned)
-{
-	return smp_processor_id();
-}
 #endif
 
 /*

include/linux/sched/sysctl.h +0 ~~-12~~

Index: tip/include/linux/sched/sysctl.h
===================================================================
@@ -57,24 +57,12 @@ extern unsigned int sysctl_numa_balancin
 extern unsigned int sysctl_sched_migration_cost;
 extern unsigned int sysctl_sched_nr_migrate;
 extern unsigned int sysctl_sched_time_avg;
-extern unsigned int sysctl_timer_migration;
 extern unsigned int sysctl_sched_shares_window;
 
 int sched_proc_update_handler(struct ctl_table *table, int write,
 		void __user *buffer, size_t *length,
 		loff_t *ppos);
 #endif
-#ifdef CONFIG_SCHED_DEBUG
-static inline unsigned int get_sysctl_timer_migration(void)
-{
-	return sysctl_timer_migration;
-}
-#else
-static inline unsigned int get_sysctl_timer_migration(void)
-{
-	return 1;
-}
-#endif
 
 /*
  *  control realtime throttling:

include/linux/timer.h +9 -0

Index: tip/include/linux/timer.h
===================================================================
@@ -238,6 +238,15 @@ extern void run_local_timers(void);
 struct hrtimer;
 extern enum hrtimer_restart it_real_fn(struct hrtimer *);
 
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+#include <linux/sysctl.h>
+
+extern unsigned int sysctl_timer_migration;
+int timer_migration_handler(struct ctl_table *table, int write,
+			    void __user *buffer, size_t *lenp,
+			    loff_t *ppos);
+#endif
+
 unsigned long __round_jiffies(unsigned long j, int cpu);
 unsigned long __round_jiffies_relative(unsigned long j, int cpu);
 unsigned long round_jiffies(unsigned long j);

kernel/rcu/tree_plugin.h +0 -2

Index: tip/kernel/rcu/tree_plugin.h
===================================================================
@@ -1432,8 +1432,6 @@ module_param(rcu_idle_gp_delay, int, 064
 static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
 module_param(rcu_idle_lazy_gp_delay, int, 0644);
 
-extern int tick_nohz_active;
-
 /*
  * Try to advance callbacks for all flavors of RCU on the current CPU, but
  * only if it has been awhile since the last time we did so.  Afterwards,

kernel/sched/core.c +3 -6

Index: tip/kernel/sched/core.c
===================================================================
@@ -572,13 +572,12 @@ void resched_cpu(int cpu)
  * selecting an idle cpu will add more delays to the timers than intended
  * (as that cpu's timer base may not be uptodate wrt jiffies etc).
  */
-int get_nohz_timer_target(int pinned)
+int get_nohz_timer_target(void)
 {
-	int cpu = smp_processor_id();
-	int i;
+	int i, cpu = smp_processor_id();
 	struct sched_domain *sd;
 
-	if (pinned || !get_sysctl_timer_migration() || !idle_cpu(cpu))
+	if (!idle_cpu(cpu))
 		return cpu;
 
 	rcu_read_lock();

@@ -7050,8 +7049,6 @@ void __init sched_init_smp(void)
 }
 #endif /* CONFIG_SMP */
 
-const_debug unsigned int sysctl_timer_migration = 1;
-
 int in_sched_functions(unsigned long addr)
 {
 	return in_lock_functions(addr) ||

kernel/sysctl.c +9 -9

Index: tip/kernel/sysctl.c
===================================================================
@@ -349,15 +349,6 @@ static struct ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
-	{
-		.procname	= "timer_migration",
-		.data		= &sysctl_timer_migration,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &zero,
-		.extra2		= &one,
-	},
 #endif /* CONFIG_SMP */
 #ifdef CONFIG_NUMA_BALANCING
 	{

@@ -1132,6 +1123,15 @@ static struct ctl_table kern_table[] = {
 		.extra1		= &zero,
 		.extra2		= &one,
 	},
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+	{
+		.procname	= "timer_migration",
+		.data		= &sysctl_timer_migration,
+		.maxlen		= sizeof(unsigned int),
+		.mode		= 0644,
+		.proc_handler	= timer_migration_handler,
+	},
+#endif
 	{ }
 };

kernel/time/hrtimer.c +27 -8

Index: tip/kernel/time/hrtimer.c
===================================================================
@@ -164,6 +164,24 @@ hrtimer_check_target(struct hrtimer *tim
 #endif
 }
 
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+static inline
+struct hrtimer_cpu_base *get_target_base(struct hrtimer_cpu_base *base,
+					 int pinned)
+{
+	if (pinned || !base->migration_enabled)
+		return this_cpu_ptr(&hrtimer_bases);
+	return &per_cpu(hrtimer_bases, get_nohz_timer_target());
+}
+#else
+static inline
+struct hrtimer_cpu_base *get_target_base(struct hrtimer_cpu_base *base,
+					 int pinned)
+{
+	return this_cpu_ptr(&hrtimer_bases);
+}
+#endif
+
 /*
  * Switch the timer base to the current CPU when possible.
  */

@@ -171,14 +189,13 @@ static inline struct hrtimer_clock_base
 switch_hrtimer_base(struct hrtimer *timer, struct hrtimer_clock_base *base,
 		    int pinned)
 {
+	struct hrtimer_cpu_base *new_cpu_base, *this_base;
 	struct hrtimer_clock_base *new_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;
 
+	this_base = this_cpu_ptr(&hrtimer_bases);
+	new_cpu_base = get_target_base(this_base, pinned);
 again:
-	new_cpu_base = &per_cpu(hrtimer_bases, cpu);
 	new_base = &new_cpu_base->clock_base[basenum];
 
 	if (base != new_base) {

@@ -199,17 +216,19 @@ again:
 		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;
+		if (new_cpu_base != this_base &&
+		    hrtimer_check_target(timer, new_base)) {
 			raw_spin_unlock(&new_base->cpu_base->lock);
 			raw_spin_lock(&base->cpu_base->lock);
+			new_cpu_base = this_base;
 			timer->base = base;
 			goto again;
 		}
 		timer->base = new_base;
 	} else {
-		if (cpu != this_cpu && hrtimer_check_target(timer, new_base)) {
-			cpu = this_cpu;
+		if (new_cpu_base != this_base &&
+		    hrtimer_check_target(timer, new_base)) {
+			new_cpu_base = this_base;
 			goto again;
 		}
 	}

kernel/time/tick-internal.h +14 -0

Index: tip/kernel/time/tick-internal.h
===================================================================
@@ -138,4 +138,18 @@ extern void tick_nohz_init(void);
 static inline void tick_nohz_init(void) { }
 #endif
 
+#ifdef CONFIG_NO_HZ_COMMON
+extern unsigned long tick_nohz_active;
+#else
+#define tick_nohz_active (0)
+#endif
+
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+extern void timers_update_migration(void);
+#else
+static inline void timers_update_migration(void) { }
+#endif
+
+DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases);
+
 extern u64 get_next_timer_interrupt(unsigned long basej, u64 basem);

kernel/time/tick-sched.c +14 ~~-11~~

Index: tip/kernel/time/tick-sched.c
===================================================================
@@ -399,7 +399,7 @@ void __init tick_nohz_init(void)
  * NO HZ enabled ?
  */
 static int tick_nohz_enabled __read_mostly  = 1;
-int tick_nohz_active  __read_mostly;
+unsigned long tick_nohz_active  __read_mostly;
 /*
  * Enable / Disable tickless mode
  */

@@ -956,6 +956,16 @@ static void tick_nohz_handler(struct clo
 	tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
 }
 
+static inline void tick_nohz_activate(struct tick_sched *ts, int mode)
+{
+	if (!tick_nohz_enabled)
+		return;
+	ts->nohz_mode = mode;
+	/* One update is enough */
+	if (!test_and_set_bit(0, &tick_nohz_active))
+		timers_update_migration();
+}
+
 /**
  * tick_nohz_switch_to_nohz - switch to nohz mode
  */

@@ -970,9 +980,6 @@ static void tick_nohz_switch_to_nohz(voi
 	if (tick_switch_to_oneshot(tick_nohz_handler))
 		return;
 
-	tick_nohz_active = 1;
-	ts->nohz_mode = NOHZ_MODE_LOWRES;
-
 	/*
 	 * Recycle the hrtimer in ts, so we can share the
 	 * hrtimer_forward with the highres code.

@@ -984,6 +991,7 @@ static void tick_nohz_switch_to_nohz(voi
 	hrtimer_forward_now(&ts->sched_timer, tick_period);
 	hrtimer_set_expires(&ts->sched_timer, next);
 	tick_program_event(next, 1);
+	tick_nohz_activate(ts, NOHZ_MODE_LOWRES);
 }
 
 /*

@@ -1035,6 +1043,7 @@ static inline void tick_nohz_irq_enter(v
 
 static inline void tick_nohz_switch_to_nohz(void) { }
 static inline void tick_nohz_irq_enter(void) { }
+static inline void tick_nohz_activate(struct tick_sched *ts, int mode) { }
 
 #endif /* CONFIG_NO_HZ_COMMON */

@@ -1117,13 +1126,7 @@ void tick_setup_sched_timer(void)
 
 	hrtimer_forward(&ts->sched_timer, now, tick_period);
 	hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
-
-#ifdef CONFIG_NO_HZ_COMMON
-	if (tick_nohz_enabled) {
-		ts->nohz_mode = NOHZ_MODE_HIGHRES;
-		tick_nohz_active = 1;
-	}
-#endif
+	tick_nohz_activate(ts, NOHZ_MODE_HIGHRES);
 }
 #endif /* HIGH_RES_TIMERS */

kernel/time/timer.c +54 -5

92 lines not on this page · raw message

kernel/time/timer_list.c +0 -2

Index: tip/kernel/time/timer_list.c
===================================================================
@@ -29,8 +29,6 @@ struct timer_list_iter {
 
 typedef void (*print_fn_t)(struct seq_file *m, unsigned int *classes);
 
-DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases);
-
 /*
  * This allows printing both to /proc/timer_list and
  * to the console (on SysRq-Q):

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:24 UTC #22 lore raw

[tip:timers/core] timer: Reduce timer migration overhead if disabled

timer: Reduce timer migration overhead if disabled

Commit-ID:  bc7a34b8b9ebfb0f4b8a35a72a0b134fd6c5ef50
Gitweb:     http://git.kernel.org/tip/bc7a34b8b9ebfb0f4b8a35a72a0b134fd6c5ef50
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:33 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:28 +0200

timer: Reduce timer migration overhead if disabled

The old logic enabled the timer migration unconditionally if CONFIG_NO_HZ was set even if nohz was disabled on the kernel command line.

Before:

  47.76%  hog       [.] main
  14.84%  [kernel]  [k] _raw_spin_lock_irqsave
   9.55%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.71%  [kernel]  [k] mod_timer
   6.24%  [kernel]  [k] lock_timer_base.isra.38
   3.76%  [kernel]  [k] detach_if_pending
   3.71%  [kernel]  [k] del_timer
   2.50%  [kernel]  [k] internal_add_timer
   1.51%  [kernel]  [k] get_nohz_timer_target
   1.28%  [kernel]  [k] __internal_add_timer
   0.78%  [kernel]  [k] timerfn
   0.48%  [kernel]  [k] wake_up_nohz_cpu

After:

  48.10%  hog       [.] main
  15.25%  [kernel]  [k] _raw_spin_lock_irqsave
   9.76%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.50%  [kernel]  [k] mod_timer
   6.44%  [kernel]  [k] lock_timer_base.isra.38
   3.87%  [kernel]  [k] detach_if_pending
   3.80%  [kernel]  [k] del_timer
   2.67%  [kernel]  [k] internal_add_timer
   1.33%  [kernel]  [k] __internal_add_timer
   0.73%  [kernel]  [k] timerfn
   0.54%  [kernel]  [k] wake_up_nohz_cpu

Reported-by Eric Dumazet <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 include/linux/hrtimer.h      |  2 ++
 include/linux/sched.h        |  6 +----
 include/linux/sched/sysctl.h | 12 ---------
 include/linux/timer.h        |  9 +++++++
 kernel/rcu/tree_plugin.h     |  2 --
 kernel/sched/core.c          |  9 +++----
 kernel/sysctl.c              | 18 +++++++-------
 kernel/time/hrtimer.c        | 35 ++++++++++++++++++++------
 kernel/time/tick-internal.h  | 14 +++++++++++
 kernel/time/tick-sched.c     | 25 ++++++++++---------
 kernel/time/timer.c          | 59 ++++++++++++++++++++++++++++++++++++++++----
 kernel/time/timer_list.c     |  2 --
 12 files changed, 133 insertions(+), 60 deletions(-)

include/linux/hrtimer.h +2 -0

index 5db0558..6955102 100644
@@ -163,6 +163,7 @@ enum  hrtimer_base_type {
  * @cpu:		cpu number
  * @active_bases:	Bitfield to mark bases with active timers
  * @clock_was_set_seq:	Sequence counter of clock was set events
+ * @migration_enabled:	The migration of hrtimers to other cpus is enabled
  * @expires_next:	absolute time of the next event which was scheduled
  *			via clock_set_next_event()
  * @next_timer:		Pointer to the first expiring timer

@@ -186,6 +187,7 @@ struct hrtimer_cpu_base {
 	unsigned int			cpu;
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
+	bool				migration_enabled;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hres_active	: 1,

include/linux/sched.h +1 -5

index 26a2e61..d715146 100644
@@ -335,14 +335,10 @@ extern int runqueue_is_locked(int cpu);
 #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
 extern void nohz_balance_enter_idle(int cpu);
 extern void set_cpu_sd_state_idle(void);
-extern int get_nohz_timer_target(int pinned);
+extern int get_nohz_timer_target(void);
 #else
 static inline void nohz_balance_enter_idle(int cpu) { }
 static inline void set_cpu_sd_state_idle(void) { }
-static inline int get_nohz_timer_target(int pinned)
-{
-	return smp_processor_id();
-}
 #endif
 
 /*

include/linux/sched/sysctl.h +0 ~~-12~~

index 596a0e0..c9e4731 100644
@@ -57,24 +57,12 @@ extern unsigned int sysctl_numa_balancing_scan_size;
 extern unsigned int sysctl_sched_migration_cost;
 extern unsigned int sysctl_sched_nr_migrate;
 extern unsigned int sysctl_sched_time_avg;
-extern unsigned int sysctl_timer_migration;
 extern unsigned int sysctl_sched_shares_window;
 
 int sched_proc_update_handler(struct ctl_table *table, int write,
 		void __user *buffer, size_t *length,
 		loff_t *ppos);
 #endif
-#ifdef CONFIG_SCHED_DEBUG
-static inline unsigned int get_sysctl_timer_migration(void)
-{
-	return sysctl_timer_migration;
-}
-#else
-static inline unsigned int get_sysctl_timer_migration(void)
-{
-	return 1;
-}
-#endif
 
 /*
  *  control realtime throttling:

include/linux/timer.h +9 -0

index ff0689b..61aa61d 100644
@@ -238,6 +238,15 @@ extern void run_local_timers(void);
 struct hrtimer;
 extern enum hrtimer_restart it_real_fn(struct hrtimer *);
 
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+#include <linux/sysctl.h>
+
+extern unsigned int sysctl_timer_migration;
+int timer_migration_handler(struct ctl_table *table, int write,
+			    void __user *buffer, size_t *lenp,
+			    loff_t *ppos);
+#endif
+
 unsigned long __round_jiffies(unsigned long j, int cpu);
 unsigned long __round_jiffies_relative(unsigned long j, int cpu);
 unsigned long round_jiffies(unsigned long j);

kernel/rcu/tree_plugin.h +0 -2

index 0ef80a0..d72fa24 100644
@@ -1432,8 +1432,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
 static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
 module_param(rcu_idle_lazy_gp_delay, int, 0644);
 
-extern int tick_nohz_active;
-
 /*
  * Try to advance callbacks for all flavors of RCU on the current CPU, but
  * only if it has been awhile since the last time we did so.  Afterwards,

kernel/sched/core.c +3 -6

index ecb7c42..e9f25ce 100644
@@ -572,13 +572,12 @@ void resched_cpu(int cpu)
  * selecting an idle cpu will add more delays to the timers than intended
  * (as that cpu's timer base may not be uptodate wrt jiffies etc).
  */
-int get_nohz_timer_target(int pinned)
+int get_nohz_timer_target(void)
 {
-	int cpu = smp_processor_id();
-	int i;
+	int i, cpu = smp_processor_id();
 	struct sched_domain *sd;
 
-	if (pinned || !get_sysctl_timer_migration() || !idle_cpu(cpu))
+	if (!idle_cpu(cpu))
 		return cpu;
 
 	rcu_read_lock();

@@ -7050,8 +7049,6 @@ void __init sched_init_smp(void)
 }
 #endif /* CONFIG_SMP */
 
-const_debug unsigned int sysctl_timer_migration = 1;
-
 int in_sched_functions(unsigned long addr)
 {
 	return in_lock_functions(addr) ||

kernel/sysctl.c +9 -9

index 2082b1a..b13e9d2 100644
@@ -349,15 +349,6 @@ static struct ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
-	{
-		.procname	= "timer_migration",
-		.data		= &sysctl_timer_migration,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &zero,
-		.extra2		= &one,
-	},
 #endif /* CONFIG_SMP */
 #ifdef CONFIG_NUMA_BALANCING
 	{

@@ -1132,6 +1123,15 @@ static struct ctl_table kern_table[] = {
 		.extra1		= &zero,
 		.extra2		= &one,
 	},
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+	{
+		.procname	= "timer_migration",
+		.data		= &sysctl_timer_migration,
+		.maxlen		= sizeof(unsigned int),
+		.mode		= 0644,
+		.proc_handler	= timer_migration_handler,
+	},
+#endif
 	{ }
 };

kernel/time/hrtimer.c +27 -8

index f026413..6115f4d 100644
@@ -177,6 +177,24 @@ hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base)
 #endif
 }
 
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+static inline
+struct hrtimer_cpu_base *get_target_base(struct hrtimer_cpu_base *base,
+					 int pinned)
+{
+	if (pinned || !base->migration_enabled)
+		return this_cpu_ptr(&hrtimer_bases);
+	return &per_cpu(hrtimer_bases, get_nohz_timer_target());
+}
+#else
+static inline
+struct hrtimer_cpu_base *get_target_base(struct hrtimer_cpu_base *base,
+					 int pinned)
+{
+	return this_cpu_ptr(&hrtimer_bases);
+}
+#endif
+
 /*
  * Switch the timer base to the current CPU when possible.
  */

@@ -184,14 +202,13 @@ static inline struct hrtimer_clock_base *
 switch_hrtimer_base(struct hrtimer *timer, struct hrtimer_clock_base *base,
 		    int pinned)
 {
+	struct hrtimer_cpu_base *new_cpu_base, *this_base;
 	struct hrtimer_clock_base *new_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;
 
+	this_base = this_cpu_ptr(&hrtimer_bases);
+	new_cpu_base = get_target_base(this_base, pinned);
 again:
-	new_cpu_base = &per_cpu(hrtimer_bases, cpu);
 	new_base = &new_cpu_base->clock_base[basenum];
 
 	if (base != new_base) {

@@ -212,17 +229,19 @@ again:
 		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;
+		if (new_cpu_base != this_base &&
+		    hrtimer_check_target(timer, new_base)) {
 			raw_spin_unlock(&new_base->cpu_base->lock);
 			raw_spin_lock(&base->cpu_base->lock);
+			new_cpu_base = this_base;
 			timer->base = base;
 			goto again;
 		}
 		timer->base = new_base;
 	} else {
-		if (cpu != this_cpu && hrtimer_check_target(timer, new_base)) {
-			cpu = this_cpu;
+		if (new_cpu_base != this_base &&
+		    hrtimer_check_target(timer, new_base)) {
+			new_cpu_base = this_base;
 			goto again;
 		}
 	}

kernel/time/tick-internal.h +14 -0

index ec2208a..2edde84 100644
@@ -149,4 +149,18 @@ extern void tick_nohz_init(void);
 static inline void tick_nohz_init(void) { }
 #endif
 
+#ifdef CONFIG_NO_HZ_COMMON
+extern unsigned long tick_nohz_active;
+#else
+#define tick_nohz_active (0)
+#endif
+
+#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
+extern void timers_update_migration(void);
+#else
+static inline void timers_update_migration(void) { }
+#endif
+
+DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases);
+
 extern u64 get_next_timer_interrupt(unsigned long basej, u64 basem);

kernel/time/tick-sched.c +14 ~~-11~~

index 812f7a3..b1cb016 100644
@@ -399,7 +399,7 @@ void __init tick_nohz_init(void)
  * NO HZ enabled ?
  */
 static int tick_nohz_enabled __read_mostly  = 1;
-int tick_nohz_active  __read_mostly;
+unsigned long tick_nohz_active  __read_mostly;
 /*
  * Enable / Disable tickless mode
  */

@@ -956,6 +956,16 @@ static void tick_nohz_handler(struct clock_event_device *dev)
 	tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
 }
 
+static inline void tick_nohz_activate(struct tick_sched *ts, int mode)
+{
+	if (!tick_nohz_enabled)
+		return;
+	ts->nohz_mode = mode;
+	/* One update is enough */
+	if (!test_and_set_bit(0, &tick_nohz_active))
+		timers_update_migration();
+}
+
 /**
  * tick_nohz_switch_to_nohz - switch to nohz mode
  */

@@ -970,9 +980,6 @@ static void tick_nohz_switch_to_nohz(void)
 	if (tick_switch_to_oneshot(tick_nohz_handler))
 		return;
 
-	tick_nohz_active = 1;
-	ts->nohz_mode = NOHZ_MODE_LOWRES;
-
 	/*
 	 * Recycle the hrtimer in ts, so we can share the
 	 * hrtimer_forward with the highres code.

@@ -984,6 +991,7 @@ static void tick_nohz_switch_to_nohz(void)
 	hrtimer_forward_now(&ts->sched_timer, tick_period);
 	hrtimer_set_expires(&ts->sched_timer, next);
 	tick_program_event(next, 1);
+	tick_nohz_activate(ts, NOHZ_MODE_LOWRES);
 }
 
 /*

@@ -1035,6 +1043,7 @@ static inline void tick_nohz_irq_enter(void)
 
 static inline void tick_nohz_switch_to_nohz(void) { }
 static inline void tick_nohz_irq_enter(void) { }
+static inline void tick_nohz_activate(struct tick_sched *ts, int mode) { }
 
 #endif /* CONFIG_NO_HZ_COMMON */

@@ -1117,13 +1126,7 @@ void tick_setup_sched_timer(void)
 
 	hrtimer_forward(&ts->sched_timer, now, tick_period);
 	hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
-
-#ifdef CONFIG_NO_HZ_COMMON
-	if (tick_nohz_enabled) {
-		ts->nohz_mode = NOHZ_MODE_HIGHRES;
-		tick_nohz_active = 1;
-	}
-#endif
+	tick_nohz_activate(ts, NOHZ_MODE_HIGHRES);
 }
 #endif /* HIGH_RES_TIMERS */

kernel/time/timer.c +54 -5

92 lines not on this page · raw message

kernel/time/timer_list.c +0 -2

index 1327004..a4536e1 100644
@@ -29,8 +29,6 @@ struct timer_list_iter {
 
 typedef void (*print_fn_t)(struct seq_file *m, unsigned int *classes);
 
-DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases);
-
 /*
  * This allows printing both to /proc/timer_list and
  * to the console (on SysRq-Q):

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Thomas Gleixner

author

2015-05-26 22:50 UTC #23 lore raw

PATCH 7/7 timer: Minimize nohz off overhead

If nohz is disabled on the kernel command line the [hr]timer code still calls wake_up_nohz_cpu() and tick_nohz_full_cpu(), a pretty pointless exercise. Cache nohz_active in [hr]timer per cpu bases and avoid the overhead.

Before:

  48.10%  hog       [.] main
  15.25%  [kernel]  [k] _raw_spin_lock_irqsave
   9.76%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.50%  [kernel]  [k] mod_timer
   6.44%  [kernel]  [k] lock_timer_base.isra.38
   3.87%  [kernel]  [k] detach_if_pending
   3.80%  [kernel]  [k] del_timer
   2.67%  [kernel]  [k] internal_add_timer
   1.33%  [kernel]  [k] __internal_add_timer
   0.73%  [kernel]  [k] timerfn
   0.54%  [kernel]  [k] wake_up_nohz_cpu

After:

  48.73%  hog       [.] main
  15.36%  [kernel]  [k] _raw_spin_lock_irqsave
   9.77%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.61%  [kernel]  [k] lock_timer_base.isra.38
   6.42%  [kernel]  [k] mod_timer
   3.90%  [kernel]  [k] detach_if_pending
   3.76%  [kernel]  [k] del_timer
   2.41%  [kernel]  [k] internal_add_timer
   1.39%  [kernel]  [k] __internal_add_timer
   0.76%  [kernel]  [k] timerfn

We probably should have a cached value for nohz full in the per cpu bases as well to avoid the cpumask check. The base cache line is hot already, the cpumask not necessarily.

---
 include/linux/hrtimer.h     |    2 ++
 kernel/time/hrtimer.c       |    3 ++-
 kernel/time/tick-internal.h |    4 ++--
 kernel/time/tick-sched.c    |    2 +-
 kernel/time/timer.c         |   16 ++++++++++++----
 5 files changed, 19 insertions(+), 8 deletions(-)

include/linux/hrtimer.h +2 -0

Index: tip/include/linux/hrtimer.h
===================================================================
@@ -171,6 +171,7 @@ enum  hrtimer_base_type {
  * @active_bases:	Bitfield to mark bases with active timers
  * @clock_was_set_seq:	Sequence counter of clock was set events
  * @migration_enabled:	The migration of hrtimers to other cpus is enabled
+ * @nohz_active:	The nohz functionality is enabled
  * @expires_next:	absolute time of the next event which was scheduled
  *			via clock_set_next_event()
  * @next_timer:		Pointer to the first expiring timer

@@ -193,6 +194,7 @@ struct hrtimer_cpu_base {
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
 	bool				migration_enabled;
+	bool				nohz_active;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hres_active	: 1,

kernel/time/hrtimer.c +2 -1

Index: tip/kernel/time/hrtimer.c
===================================================================
@@ -987,7 +987,8 @@ void hrtimer_start_range_ns(struct hrtim
 		 * Kick to reschedule the next tick to handle the new timer
 		 * on dynticks target.
 		 */
-		wake_up_nohz_cpu(new_base->cpu_base->cpu);
+		if (new_base->cpu_base->nohz_active)
+			wake_up_nohz_cpu(new_base->cpu_base->cpu);
 	} else {
 		hrtimer_reprogram(timer, new_base);
 	}

kernel/time/tick-internal.h +2 -2

Index: tip/kernel/time/tick-internal.h
===================================================================
@@ -145,9 +145,9 @@ extern unsigned long tick_nohz_active;
 #endif
 
 #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
-extern void timers_update_migration(void);
+extern void timers_update_migration(bool update_nohz);
 #else
-static inline void timers_update_migration(void) { }
+static inline void timers_update_migration(bool update_nohz) { }
 #endif
 
 DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases);

kernel/time/tick-sched.c +1 -1

Index: tip/kernel/time/tick-sched.c
===================================================================
@@ -963,7 +963,7 @@ static inline void tick_nohz_activate(st
 	ts->nohz_mode = mode;
 	/* One update is enough */
 	if (!test_and_set_bit(0, &tick_nohz_active))
-		timers_update_migration();
+		timers_update_migration(true);
 }
 
 /**

kernel/time/timer.c +12 -4

Index: tip/kernel/time/timer.c
===================================================================
@@ -86,6 +86,7 @@ struct tvec_base {
 	unsigned long all_timers;
 	int cpu;
 	bool migration_enabled;
+	bool nohz_active;
 	struct tvec_root tv1;
 	struct tvec tv2;
 	struct tvec tv3;

@@ -99,7 +100,7 @@ static DEFINE_PER_CPU(struct tvec_base,
 #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
 unsigned int sysctl_timer_migration = 1;
 
-void timers_update_migration(void)
+void timers_update_migration(bool update_nohz)
 {
 	bool on = sysctl_timer_migration && tick_nohz_active;
 	unsigned int cpu;

@@ -111,6 +112,10 @@ void timers_update_migration(void)
 	for_each_possible_cpu(cpu) {
 		per_cpu(tvec_bases.migration_enabled, cpu) = on;
 		per_cpu(hrtimer_bases.migration_enabled, cpu) = on;
+		if (!update_nohz)
+			continue;
+		per_cpu(tvec_bases.nohz_active, cpu) = true;
+		per_cpu(hrtimer_bases.nohz_active, cpu) = true;
 	}
 }

@@ -124,7 +129,7 @@ int timer_migration_handler(struct ctl_t
 	mutex_lock(&mutex);
 	ret = proc_dointvec(table, write, buffer, lenp, ppos);
 	if (!ret && write)
-		timers_update_migration();
+		timers_update_migration(false);
 	mutex_unlock(&mutex);
 	return ret;
 }

@@ -450,8 +455,11 @@ static void internal_add_timer(struct tv
 	 * require special care against races with idle_cpu(), lets deal
 	 * with that later.
 	 */
-	if (!(timer->flags & TIMER_DEFERRABLE) || tick_nohz_full_cpu(base->cpu))
-		wake_up_nohz_cpu(base->cpu);
+	if (base->nohz_active) {
+		if (!(timer->flags & TIMER_DEFERRABLE) ||
+		    tick_nohz_full_cpu(base->cpu))
+			wake_up_nohz_cpu(base->cpu);
+	}
 }
 
 #ifdef CONFIG_TIMER_STATS

tip-bot for Thomas Gleixner

merge bot

2015-06-19 13:25 UTC #24 lore raw

[tip:timers/core] timer: Minimize nohz off overhead

timer: Minimize nohz off overhead

Commit-ID:  683be13a284720205228e29207ef11a1c3c322b9
Gitweb:     http://git.kernel.org/tip/683be13a284720205228e29207ef11a1c3c322b9
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 26 May 2015 22:50:35 +0000
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 19 Jun 2015 15:18:28 +0200

timer: Minimize nohz off overhead

Before:

  48.10%  hog       [.] main
  15.25%  [kernel]  [k] _raw_spin_lock_irqsave
   9.76%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.50%  [kernel]  [k] mod_timer
   6.44%  [kernel]  [k] lock_timer_base.isra.38
   3.87%  [kernel]  [k] detach_if_pending
   3.80%  [kernel]  [k] del_timer
   2.67%  [kernel]  [k] internal_add_timer
   1.33%  [kernel]  [k] __internal_add_timer
   0.73%  [kernel]  [k] timerfn
   0.54%  [kernel]  [k] wake_up_nohz_cpu

After:

  48.73%  hog       [.] main
  15.36%  [kernel]  [k] _raw_spin_lock_irqsave
   9.77%  [kernel]  [k] _raw_spin_unlock_irqrestore
   6.61%  [kernel]  [k] lock_timer_base.isra.38
   6.42%  [kernel]  [k] mod_timer
   3.90%  [kernel]  [k] detach_if_pending
   3.76%  [kernel]  [k] del_timer
   2.41%  [kernel]  [k] internal_add_timer
   1.39%  [kernel]  [k] __internal_add_timer
   0.76%  [kernel]  [k] timerfn

We probably should have a cached value for nohz full in the per cpu bases as well to avoid the cpumask check. The base cache line is hot already, the cpumask not necessarily.

Cc Peter Zijlstra <[email protected]>

Cc Paul McKenney <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc John Stultz <[email protected]>

Cc Joonwoo Park <[email protected]>

Cc Wenbo Wang <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

---
 include/linux/hrtimer.h     |  2 ++
 kernel/time/hrtimer.c       |  3 ++-
 kernel/time/tick-internal.h |  4 ++--
 kernel/time/tick-sched.c    |  2 +-
 kernel/time/timer.c         | 16 ++++++++++++----
 5 files changed, 19 insertions(+), 8 deletions(-)

include/linux/hrtimer.h +2 -0

index 6955102..76dd4f0 100644
@@ -164,6 +164,7 @@ enum  hrtimer_base_type {
  * @active_bases:	Bitfield to mark bases with active timers
  * @clock_was_set_seq:	Sequence counter of clock was set events
  * @migration_enabled:	The migration of hrtimers to other cpus is enabled
+ * @nohz_active:	The nohz functionality is enabled
  * @expires_next:	absolute time of the next event which was scheduled
  *			via clock_set_next_event()
  * @next_timer:		Pointer to the first expiring timer

@@ -188,6 +189,7 @@ struct hrtimer_cpu_base {
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
 	bool				migration_enabled;
+	bool				nohz_active;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hres_active	: 1,

kernel/time/hrtimer.c +2 -1

index 6115f4d..db5c950 100644
@@ -994,7 +994,8 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 		 * Kick to reschedule the next tick to handle the new timer
 		 * on dynticks target.
 		 */
-		wake_up_nohz_cpu(new_base->cpu_base->cpu);
+		if (new_base->cpu_base->nohz_active)
+			wake_up_nohz_cpu(new_base->cpu_base->cpu);
 	} else {
 		hrtimer_reprogram(timer, new_base);
 	}

kernel/time/tick-internal.h +2 -2

index 2edde84..966a5a6 100644
@@ -156,9 +156,9 @@ extern unsigned long tick_nohz_active;
 #endif
 
 #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
-extern void timers_update_migration(void);
+extern void timers_update_migration(bool update_nohz);
 #else
-static inline void timers_update_migration(void) { }
+static inline void timers_update_migration(bool update_nohz) { }
 #endif
 
 DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases);

kernel/time/tick-sched.c +1 -1

index b1cb016..c792429 100644
@@ -963,7 +963,7 @@ static inline void tick_nohz_activate(struct tick_sched *ts, int mode)
 	ts->nohz_mode = mode;
 	/* One update is enough */
 	if (!test_and_set_bit(0, &tick_nohz_active))
-		timers_update_migration();
+		timers_update_migration(true);
 }
 
 /**

kernel/time/timer.c +12 -4

index 343142e..520499d 100644
@@ -86,6 +86,7 @@ struct tvec_base {
 	unsigned long all_timers;
 	int cpu;
 	bool migration_enabled;
+	bool nohz_active;
 	struct tvec_root tv1;
 	struct tvec tv2;
 	struct tvec tv3;

@@ -99,7 +100,7 @@ static DEFINE_PER_CPU(struct tvec_base, tvec_bases);
 #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
 unsigned int sysctl_timer_migration = 1;
 
-void timers_update_migration(void)
+void timers_update_migration(bool update_nohz)
 {
 	bool on = sysctl_timer_migration && tick_nohz_active;
 	unsigned int cpu;

@@ -111,6 +112,10 @@ void timers_update_migration(void)
 	for_each_possible_cpu(cpu) {
 		per_cpu(tvec_bases.migration_enabled, cpu) = on;
 		per_cpu(hrtimer_bases.migration_enabled, cpu) = on;
+		if (!update_nohz)
+			continue;
+		per_cpu(tvec_bases.nohz_active, cpu) = true;
+		per_cpu(hrtimer_bases.nohz_active, cpu) = true;
 	}
 }

@@ -124,7 +129,7 @@ int timer_migration_handler(struct ctl_table *table, int write,
 	mutex_lock(&mutex);
 	ret = proc_dointvec(table, write, buffer, lenp, ppos);
 	if (!ret && write)
-		timers_update_migration();
+		timers_update_migration(false);
 	mutex_unlock(&mutex);
 	return ret;
 }

@@ -436,8 +441,11 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 	 * require special care against races with idle_cpu(), lets deal
 	 * with that later.
 	 */
-	if (!(timer->flags & TIMER_DEFERRABLE) || tick_nohz_full_cpu(base->cpu))
-		wake_up_nohz_cpu(base->cpu);
+	if (base->nohz_active) {
+		if (!(timer->flags & TIMER_DEFERRABLE) ||
+		    tick_nohz_full_cpu(base->cpu))
+			wake_up_nohz_cpu(base->cpu);
+	}
 }
 
 #ifdef CONFIG_TIMER_STATS

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Eric Dumazet

2015-05-27 14:53 UTC #25 lore raw