[PATCH 0/8] [GIT PULL] jump label: various fixes and updates

[PATCH 0/8] [GIT PULL] jump label: various fixes and updates

[Steven Rostedt]

Ingo,

Please pull the latest tip/perf/jump-label tree, which can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/jump-label


David Daney (2):
      jump label: Make arch_jump_label_text_poke_early() optional
      jump label: Add MIPS support.

David Miller (1):
      jump_label: Fix unaligned traps on sparc.

H. Peter Anvin (1):
      x86, ftrace: Use safe noops, drop trap test

Jason Baron (3):
      jump label: Fix module __init section race
      jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex
      jump label: Disable due to compiler bug

Steven Rostedt (1):
      jump label: Fix error with preempt disable holding mutex

----
 arch/mips/Kconfig                   |    1 +
 arch/mips/include/asm/jump_label.h  |   48 ++++++++++++++++++++++++
 arch/mips/kernel/Makefile           |    3 +-
 arch/mips/kernel/jump_label.c       |   50 +++++++++++++++++++++++++
 arch/mips/kernel/module.c           |    5 ++
 arch/sparc/include/asm/jump_label.h |    2 +
 arch/x86/include/asm/jump_label.h   |    1 +
 arch/x86/kernel/alternative.c       |   69 +++++++---------------------------
 include/linux/jump_label.h          |   20 ++++++++++
 kernel/jump_label.c                 |   70 +++++++++++++++++++++++++++++-----
 kernel/kprobes.c                    |   26 ++++++++-----
 11 files changed, 219 insertions(+), 76 deletions(-)

[PATCH 1/8] jump label: Fix module __init section race

[Steven Rostedt]

[-- Attachment #1: 0001-jump-label-Fix-module-__init-section-race.patch --]
[-- Type: text/plain, Size: 3257 bytes --]

From: Jason Baron <jbaron@redhat.com>

Jump label uses is_module_text_address() to ensure that the module
__init sections are valid before updating them. However, between the
check for a valid module __init section and the subsequent jump
label update, the module's __init section could be freed out from under
us.

We fix this potential race by adding a notifier callback to the
MODULE_STATE_LIVE state. This notifier is called *after* the __init
section has been run but before it is going to be freed. In the
callback, the jump label code zeros the key value for any __init jump
code within the module, and we add a check for a non-zero key value when
we update jump labels. In this way we require no additional data
structures.

Thanks to Mathieu Desnoyers for pointing out this race condition.

Reported-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <c6f037b7598777668025ceedd9294212fd95fa34.1285965957.git.jbaron@redhat.com>

[ Renamed remove_module_init() to remove_jump_label_module_init()
  as suggested by Masami Hiramatsu. ]

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/jump_label.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 7be868b..be9e105 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -168,7 +168,8 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 			count = e_module->nr_entries;
 			iter = e_module->table;
 			while (count--) {
-				if (kernel_text_address(iter->code))
+				if (iter->key &&
+						kernel_text_address(iter->code))
 					arch_jump_label_transform(iter, type);
 				iter++;
 			}
@@ -366,6 +367,39 @@ static void remove_jump_label_module(struct module *mod)
 	}
 }
 
+static void remove_jump_label_module_init(struct module *mod)
+{
+	struct hlist_head *head;
+	struct hlist_node *node, *node_next, *module_node, *module_node_next;
+	struct jump_label_entry *e;
+	struct jump_label_module_entry *e_module;
+	struct jump_entry *iter;
+	int i, count;
+
+	/* if the module doesn't have jump label entries, just return */
+	if (!mod->num_jump_entries)
+		return;
+
+	for (i = 0; i < JUMP_LABEL_TABLE_SIZE; i++) {
+		head = &jump_label_table[i];
+		hlist_for_each_entry_safe(e, node, node_next, head, hlist) {
+			hlist_for_each_entry_safe(e_module, module_node,
+						  module_node_next,
+						  &(e->modules), hlist) {
+				if (e_module->mod != mod)
+					continue;
+				count = e_module->nr_entries;
+				iter = e_module->table;
+				while (count--) {
+					if (within_module_init(iter->code, mod))
+						iter->key = 0;
+					iter++;
+				}
+			}
+		}
+	}
+}
+
 static int
 jump_label_module_notify(struct notifier_block *self, unsigned long val,
 			 void *data)
