public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Use jump_label for sched_feat()
@ 2011-12-06 17:15 Peter Zijlstra
  2011-12-06 17:15 ` [PATCH 1/3] jump_label, x86: Fix section mismatch Peter Zijlstra
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Peter Zijlstra @ 2011-12-06 17:15 UTC (permalink / raw)
  To: Ingo Molnar, Jason Baron
  Cc: linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo

These patches implement sched_feat() using jump_labels. The first is a section
mismatch fix. The second patch provides a little infrastructure and the third
patch applies it to make sched_feat() use static_branch().

It uses static_branch() coupled with unlikely/likely in order to control the
branch block position. I haven't checked if it works, but it likely doesn't,
although it should.



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

* [PATCH 1/3] jump_label, x86: Fix section mismatch
  2011-12-06 17:15 [PATCH 0/3] Use jump_label for sched_feat() Peter Zijlstra
@ 2011-12-06 17:15 ` Peter Zijlstra
  2011-12-06 17:15 ` [PATCH 2/3] sched: Fix compile error for UP,!NOHZ Peter Zijlstra
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2011-12-06 17:15 UTC (permalink / raw)
  To: Ingo Molnar, Jason Baron
  Cc: linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo, Peter Zijlstra

[-- Attachment #1: jump_label-section-mismatch.patch --]
[-- Type: text/plain, Size: 1738 bytes --]

WARNING: arch/x86/kernel/built-in.o(.text+0x4c71): Section mismatch in
reference from the function arch_jump_label_transform_static() to the
function .init.text:text_poke_early()
The function arch_jump_label_transform_static() references
the function __init text_poke_early().
This is often because arch_jump_label_transform_static lacks a __init 
annotation or the annotation of text_poke_early is wrong.

Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 arch/x86/kernel/jump_label.c |    2 +-
 kernel/jump_label.c          |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/jump_label.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/jump_label.c
+++ linux-2.6/arch/x86/kernel/jump_label.c
@@ -50,7 +50,7 @@ void arch_jump_label_transform(struct ju
 	put_online_cpus();
 }
 
-void arch_jump_label_transform_static(struct jump_entry *entry,
+__init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
 				      enum jump_label_type type)
 {
 	__jump_label_transform(entry, type, text_poke_early);
Index: linux-2.6/kernel/jump_label.c
===================================================================
--- linux-2.6.orig/kernel/jump_label.c
+++ linux-2.6/kernel/jump_label.c
@@ -142,7 +142,7 @@ static int __jump_label_text_reserved(st
  * running code can override this to make the non-live update case
  * cheaper.
  */
-void __weak arch_jump_label_transform_static(struct jump_entry *entry,
+void __weak __init_or_module arch_jump_label_transform_static(struct jump_entry *entry,
 					    enum jump_label_type type)
 {
 	arch_jump_label_transform(entry, type);	



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

* [PATCH 2/3] sched: Fix compile error for UP,!NOHZ
  2011-12-06 17:15 [PATCH 0/3] Use jump_label for sched_feat() Peter Zijlstra
  2011-12-06 17:15 ` [PATCH 1/3] jump_label, x86: Fix section mismatch Peter Zijlstra
@ 2011-12-06 17:15 ` Peter Zijlstra
  2011-12-06 17:35   ` Peter Zijlstra
  2011-12-06 17:15 ` [PATCH 3/3] sched: Use jump_labels for sched_feat Peter Zijlstra
  2011-12-06 18:10 ` [PATCH 0/3] Use jump_label for sched_feat() Jason Baron
  3 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2011-12-06 17:15 UTC (permalink / raw)
  To: Ingo Molnar, Jason Baron
  Cc: linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo, Suresh Siddha, Peter Zijlstra