@@ -386,6 +420,11 @@ jump_label_module_notify(struct notifier_block *self, unsigned long val,
 		remove_jump_label_module(mod);
 		mutex_unlock(&jump_label_mutex);
 		break;
+	case MODULE_STATE_LIVE:
+		mutex_lock(&jump_label_mutex);
+		remove_jump_label_module_init(mod);
+		mutex_unlock(&jump_label_mutex);
+		break;
 	}
 	return ret;
 }
-- 
1.7.1

[PATCH 2/8] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex

[Steven Rostedt]

[-- Attachment #1: 0002-jump-label-Fix-deadlock-b-w-jump_label_mutex-vs.-tex.patch --]
[-- Type: text/plain, Size: 5944 bytes --]

From: Jason Baron <jbaron@redhat.com>

register_kprobe() downs the 'text_mutex' and then calls
jump_label_text_reserved(), which downs the 'jump_label_mutex'.
However, the jump label code takes those mutexes in the reverse
order.

Fix by requiring the caller of jump_label_text_reserved() to do
the jump label locking via the newly added: jump_label_lock(),
jump_label_unlock(). Currently, kprobes is the only user
of jump_label_text_reserved().

Reported-by: Ingo Molnar <mingo@elte.hu>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <759032c48d5e30c27f0bba003d09bffa8e9f28bb.1285965957.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/jump_label.h |    5 +++++
 kernel/jump_label.c        |   33 +++++++++++++++++++++------------
 kernel/kprobes.c           |    6 ++++++
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index b67cb18..1947a12 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -18,6 +18,8 @@ struct module;
 extern struct jump_entry __start___jump_table[];
 extern struct jump_entry __stop___jump_table[];
 
+extern void jump_label_lock(void);
+extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
 				 enum jump_label_type type);
 extern void arch_jump_label_text_poke_early(jump_label_t addr);
@@ -59,6 +61,9 @@ static inline int jump_label_text_reserved(void *start, void *end)
 	return 0;
 }
 
+static inline void jump_label_lock(void) {}
+static inline void jump_label_unlock(void) {}
+
 #endif
 
 #define COND_STMT(key, stmt)					\
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index be9e105..12cce78 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -39,6 +39,16 @@ struct jump_label_module_entry {
 	struct module *mod;
 };
 
+void jump_label_lock(void)
+{
+	mutex_lock(&jump_label_mutex);
+}
+
+void jump_label_unlock(void)
+{
+	mutex_unlock(&jump_label_mutex);
+}
+
 static int jump_label_cmp(const void *a, const void *b)
 {
 	const struct jump_entry *jea = a;
@@ -152,7 +162,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 	struct jump_label_module_entry *e_module;
 	int count;
 
-	mutex_lock(&jump_label_mutex);
+	jump_label_lock();
 	entry = get_jump_label_entry((jump_label_t)key);
 	if (entry) {
 		count = entry->nr_entries;
@@ -175,7 +185,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 			}
 		}
 	}
-	mutex_unlock(&jump_label_mutex);
+	jump_label_unlock();
 }
 
 static int addr_conflict(struct jump_entry *entry, void *start, void *end)
@@ -232,6 +242,7 @@ out:
  * overlaps with any of the jump label patch addresses. Code
  * that wants to modify kernel text should first verify that
  * it does not overlap with any of the jump label addresses.
+ * Caller must hold jump_label_mutex.
  *
  * returns 1 if there is an overlap, 0 otherwise
  */
@@ -242,7 +253,6 @@ int jump_label_text_reserved(void *start, void *end)
 	struct jump_entry *iter_stop = __start___jump_table;
 	int conflict = 0;
 
-	mutex_lock(&jump_label_mutex);
 	iter = iter_start;
 	while (iter < iter_stop) {
 		if (addr_conflict(iter, start, end)) {
@@ -257,7 +267,6 @@ int jump_label_text_reserved(void *start, void *end)
 	conflict = module_conflict(start, end);
 #endif
 out:
-	mutex_unlock(&jump_label_mutex);
 	return conflict;
 }
 
@@ -268,7 +277,7 @@ static __init int init_jump_label(void)
 	struct jump_entry *iter_stop = __stop___jump_table;
 	struct jump_entry *iter;
 
-	mutex_lock(&jump_label_mutex);
+	jump_label_lock();
 	ret = build_jump_label_hashtable(__start___jump_table,
 					 __stop___jump_table);
 	iter = iter_start;
@@ -276,7 +285,7 @@ static __init int init_jump_label(void)
 		arch_jump_label_text_poke_early(iter->code);
 		iter++;
 	}
-	mutex_unlock(&jump_label_mutex);
+	jump_label_unlock();
 	return ret;
 }
 early_initcall(init_jump_label);
@@ -409,21 +418,21 @@ jump_label_module_notify(struct notifier_block *self, unsigned long val,
 
 	switch (val) {
 	case MODULE_STATE_COMING:
-		mutex_lock(&jump_label_mutex);
+		jump_label_lock();
 		ret = add_jump_label_module(mod);
 		if (ret)
 			remove_jump_label_module(mod);
-		mutex_unlock(&jump_label_mutex);
+		jump_label_unlock();
 		break;
 	case MODULE_STATE_GOING:
-		mutex_lock(&jump_label_mutex);
+		jump_label_lock();
 		remove_jump_label_module(mod);
-		mutex_unlock(&jump_label_mutex);
+		jump_label_unlock();
 		break;
 	case MODULE_STATE_LIVE:
-		mutex_lock(&jump_label_mutex);
+		jump_label_lock();
 		remove_jump_label_module_init(mod);
-		mutex_unlock(&jump_label_mutex);
+		jump_label_unlock();
 		break;
 	}
 	return ret;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 99865c3..9437e14 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1146,13 +1146,16 @@ int __kprobes register_kprobe(struct kprobe *p)
 		return ret;
 
 	preempt_disable();
+	jump_label_lock();
 	if (!kernel_text_address((unsigned long) p->addr) ||
 	    in_kprobes_functions((unsigned long) p->addr) ||
 	    ftrace_text_reserved(p->addr, p->addr) ||
 	    jump_label_text_reserved(p->addr, p->addr)) {
 		preempt_enable();
+		jump_label_unlock();
 		return -EINVAL;
 	}
+	jump_label_unlock();
 
 	/* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */
 	p->flags &= KPROBE_FLAG_DISABLED;
@@ -1187,6 +1190,8 @@ int __kprobes register_kprobe(struct kprobe *p)
 	INIT_LIST_HEAD(&p->list);
 	mutex_lock(&kprobe_mutex);
 
+	jump_label_lock(); /* needed to call jump_label_text_reserved() */
+
 	get_online_cpus();	/* For avoiding text_mutex deadlock. */
 	mutex_lock(&text_mutex);
 
@@ -1214,6 +1219,7 @@ int __kprobes register_kprobe(struct kprobe *p)
 out:
 	mutex_unlock(&text_mutex);
 	put_online_cpus();
+	jump_label_unlock();
 	mutex_unlock(&kprobe_mutex);
 
 	if (probed_mod)
-- 
1.7.1

[PATCH 3/8] jump label: Fix error with preempt disable holding mutex

[Steven Rostedt]

[-- Attachment #1: 0003-jump-label-Fix-error-with-preempt-disable-holding-mu.patch --]
[-- Type: text/plain, Size: 2542 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Kprobes and jump label were having a race between mutexes that
was fixed by reordering the jump label. But this reordering
moved the jump label mutex into a preempt disable location.

This patch does a little fiddling to move the grabbing of
the jump label mutex from inside the preempt disable section
and still keep the order correct between the mutex and the
kprobes lock.

Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/kprobes.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 9437e14..9737a76 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1145,17 +1145,13 @@ int __kprobes register_kprobe(struct kprobe *p)
 	if (ret)
 		return ret;
 
-	preempt_disable();
 	jump_label_lock();
+	preempt_disable();
 	if (!kernel_text_address((unsigned long) p->addr) ||
 	    in_kprobes_functions((unsigned long) p->addr) ||
 	    ftrace_text_reserved(p->addr, p->addr) ||
-	    jump_label_text_reserved(p->addr, p->addr)) {
-		preempt_enable();
-		jump_label_unlock();
-		return -EINVAL;
-	}
-	jump_label_unlock();
+	    jump_label_text_reserved(p->addr, p->addr))
+		goto fail_with_jump_label;
 
 	/* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */
 	p->flags &= KPROBE_FLAG_DISABLED;
@@ -1169,10 +1165,9 @@ int __kprobes register_kprobe(struct kprobe *p)
 		 * We must hold a refcount of the probed module while updating
 		 * its code to prohibit unexpected unloading.
 		 */
-		if (unlikely(!try_module_get(probed_mod))) {
-			preempt_enable();
-			return -EINVAL;
-		}
+		if (unlikely(!try_module_get(probed_mod)))
+			goto fail_with_jump_label;
+
 		/*
 		 * If the module freed .init.text, we couldn't insert
 		 * kprobes in there.
@@ -1180,11 +1175,11 @@ int __kprobes register_kprobe(struct kprobe *p)
 		if (within_module_init((unsigned long)p->addr, probed_mod) &&
 		    probed_mod->state != MODULE_STATE_COMING) {
 			module_put(probed_mod);
-			preempt_enable();
-			return -EINVAL;
+			goto fail_with_jump_label;
 		}
 	}
 	preempt_enable();
+	jump_label_unlock();
 
 	p->nmissed = 0;
 	INIT_LIST_HEAD(&p->list);
@@ -1226,6 +1221,11 @@ out:
 		module_put(probed_mod);
 
 	return ret;
+
+fail_with_jump_label:
+	preempt_enable();
+	jump_label_unlock();
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(register_kprobe);
 
-- 
1.7.1

Re: [PATCH 3/8] jump label: Fix error with preempt disable holding mutex

[Masami Hiramatsu]

(2010/10/28 22:55), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Kprobes and jump label were having a race between mutexes that
> was fixed by reordering the jump label. But this reordering
> moved the jump label mutex into a preempt disable location.
> 
> This patch does a little fiddling to move the grabbing of
> the jump label mutex from inside the preempt disable section
> and still keep the order correct between the mutex and the
> kprobes lock.

Ah, right. Thank you for fixing it.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

> 
> Reported-by: Ingo Molnar <mingo@elte.hu>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/kprobes.c |   26 +++++++++++++-------------
>  1 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 9437e14..9737a76 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1145,17 +1145,13 @@ int __kprobes register_kprobe(struct kprobe *p)
>  	if (ret)
>  		return ret;
>  
> -	preempt_disable();
>  	jump_label_lock();
> +	preempt_disable();
>  	if (!kernel_text_address((unsigned long) p->addr) ||
>  	    in_kprobes_functions((unsigned long) p->addr) ||
>  	    ftrace_text_reserved(p->addr, p->addr) ||
> -	    jump_label_text_reserved(p->addr, p->addr)) {
> -		preempt_enable();
> -		jump_label_unlock();
> -		return -EINVAL;
> -	}
> -	jump_label_unlock();
> +	    jump_label_text_reserved(p->addr, p->addr))
> +		goto fail_with_jump_label;
>  
>  	/* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */
>  	p->flags &= KPROBE_FLAG_DISABLED;
> @@ -1169,10 +1165,9 @@ int __kprobes register_kprobe(struct kprobe *p)
>  		 * We must hold a refcount of the probed module while updating
>  		 * its code to prohibit unexpected unloading.
>  		 */
> -		if (unlikely(!try_module_get(probed_mod))) {
> -			preempt_enable();
> -			return -EINVAL;
> -		}
> +		if (unlikely(!try_module_get(probed_mod)))
> +			goto fail_with_jump_label;
> +
>  		/*
>  		 * If the module freed .init.text, we couldn't insert
>  		 * kprobes in there.
> @@ -1180,11 +1175,11 @@ int __kprobes register_kprobe(struct kprobe *p)
>  		if (within_module_init((unsigned long)p->addr, probed_mod) &&
>  		    probed_mod->state != MODULE_STATE_COMING) {
>  			module_put(probed_mod);
> -			preempt_enable();
> -			return -EINVAL;
> +			goto fail_with_jump_label;
>  		}
>  	}
>  	preempt_enable();
> +	jump_label_unlock();
>  
>  	p->nmissed = 0;
>  	INIT_LIST_HEAD(&p->list);
> @@ -1226,6 +1221,11 @@ out:
>  		module_put(probed_mod);
>  
>  	return ret;
> +
> +fail_with_jump_label:
> +	preempt_enable();
> +	jump_label_unlock();
> +	return -EINVAL;
>  }
>  EXPORT_SYMBOL_GPL(register_kprobe);
>  


-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH 4/8] jump label: Make arch_jump_label_text_poke_early() optional

[Steven Rostedt]

[-- Attachment #1: 0004-jump-label-Make-arch_jump_label_text_poke_early-opti.patch --]
[-- Type: text/plain, Size: 2318 bytes --]

From: David Daney <ddaney@caviumnetworks.com>

For the forthcoming MIPS jump label support,
arch_jump_label_text_poke_early() is unneeded as the MIPS NOP
instruction is already optimal.

Supply a default implementation that does nothing.  Flag x86 and SPARC
as having arch_jump_label_text_poke_early().

Cc: Jason Baron <jbaron@redhat.com>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
LKML-Reference: <1286218615-24011-2-git-send-email-ddaney@caviumnetworks.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sparc/include/asm/jump_label.h |    1 +
 arch/x86/include/asm/jump_label.h   |    1 +
 include/linux/jump_label.h          |    6 ++++++
 3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
index 62e66d7..9aa82d7 100644
--- a/arch/sparc/include/asm/jump_label.h
+++ b/arch/sparc/include/asm/jump_label.h
@@ -6,6 +6,7 @@
 #include <linux/types.h>
 #include <asm/system.h>
 
+#define HAVE_ARCH_JUMP_LABEL_TEXT_POKE_EARLY
 #define JUMP_LABEL_NOP_SIZE 4
 
 #define JUMP_LABEL(key, label)					\
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index f52d42e..169cfd8 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -6,6 +6,7 @@
 #include <linux/types.h>
 #include <asm/nops.h>
 
+#define HAVE_ARCH_JUMP_LABEL_TEXT_POKE_EARLY
 #define JUMP_LABEL_NOP_SIZE 5
 
 # define JUMP_LABEL_INITIAL_NOP ".byte 0xe9 \n\t .long 0\n\t"
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 1947a12..b3835f6 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -22,7 +22,13 @@ extern void jump_label_lock(void);
 extern void jump_label_unlock(void);
 extern void arch_jump_label_transform(struct jump_entry *entry,
 				 enum jump_label_type type);
+
+#ifdef HAVE_ARCH_JUMP_LABEL_TEXT_POKE_EARLY
 extern void arch_jump_label_text_poke_early(jump_label_t addr);
+#else
+static inline void arch_jump_label_text_poke_early(jump_label_t addr) {}
+#endif
+
 extern void jump_label_update(unsigned long key, enum jump_label_type type);
 extern void jump_label_apply_nops(struct module *mod);
 extern int jump_label_text_reserved(void *start, void *end);
-- 
1.7.1

Re: [PATCH 4/8] jump label: Make arch_jump_label_text_poke_early() optional

[David Miller]

From: Steven Rostedt <rostedt@goodmis.org>
Date: Thu, 28 Oct 2010 09:55:52 -0400

> From: David Daney <ddaney@caviumnetworks.com>
> 
> For the forthcoming MIPS jump label support,
> arch_jump_label_text_poke_early() is unneeded as the MIPS NOP
> instruction is already optimal.
> 
> Supply a default implementation that does nothing.  Flag x86 and SPARC
> as having arch_jump_label_text_poke_early().
> 
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
> LKML-Reference: <1286218615-24011-2-git-send-email-ddaney@caviumnetworks.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

On the SPARC side, it's not that the nop isn't optimal, on sparc it's
always 0x01000000.  The issue is that when written an I-cache flush is
necessary using a 'flushi' instruction.

Does MIPS not need a flush when poking instructions?  I find this hard
to believe, although it's been some time since I last touched that
architecture :-)

Re: [PATCH 4/8] jump label: Make arch_jump_label_text_poke_early() optional

[David Daney]

On 10/29/2010 01:37 PM, David Miller wrote:
> From: Steven Rostedt<rostedt@goodmis.org>
> Date: Thu, 28 Oct 2010 09:55:52 -0400
>
>> From: David Daney<ddaney@caviumnetworks.com>
>>
>> For the forthcoming MIPS jump label support,
>> arch_jump_label_text_poke_early() is unneeded as the MIPS NOP
>> instruction is already optimal.
>>
>> Supply a default implementation that does nothing.  Flag x86 and SPARC
>> as having arch_jump_label_text_poke_early().
>>
>> Cc: Jason Baron<jbaron@redhat.com>
>> Cc: David Miller<davem@davemloft.net>
>> Signed-off-by: David Daney<ddaney@caviumnetworks.com>
>> LKML-Reference:<1286218615-24011-2-git-send-email-ddaney@caviumnetworks.com>
>> Signed-off-by: Steven Rostedt<rostedt@goodmis.org>
>
> On the SPARC side, it's not that the nop isn't optimal, on sparc it's
> always 0x01000000.

That's what I thought.  You could remove your
arch_jump_label_text_poke_early() function to no determent.

> The issue is that when written an I-cache flush is
> necessary using a 'flushi' instruction.
>
> Does MIPS not need a flush when poking instructions?

Yes, it does.

> I find this hard to believe, although it's been some time since I
> last touched that architecture :-)

I just looked at my patch again.  I am indeed flushing the I cache
after patching the code, so I don't really know what you are talking
about.

David Daney

[PATCH 5/8] jump label: Add MIPS support.

[Steven Rostedt]

[-- Attachment #1: 0005-jump-label-Add-MIPS-support.patch --]
[-- Type: text/plain, Size: 5255 bytes --]

From: David Daney <ddaney@caviumnetworks.com>

When in Rome...

In order not to be left behind, we add jump label support for MIPS.

Tested on 64-bit big endian (Octeon), and 32-bit little endian
(malta/qemu).

Cc: Jason Baron <jbaron@redhat.com>
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
LKML-Reference: <1286218615-24011-3-git-send-email-ddaney@caviumnetworks.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/mips/Kconfig                  |    1 +
 arch/mips/include/asm/jump_label.h |   48 ++++++++++++++++++++++++++++++++++
 arch/mips/kernel/Makefile          |    3 +-
 arch/mips/kernel/jump_label.c      |   50 ++++++++++++++++++++++++++++++++++++
 arch/mips/kernel/module.c          |    5 +++
 5 files changed, 106 insertions(+), 1 deletions(-)
 create mode 100644 arch/mips/include/asm/jump_label.h
 create mode 100644 arch/mips/kernel/jump_label.c

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 46cae2b..ebea886 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -12,6 +12,7 @@ config MIPS
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
+	select HAVE_ARCH_JUMP_LABEL
 	select RTC_LIB if !MACH_LOONGSON
 	select GENERIC_ATOMIC64 if !64BIT
 
diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
new file mode 100644
index 0000000..7622ccf
--- /dev/null
+++ b/arch/mips/include/asm/jump_label.h
@@ -0,0 +1,48 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (c) 2010 Cavium Networks, Inc.
+ */
+#ifndef _ASM_MIPS_JUMP_LABEL_H
+#define _ASM_MIPS_JUMP_LABEL_H
+
+#include <linux/types.h>
+
+#ifdef __KERNEL__
+
+#define JUMP_LABEL_NOP_SIZE 4
+
+#ifdef CONFIG_64BIT
+#define WORD_INSN ".dword"
+#else
+#define WORD_INSN ".word"
+#endif
+
+#define JUMP_LABEL(key, label)						\
+	do {								\
+		asm goto("1:\tnop\n\t"					\
+			"nop\n\t"					\
+			".pushsection __jump_table,  \"a\"\n\t"		\
+			WORD_INSN " 1b, %l[" #label "], %0\n\t"		\
+			".popsection\n\t"				\
+			: :  "i" (key) :  : label);			\
+	} while (0)
+
+
+#endif /* __KERNEL__ */
+
+#ifdef CONFIG_64BIT
+typedef u64 jump_label_t;
+#else
+typedef u32 jump_label_t;
+#endif
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
+#endif /* _ASM_MIPS_JUMP_LABEL_H */
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 8088498..c4a40e9 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -6,7 +6,8 @@ extra-y		:= head.o init_task.o vmlinux.lds
 
 obj-y		+= cpu-probe.o branch.o entry.o genex.o irq.o process.o \
 		   ptrace.o reset.o setup.o signal.o syscall.o \
-		   time.o topology.o traps.o unaligned.o watch.o vdso.o
+		   time.o topology.o traps.o unaligned.o watch.o vdso.o \
+		   jump_label.o
 
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_ftrace.o = -pg
diff --git a/arch/mips/kernel/jump_label.c b/arch/mips/kernel/jump_label.c
new file mode 100644
index 0000000..52d3c70
--- /dev/null
+++ b/arch/mips/kernel/jump_label.c
@@ -0,0 +1,50 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (c) 2010 Cavium Networks, Inc.
+ */
+
+#include <linux/jump_label.h>
+#include <linux/kernel.h>
+#include <linux/memory.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/cpu.h>
+
+#include <asm/cacheflush.h>
+#include <asm/inst.h>
+
+#define J_RANGE_MASK ((1ul << 28) - 1)
+
+void arch_jump_label_transform(struct jump_entry *e,
+			       enum jump_label_type type)
+{
+	union mips_instruction insn;
+	union mips_instruction *insn_p =
+		(union mips_instruction *)(unsigned long)e->code;
+
+	/* Jump only works within a 256MB aligned region. */
+	BUG_ON((e->target & ~J_RANGE_MASK) != (e->code & ~J_RANGE_MASK));
+
+	/* Target must have 4 byte alignment. */
+	BUG_ON((e->target & 3) != 0);
+
+	if (type == JUMP_LABEL_ENABLE) {
+		insn.j_format.opcode = j_op;
+		insn.j_format.target = (e->target & J_RANGE_MASK) >> 2;
+	} else {
+		insn.word = 0; /* nop */
+	}
+
+	get_online_cpus();
+	mutex_lock(&text_mutex);
+	*insn_p = insn;
+
+	flush_icache_range((unsigned long)insn_p,
+			   (unsigned long)insn_p + sizeof(*insn_p));
+
+	mutex_unlock(&text_mutex);
+	put_online_cpus();
+}
diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 6f51dda..bb9cde4 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -30,6 +30,8 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
+#include <linux/jump_label.h>
+
 #include <asm/pgtable.h>	/* MODULE_START */
 
 struct mips_hi16 {
@@ -390,6 +392,9 @@ int module_finalize(const Elf_Ehdr *hdr,
 	const Elf_Shdr *s;
 	char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
 
+	/* Make jump label nops. */
+	jump_label_apply_nops(me);
+
 	INIT_LIST_HEAD(&me->arch.dbe_list);
 	for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
 		if (strcmp("__dbe_table", secstrings + s->sh_name) != 0)
-- 
1.7.1

[PATCH 6/8] jump_label: Fix unaligned traps on sparc.

[Steven Rostedt]

[-- Attachment #1: 0006-jump_label-Fix-unaligned-traps-on-sparc.patch --]
[-- Type: text/plain, Size: 1049 bytes --]

From: David Miller <davem@davemloft.net>

The vmlinux.lds.h knobs to emit the __jump_table section in the main
kernel image takes care to align the section, but this doesn't help
for the __jump_table section that gets emitted into modules.

Fix the resulting lack of section alignment by explicitly specifying
it in the assembler.

Signed-off-by: David S. Miller <davem@davemloft.net>
LKML-Reference: <20101023.110624.226758370.davem@davemloft.net>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sparc/include/asm/jump_label.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
index 9aa82d7..8ae160a 100644
--- a/arch/sparc/include/asm/jump_label.h
+++ b/arch/sparc/include/asm/jump_label.h
@@ -15,6 +15,7 @@
 			 "nop\n\t"				\
 			 "nop\n\t"				\
 			 ".pushsection __jump_table,  \"a\"\n\t"\
+			 ".align 4\n\t"				\
 			 ".word 1b, %l[" #label "], %c0\n\t"	\
 			 ".popsection \n\t"			\
 			 : :  "i" (key) :  : label);\
-- 
1.7.1

[PATCH 7/8] x86, ftrace: Use safe noops, drop trap test

[Steven Rostedt]

[-- Attachment #1: 0007-x86-ftrace-Use-safe-noops-drop-trap-test.patch --]
[-- Type: text/plain, Size: 3316 bytes --]

From: H. Peter Anvin <hpa@zytor.com>

Always use a safe 5-byte noop sequence.  Drop the trap test, since it
is known to return false negatives on some virtualization platforms on
32 bits.  The resulting code is both simpler and safer.

Cc: Daniel Drake <dsd@laptop.org>
Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/alternative.c |   69 +++++++++--------------------------------
 1 files changed, 15 insertions(+), 54 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index a36bb90..0b30214 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -644,65 +644,26 @@ void *__kprobes text_poke_smp(void *addr, const void *opcode, size_t len)
 
 #if defined(CONFIG_DYNAMIC_FTRACE) || defined(HAVE_JUMP_LABEL)
 
-unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
+#ifdef CONFIG_X86_64
+unsigned char ideal_nop5[5] = { 0x66, 0x66, 0x66, 0x66, 0x90 };
+#else
+unsigned char ideal_nop5[5] = { 0x3e, 0x8d, 0x74, 0x26, 0x00 };
+#endif
 
 void __init arch_init_ideal_nop5(void)
 {
-	extern const unsigned char ftrace_test_p6nop[];
-	extern const unsigned char ftrace_test_nop5[];
-	extern const unsigned char ftrace_test_jmp[];
-	int faulted = 0;
-
 	/*
-	 * There is no good nop for all x86 archs.
-	 * We will default to using the P6_NOP5, but first we
-	 * will test to make sure that the nop will actually
-	 * work on this CPU. If it faults, we will then
-	 * go to a lesser efficient 5 byte nop. If that fails
-	 * we then just use a jmp as our nop. This isn't the most
-	 * efficient nop, but we can not use a multi part nop
-	 * since we would then risk being preempted in the middle
-	 * of that nop, and if we enabled tracing then, it might
-	 * cause a system crash.
+	 * There is no good nop for all x86 archs.  This selection
+	 * algorithm should be unified with the one in find_nop_table(),
+	 * but this should be good enough for now.
 	 *
-	 * TODO: check the cpuid to determine the best nop.
+	 * For cases other than the ones below, use the safe (as in
+	 * always functional) defaults above.
 	 */
-	asm volatile (
-		"ftrace_test_jmp:"
-		"jmp ftrace_test_p6nop\n"
-		"nop\n"
-		"nop\n"
-		"nop\n"  /* 2 byte jmp + 3 bytes */
-		"ftrace_test_p6nop:"
-		P6_NOP5
-		"jmp 1f\n"
-		"ftrace_test_nop5:"
-		".byte 0x66,0x66,0x66,0x66,0x90\n"
-		"1:"
-		".section .fixup, \"ax\"\n"
-		"2:	movl $1, %0\n"
-		"	jmp ftrace_test_nop5\n"
-		"3:	movl $2, %0\n"
-		"	jmp 1b\n"
-		".previous\n"
-		_ASM_EXTABLE(ftrace_test_p6nop, 2b)
-		_ASM_EXTABLE(ftrace_test_nop5, 3b)
-		: "=r"(faulted) : "0" (faulted));
-
-	switch (faulted) {
-	case 0:
-		pr_info("converting mcount calls to 0f 1f 44 00 00\n");
-		memcpy(ideal_nop5, ftrace_test_p6nop, IDEAL_NOP_SIZE_5);
-		break;
-	case 1:
-		pr_info("converting mcount calls to 66 66 66 66 90\n");
-		memcpy(ideal_nop5, ftrace_test_nop5, IDEAL_NOP_SIZE_5);
-		break;
-	case 2:
-		pr_info("converting mcount calls to jmp . + 5\n");
-		memcpy(ideal_nop5, ftrace_test_jmp, IDEAL_NOP_SIZE_5);
-		break;
-	}
-
+#ifdef CONFIG_X86_64
+	/* Don't use these on 32 bits due to broken virtualizers */
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		memcpy(ideal_nop5, p6_nops[5], 5);
+#endif
 }
 #endif
-- 
1.7.1

[PATCH 8/8] jump label: Disable due to compiler bug

[Steven Rostedt]

[-- Attachment #1: 0008-jump-label-Disable-due-to-compiler-bug.patch --]
[-- Type: text/plain, Size: 1332 bytes --]

From: Jason Baron <jbaron@redhat.com>

Unfortunately, we found a compiler bug in the implementation of
'asm goto'. The bug can cause the kernel to crash.

For now, we are disabling jump labels with a big hammer. When the
gcc fix is committed, we will update the kernel with a better check
for either the version number it's fix in, or some detection of
whether gcc has the fix in place.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <2d1b1d5d2b3930e1acb063cefffbcb2faf049854.1288212486.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/jump_label.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index b3835f6..d9d367c 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -1,11 +1,20 @@
 #ifndef _LINUX_JUMP_LABEL_H
 #define _LINUX_JUMP_LABEL_H
 
+/*
+ * A compiler bug was found in the implementation of 'asm goto'. Thus,
+ * we are disabling it for now pending a better check for compiler version
+ * that fixes it.
+ */
+#if 0
+
 #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_HAVE_ARCH_JUMP_LABEL)
 # include <asm/jump_label.h>
 # define HAVE_JUMP_LABEL
 #endif
 
+#endif
+
 enum jump_label_type {
 	JUMP_LABEL_ENABLE,
 	JUMP_LABEL_DISABLE
-- 
1.7.1