[-- Attachment #1: sched-fix-split-fallout.patch --]
[-- Type: text/plain, Size: 788 bytes --]

Commit 69e1e811 ("sched, nohz: Track nr_busy_cpus in the
sched_group_power") messed up the static inline function definition.

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/sched.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -277,7 +277,7 @@ extern void set_cpu_sd_state_idle(void);
 extern int get_nohz_timer_target(void);
 #else
 static inline void select_nohz_load_balancer(int stop_tick) { }
-static inline void set_cpu_sd_state_idle(void);
+static inline void set_cpu_sd_state_idle(void) { }
 #endif
 
 /*



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

* [PATCH 3/3] sched: Use jump_labels for sched_feat
  2011-12-06 17:15 [PATCH 0/3] Use jump_label for sched_feat() Peter Zijlstra
  2011-12-06 17:15 ` [PATCH 1/3] jump_label, x86: Fix section mismatch Peter Zijlstra
  2011-12-06 17:15 ` [PATCH 2/3] sched: Fix compile error for UP,!NOHZ Peter Zijlstra
@ 2011-12-06 17:15 ` Peter Zijlstra
  2011-12-06 18:10 ` [PATCH 0/3] Use jump_label for sched_feat() Jason Baron
  3 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2011-12-06 17:15 UTC (permalink / raw)
  To: Ingo Molnar, Jason Baron
  Cc: linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo, Peter Zijlstra

[-- Attachment #1: sched-debug-feat-static_branch.patch --]
[-- Type: text/plain, Size: 6720 bytes --]

Now that we initialize jump_labels before sched_init() we can use them
for the debug features without having to worry about a window where
they have the wrong setting.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/jump_label.h |    3 ++
 kernel/jump_label.c        |   12 +++++++----
 kernel/sched/core.c        |   46 ++++++++++++++++++++++++++++++++++++++-------
 kernel/sched/features.h    |   30 ++++++++++++++---------------
 kernel/sched/sched.h       |   28 +++++++++++++++++++++++++++
 5 files changed, 93 insertions(+), 26 deletions(-)
Index: linux-2.6/kernel/sched/core.c
===================================================================
--- linux-2.6.orig/kernel/sched/core.c
+++ linux-2.6/kernel/sched/core.c
@@ -149,7 +149,7 @@ static int sched_feat_show(struct seq_fi
 {
 	int i;
 
-	for (i = 0; sched_feat_names[i]; i++) {
+	for (i = 0; i < __SCHED_FEAT_NR; i++) {
 		if (!(sysctl_sched_features & (1UL << i)))
 			seq_puts(m, "NO_");
 		seq_printf(m, "%s ", sched_feat_names[i]);
@@ -159,6 +159,36 @@ static int sched_feat_show(struct seq_fi
 	return 0;
 }
 
+#ifdef HAVE_JUMP_LABEL
+
+#define jump_label_key__true  jump_label_key_enabled
+#define jump_label_key__false jump_label_key_disabled
+
+#define SCHED_FEAT(name, enabled)	\
+	jump_label_key__##enabled ,
+
+struct jump_label_key sched_feat_keys[__SCHED_FEAT_NR] = {
+#include "features.h"
+};
+
+#undef SCHED_FEAT
+
+static void sched_feat_disable(int i)
+{
+	if (jump_label_enabled(&sched_feat_keys[i]))
+		jump_label_dec(&sched_feat_keys[i]);
+}
+
+static void sched_feat_enable(int i)
+{
+	if (!jump_label_enabled(&sched_feat_keys[i]))
+		jump_label_inc(&sched_feat_keys[i]);
+}
+#else
+static void sched_feat_disable(int i) { };
+static void sched_feat_enable(int i) { };
+#endif /* HAVE_JUMP_LABEL */
+
 static ssize_t
 sched_feat_write(struct file *filp, const char __user *ubuf,
 		size_t cnt, loff_t *ppos)
@@ -182,17 +212,20 @@ sched_feat_write(struct file *filp, cons
 		cmp += 3;
 	}
 
-	for (i = 0; sched_feat_names[i]; i++) {
+	for (i = 0; i < __SCHED_FEAT_NR; i++) {
 		if (strcmp(cmp, sched_feat_names[i]) == 0) {
-			if (neg)
+			if (neg) {
 				sysctl_sched_features &= ~(1UL << i);
-			else
+				sched_feat_disable(i);
+			} else {
 				sysctl_sched_features |= (1UL << i);
+				sched_feat_enable(i);
+			}
 			break;
 		}
 	}
 
-	if (!sched_feat_names[i])
+	if (i == __SCHED_FEAT_NR)
 		return -EINVAL;
 
 	*ppos += cnt;
@@ -221,8 +254,7 @@ static __init int sched_init_debug(void)
 	return 0;
 }
 late_initcall(sched_init_debug);
-
-#endif
+#endif /* CONFIG_SCHED_DEBUG */
 
 /*
  * Number of tasks to iterate in a single balance run.
Index: linux-2.6/kernel/sched/features.h
===================================================================
--- linux-2.6.orig/kernel/sched/features.h
+++ linux-2.6/kernel/sched/features.h
@@ -3,13 +3,13 @@
  * them to run sooner, but does not allow tons of sleepers to
  * rip the spread apart.
  */
-SCHED_FEAT(GENTLE_FAIR_SLEEPERS, 1)
+SCHED_FEAT(GENTLE_FAIR_SLEEPERS, true)
 
 /*
  * Place new tasks ahead so that they do not starve already running
  * tasks
  */
-SCHED_FEAT(START_DEBIT, 1)
+SCHED_FEAT(START_DEBIT, true)
 
 /*
  * Based on load and program behaviour, see if it makes sense to place
@@ -17,54 +17,54 @@ SCHED_FEAT(START_DEBIT, 1)
  * improve cache locality. Typically used with SYNC wakeups as
  * generated by pipes and the like, see also SYNC_WAKEUPS.
  */
-SCHED_FEAT(AFFINE_WAKEUPS, 1)
+SCHED_FEAT(AFFINE_WAKEUPS, true)
 
 /*
  * Prefer to schedule the task we woke last (assuming it failed
  * wakeup-preemption), since its likely going to consume data we
  * touched, increases cache locality.
  */
-SCHED_FEAT(NEXT_BUDDY, 0)
+SCHED_FEAT(NEXT_BUDDY, false)
 
 /*
  * Prefer to schedule the task that ran last (when we did
  * wake-preempt) as that likely will touch the same data, increases
  * cache locality.
  */
-SCHED_FEAT(LAST_BUDDY, 1)
+SCHED_FEAT(LAST_BUDDY, true)
 
 /*
  * Consider buddies to be cache hot, decreases the likelyness of a
  * cache buddy being migrated away, increases cache locality.
  */
-SCHED_FEAT(CACHE_HOT_BUDDY, 1)
+SCHED_FEAT(CACHE_HOT_BUDDY, true)
 
 /*
  * Use arch dependent cpu power functions
  */
-SCHED_FEAT(ARCH_POWER, 0)
+SCHED_FEAT(ARCH_POWER, false)
 
-SCHED_FEAT(HRTICK, 0)
-SCHED_FEAT(DOUBLE_TICK, 0)
-SCHED_FEAT(LB_BIAS, 1)
+SCHED_FEAT(HRTICK, false)
+SCHED_FEAT(DOUBLE_TICK, false)
+SCHED_FEAT(LB_BIAS, true)
 
 /*
  * Spin-wait on mutex acquisition when the mutex owner is running on
  * another cpu -- assumes that when the owner is running, it will soon
  * release the lock. Decreases scheduling overhead.
  */
-SCHED_FEAT(OWNER_SPIN, 1)
+SCHED_FEAT(OWNER_SPIN, true)
 
 /*
  * Decrement CPU power based on time not spent running tasks
  */
-SCHED_FEAT(NONTASK_POWER, 1)
+SCHED_FEAT(NONTASK_POWER, true)
 
 /*
  * Queue remote wakeups on the target CPU and process them
  * using the scheduler IPI. Reduces rq->lock contention/bounces.
  */
-SCHED_FEAT(TTWU_QUEUE, 1)
+SCHED_FEAT(TTWU_QUEUE, true)
 
-SCHED_FEAT(FORCE_SD_OVERLAP, 0)
-SCHED_FEAT(RT_RUNTIME_SHARE, 1)
+SCHED_FEAT(FORCE_SD_OVERLAP, false)
+SCHED_FEAT(RT_RUNTIME_SHARE, true)
Index: linux-2.6/kernel/sched/sched.h
===================================================================
--- linux-2.6.orig/kernel/sched/sched.h
+++ linux-2.6/kernel/sched/sched.h
@@ -581,6 +581,7 @@ static inline void __set_task_cpu(struct
  * Tunables that become constants when CONFIG_SCHED_DEBUG is off:
  */
 #ifdef CONFIG_SCHED_DEBUG
+# include <linux/jump_label.h>
 # define const_debug __read_mostly
 #else
 # define const_debug const
@@ -593,11 +594,37 @@ extern const_debug unsigned int sysctl_s
 
 enum {
 #include "features.h"
+	__SCHED_FEAT_NR,
 };
 
 #undef SCHED_FEAT
 
+#if defined(CONFIG_SCHED_DEBUG) && defined(HAVE_JUMP_LABEL)
+static __always_inline bool static_branch__true(struct jump_label_key *key)
+{
+	return likely(static_branch(key)); /* Not out of line branch. */
+}
+
+static __always_inline bool static_branch__false(struct jump_label_key *key)
+{
+	return unlikely(static_branch(key)); /* Out of line branch. */
+}
+
+#define SCHED_FEAT(name, enabled)					\
+static __always_inline bool static_branch_##name(struct jump_label_key *key) \
+{									\
+	return static_branch__##enabled(key);				\
+}
+
+#include "features.h"
+
+#undef SCHED_FEAT
+
+extern struct jump_label_key sched_feat_keys[__SCHED_FEAT_NR];
+#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
+#else /* !(SCHED_DEBUG && HAVE_JUMP_LABEL) */
 #define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
+#endif /* SCHED_DEBUG && HAVE_JUMP_LABEL */
 
 static inline u64 global_rt_period(void)
 {



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

* Re: [PATCH 2/3] sched: Fix compile error for UP,!NOHZ
  2011-12-06 17:15 ` [PATCH 2/3] sched: Fix compile error for UP,!NOHZ Peter Zijlstra
@ 2011-12-06 17:35   ` Peter Zijlstra
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2011-12-06 17:35 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Jason Baron, linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo, Suresh Siddha


Gah, wrong patch as nr.2

---
Subject: jump_label: Provide jump_label_key initializers
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Wed Jul 06 14:20:14 CEST 2011

Provide two initializers for jump_label_key that initialize it enabled
or disabled. Also modify all jump_label code to allow for jump_labels to be
initialized enabled.

Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-p40e3yj21b68y03z1yv825e7@git.kernel.org
---
 include/linux/jump_label.h |    3 +++
 kernel/jump_label.c        |   12 ++++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)
Index: linux-2.6/include/linux/jump_label.h
===================================================================
--- linux-2.6.orig/include/linux/jump_label.h
+++ linux-2.6/include/linux/jump_label.h
@@ -128,4 +128,7 @@ static inline void jump_label_rate_limit
 }
 #endif	/* HAVE_JUMP_LABEL */
 
+#define jump_label_key_enabled	((struct jump_label_key){ .enabled = ATOMIC_INIT(1), })
+#define jump_label_key_disabled	((struct jump_label_key){ .enabled = ATOMIC_INIT(0), })
+
 #endif	/* _LINUX_JUMP_LABEL_H */
Index: linux-2.6/kernel/jump_label.c
===================================================================
--- linux-2.6.orig/kernel/jump_label.c
+++ linux-2.6/kernel/jump_label.c
@@ -248,8 +248,13 @@ void jump_label_apply_nops(struct module
 	if (iter_start == iter_stop)
 		return;
 
-	for (iter = iter_start; iter < iter_stop; iter++)
-		arch_jump_label_transform_static(iter, JUMP_LABEL_DISABLE);
+	for (iter = iter_start; iter < iter_stop; iter++) {
+		struct jump_label_key *iterk;
+
+		iterk = (struct jump_label_key *)(unsigned long)iter->key;
+		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
+				JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
+	}
 }
 
 static int jump_label_add_module(struct module *mod)
@@ -289,8 +294,7 @@ static int jump_label_add_module(struct 
 		key->next = jlm;
 
 		if (jump_label_enabled(key))
-			__jump_label_update(key, iter, iter_stop,
-					    JUMP_LABEL_ENABLE);
+			__jump_label_update(key, iter, iter_stop, JUMP_LABEL_ENABLE);
 	}
 
 	return 0;


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

* Re: [PATCH 0/3] Use jump_label for sched_feat()
  2011-12-06 17:15 [PATCH 0/3] Use jump_label for sched_feat() Peter Zijlstra
                   ` (2 preceding siblings ...)
  2011-12-06 17:15 ` [PATCH 3/3] sched: Use jump_labels for sched_feat Peter Zijlstra
@ 2011-12-06 18:10 ` Jason Baron
  2011-12-06 19:12   ` Peter Zijlstra
  3 siblings, 1 reply; 7+ messages in thread
From: Jason Baron @ 2011-12-06 18:10 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo

On Tue, Dec 06, 2011 at 06:15:34PM +0100, Peter Zijlstra wrote:
> These patches implement sched_feat() using jump_labels. The first is a section
> mismatch fix. The second patch provides a little infrastructure and the third
> patch applies it to make sched_feat() use static_branch().
> 
> It uses static_branch() coupled with unlikely/likely in order to control the
> branch block position. I haven't checked if it works, but it likely doesn't,
> although it should.
> 
> 

It doesn't. I played around with unlikely/likely a bit, and it
didn't seem to make a difference. That said, jump_labels could still be
an improvement here - it be good if we had some numbers, I guess.

Thanks,

-Jason

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

* Re: [PATCH 0/3] Use jump_label for sched_feat()
  2011-12-06 18:10 ` [PATCH 0/3] Use jump_label for sched_feat() Jason Baron
@ 2011-12-06 19:12   ` Peter Zijlstra
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2011-12-06 19:12 UTC (permalink / raw)
  To: Jason Baron
  Cc: Ingo Molnar, linux-kernel, Mike Galbraith, Paul Turner,
	Arnaldo Carvalho de Melo

On Tue, 2011-12-06 at 13:10 -0500, Jason Baron wrote:
> On Tue, Dec 06, 2011 at 06:15:34PM +0100, Peter Zijlstra wrote:
> > These patches implement sched_feat() using jump_labels. The first is a section
> > mismatch fix. The second patch provides a little infrastructure and the third
> > patch applies it to make sched_feat() use static_branch().
> > 
> > It uses static_branch() coupled with unlikely/likely in order to control the
> > branch block position. I haven't checked if it works, but it likely doesn't,
> > although it should.
> > 
> > 
> 
> It doesn't. I played around with unlikely/likely a bit, and it
> didn't seem to make a difference. 

Right, we should look at getting that fixed though.. I think its a very
useful hint to have.

> That said, jump_labels could still be
> an improvement here - it be good if we had some numbers, I guess.

Yeah, acme had some number on an older version of this patch.

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

end of thread, other threads:[~2011-12-06 19:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-06 17:15 [PATCH 0/3] Use jump_label for sched_feat() Peter Zijlstra
2011-12-06 17:15 ` [PATCH 1/3] jump_label, x86: Fix section mismatch Peter Zijlstra
2011-12-06 17:15 ` [PATCH 2/3] sched: Fix compile error for UP,!NOHZ Peter Zijlstra
2011-12-06 17:35   ` Peter Zijlstra
2011-12-06 17:15 ` [PATCH 3/3] sched: Use jump_labels for sched_feat Peter Zijlstra
2011-12-06 18:10 ` [PATCH 0/3] Use jump_label for sched_feat() Jason Baron
2011-12-06 19:12   ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox