[PATCH v2 0/2] jump label: Add MIPS architecture support.

[PATCH v2 0/2] jump label: Add MIPS architecture support.

[David Daney]

v2: Make arch_jump_label_text_poke_early() optional.  As pointed out
by Rabin Vincent, the MIPS NOP is already optimal and does not need to
be replaced at boot time.  It is possible that SPARC should leave
arch_jump_label_text_poke_early() unimplementd, but I leave that work
for others as I cannot test it.

v1: Add MIPS jump label support.


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

 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 |    1 +
 arch/x86/include/asm/jump_label.h   |    1 +
 include/linux/jump_label.h          |    6 ++++
 8 files changed, 114 insertions(+), 1 deletions(-)
 create mode 100644 arch/mips/include/asm/jump_label.h
 create mode 100644 arch/mips/kernel/jump_label.c

Cc: David Miller <davem@davemloft.net>
-- 
1.7.2.2

[PATCH v2 1/2] jump label: Make arch_jump_label_text_poke_early() optional

[David Daney]

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().

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jason Baron <jbaron@redhat.com>
Cc: David Miller <davem@davemloft.net>
---
 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 b72cd9f..e98ad3a 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -20,7 +20,13 @@ extern struct jump_entry __stop___jump_table[];
 
 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.2.2

[PATCH v2 2/2] jump label: Add MIPS support.

[David Daney]

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).

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.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 3ad59dd..6b3bdb5 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -13,6 +13,7 @@ config MIPS
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select RTC_LIB if !MACH_LOONGSON
+	select HAVE_ARCH_JUMP_LABEL
 
 mainmenu "Linux/MIPS Kernel Configuration"
 
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 06f8482..db4feb9 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.2.2

Re: [PATCH v2 2/2] jump label: Add MIPS support.

[Ralf Baechle]

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

Re: [PATCH v2 2/2] jump label: Add MIPS support.

[David Daney]

On 10/06/2010 04:00 PM, Ralf Baechle wrote:
> Acked-by: Ralf Baechle<ralf@linux-mips.org>
>
>    Ralf
>

That should complete the Acked-by prerequisites for this set.  I hope 
Steven and Jason can get them merged for 2.6.37 (hint, hint :-))

Thanks,
David Daney

[PATCH 0/9] [GIT PULL] jump label: various 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.

Jason Baron (6):
      jump label/x86: Move arch_init_ideal_nop5 later
      jump label: Fix module __init section race
      jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex
      jump label: Add register_jump_label_key/unregister_jump_label_key
      jump label: Move jump table to r/w section
      jump label: Add docs

Steven Rostedt (1):
      tracing/x86: No need to disable interrupts when calling arch_init_ideal_nop5

----
 Documentation/jump-label.txt        |  142 ++++++++++++++++++++++
 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 |    1 +
 arch/x86/include/asm/jump_label.h   |    1 +
 arch/x86/kernel/alternative.c       |    5 +
 arch/x86/kernel/setup.c             |    6 -
 include/asm-generic/vmlinux.lds.h   |   14 +--
 include/linux/jump_label.h          |   15 +++
 kernel/jump_label.c                 |  222 ++++++++++++++++++++++++++++++-----
 kernel/kprobes.c                    |    6 +
 kernel/trace/ftrace.c               |    4 +-
 kernel/tracepoint.c                 |   34 ++++++
 lib/dynamic_debug.c                 |   20 +++
 17 files changed, 526 insertions(+), 51 deletions(-)

[PATCH 1/9] jump label/x86: Move arch_init_ideal_nop5 later

[Steven Rostedt]

[-- Attachment #1: 0001-jump-label-x86-Move-arch_init_ideal_nop5-later.patch --]
[-- Type: text/plain, Size: 2068 bytes --]

From: Jason Baron <jbaron@redhat.com>

arch_init_ideal_nop5() was being called from setup_arch() before
the exception table was setup. Move it later into
alternative_instructions().

Fixes a boot hang on OLPC's XO-1 laptop based on Geode LX
processor.

Reported-by: Daniel Drake <dsd@laptop.org>
Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <20101004154633.GA2900@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/alternative.c |    5 +++++
 arch/x86/kernel/setup.c       |    6 ------
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index a36bb90..83a56a2 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -454,6 +454,7 @@ extern struct paravirt_patch_site __start_parainstructions[],
 
 void __init alternative_instructions(void)
 {
+	unsigned long flags;
 	/* The patching is not fully atomic, so try to avoid local interruptions
 	   that might execute the to be patched code.
 	   Other CPUs are not running. */
@@ -508,6 +509,10 @@ void __init alternative_instructions(void)
 				(unsigned long)__smp_locks_end);
 
 	restart_nmi();
+
+	local_irq_save(flags);
+	arch_init_ideal_nop5();
+	local_irq_restore(flags);
 }
 
 /**
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 00e1678..c3a4fbb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -112,7 +112,6 @@
 #include <asm/numa_64.h>
 #endif
 #include <asm/mce.h>
-#include <asm/alternative.h>
 
 /*
  * end_pfn only includes RAM, while max_pfn_mapped includes all e820 entries.
@@ -727,7 +726,6 @@ void __init setup_arch(char **cmdline_p)
 {
 	int acpi = 0;
 	int k8 = 0;
-	unsigned long flags;
 
 #ifdef CONFIG_X86_32
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
@@ -1073,10 +1071,6 @@ void __init setup_arch(char **cmdline_p)
 	x86_init.oem.banner();
 
 	mcheck_init();
-
-	local_irq_save(flags);
-	arch_init_ideal_nop5();
-	local_irq_restore(flags);
 }
 
 #ifdef CONFIG_X86_32
-- 
1.7.1

[PATCH 2/9] tracing/x86: No need to disable interrupts when calling arch_init_ideal_nop5

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-x86-No-need-to-disable-interrupts-when-calli.patch --]
[-- Type: text/plain, Size: 1048 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The function arch_init_ideal_nop5() is taken from the original
ftrace_dyn_arch_init() that originally had interrupts disabled
when calling it. This was due to the old way that mcounts were converted
to nops. The disabling of interrupts is no longer needed for the
work that is done.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 65fb077..1ce8f24 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2795,15 +2795,13 @@ extern unsigned long __stop_mcount_loc[];
 
 void __init ftrace_init(void)
 {
-	unsigned long count, addr, flags;
+	unsigned long count, addr;
 	int ret;
 
 	/* Keep the ftrace pointer to the stub */
 	addr = (unsigned long)ftrace_stub;
 
-	local_irq_save(flags);
 	ftrace_dyn_arch_init(&addr);
-	local_irq_restore(flags);
 
 	/* ftrace_dyn_arch_init places the return code in addr */
 	if (addr)
-- 
1.7.1

[PATCH 3/9] jump label: Fix module __init section race

[Steven Rostedt]

[-- Attachment #1: 0003-jump-label-Fix-module-__init-section-race.patch --]
[-- Type: text/plain, Size: 3202 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>
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 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex

[Steven Rostedt]

[-- Attachment #1: 0004-jump-label-Fix-deadlock-b-w-jump_label_mutex-vs.-tex.patch --]
[-- Type: text/plain, Size: 5916 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 b72cd9f..cf213d1 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
 
 #endif
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 ec4210c..d45d72e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1145,13 +1145,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;
@@ -1186,6 +1189,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);
 
@@ -1213,6 +1218,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 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Steven Rostedt]

[-- Attachment #1: 0005-jump-label-Add-register_jump_label_key-unregister_ju.patch --]
[-- Type: text/plain, Size: 12770 bytes --]

From: Jason Baron <jbaron@redhat.com>

There are two cases where jump labels didn't match the state
transitions of tracepoints.

1)

A tracepoint is enabled in the core kernel for say kmalloc and
then a module is loaded which has a kmalloc call. We currently
would miss enabling the jump label for the kmalloc in the module.
This is addressed by associating an 'enabled' field with each
jump label. Now, when a new module is loaded we check if any of
the jump labels in that module need to be enabled. If so, we enable
them.

2)

If a tracepoint is defined in the core kernel code, but the usage
of the tracepoint is confined to a module, the current jump label
code does not create a entry in its table until the module is
loaded. Thus, if the tracepoint is enabled before the module
is loaded, we would miss the enablement of the jump label.

I'm not sure if there any tracepoints which currently fall into
this category (the bkl could fall into this category at some point
if its only used in modules). However, I do think its an important
case to address to make sure that jump label behave in a consistent
way with how consumers of the tracepoints might expect.

This case is implemented by introducing:

void register_jump_label_key(unsigned long key);
void unregister_jump_label_key(unsigned long key);

So basically any jump label that we want to use in the system must
first be registered, then it can be enabled/disabled, and then
finally it can be unregistered. For core kernel jump labels, I would
only expect them to be registered and never unregistered. However,
a jump label may be unregistred when modules are removed.

Although, this introduces some more work for consumers wanting
to use jump labels, the tracepoint and dynamic debug consumer code
seems fairly contained, at least to me.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <12fafc1958abf192532a25aedbe6b50a2587c7a3.1285965957.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/jump_label.h |    4 +
 kernel/jump_label.c        |  158 +++++++++++++++++++++++++++++++++++++-------
 kernel/tracepoint.c        |   34 ++++++++++
 lib/dynamic_debug.c        |   20 ++++++
 4 files changed, 193 insertions(+), 23 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index cf213d1..382cd23 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -26,6 +26,8 @@ extern void arch_jump_label_text_poke_early(jump_label_t addr);
 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);
+void register_jump_label_key(unsigned long key);
+void unregister_jump_label_key(unsigned long key);
 
 #define enable_jump_label(key) \
 	jump_label_update((unsigned long)key, JUMP_LABEL_ENABLE);
@@ -63,6 +65,8 @@ static inline int jump_label_text_reserved(void *start, void *end)
 
 static inline void jump_label_lock(void) {}
 static inline void jump_label_unlock(void) {}
+static inline void register_jump_label_key(unsigned long key) {}
+static inline void unregister_jump_label_key(unsigned long key) {}
 
 #endif
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 12cce78..b30ef81 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -19,6 +19,7 @@
 #define JUMP_LABEL_HASH_BITS 6
 #define JUMP_LABEL_TABLE_SIZE (1 << JUMP_LABEL_HASH_BITS)
 static struct hlist_head jump_label_table[JUMP_LABEL_TABLE_SIZE];
+static int registered;
 
 /* mutex to protect coming/going of the the jump_label table */
 static DEFINE_MUTEX(jump_label_mutex);
@@ -26,17 +27,19 @@ static DEFINE_MUTEX(jump_label_mutex);
 struct jump_label_entry {
 	struct hlist_node hlist;
 	struct jump_entry *table;
-	int nr_entries;
 	/* hang modules off here */
 	struct hlist_head modules;
 	unsigned long key;
+	__u32 nr_entries : 30;
+	__u32 registered : 1;
+	__u32 enabled : 1;
 };
 
 struct jump_label_module_entry {
 	struct hlist_node hlist;
 	struct jump_entry *table;
-	int nr_entries;
 	struct module *mod;
+	int nr_entries;
 };
 
 void jump_label_lock(void)
@@ -108,6 +111,8 @@ add_jump_label_entry(jump_label_t key, int nr_entries, struct jump_entry *table)
 	e->key = key;
 	e->table = table;
 	e->nr_entries = nr_entries;
+	e->enabled = 0;
+	e->registered = 0;
 	INIT_HLIST_HEAD(&(e->modules));
 	hlist_add_head(&e->hlist, head);
 	return e;
@@ -163,28 +168,43 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 	int count;
 
 	jump_label_lock();
+	if (unlikely(!registered)) {
+		WARN(1, KERN_ERR "jump label: update before activation!\n");
+		goto out;
+	}
 	entry = get_jump_label_entry((jump_label_t)key);
-	if (entry) {
-		count = entry->nr_entries;
-		iter = entry->table;
+	if (unlikely(!entry)) {
+		WARN(1, KERN_ERR "jump label: updating unkown key: %lu\n", key);
+		goto out;
+	}
+	if (unlikely(!entry->registered)) {
+		WARN(1, KERN_ERR "jump label: update without register:"
+			" %lu\n", key);
+		goto out;
+	}
+	if (type == JUMP_LABEL_ENABLE)
+		entry->enabled = 1;
+	else
+		entry->enabled = 0;
+	count = entry->nr_entries;
+	iter = entry->table;
+	while (count--) {
+		if (kernel_text_address(iter->code))
+			arch_jump_label_transform(iter, type);
+		iter++;
+	}
+	/* eanble/disable jump labels in modules */
+	hlist_for_each_entry(e_module, module_node, &(entry->modules),
+						hlist) {
+		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++;
 		}
-		/* eanble/disable jump labels in modules */
-		hlist_for_each_entry(e_module, module_node, &(entry->modules),
-							hlist) {
-			count = e_module->nr_entries;
-			iter = e_module->table;
-			while (count--) {
-				if (iter->key &&
-						kernel_text_address(iter->code))
-					arch_jump_label_transform(iter, type);
-				iter++;
-			}
-		}
 	}
+out:
 	jump_label_unlock();
 }
 
@@ -197,6 +217,63 @@ static int addr_conflict(struct jump_entry *entry, void *start, void *end)
 	return 0;
 }
 
+/***
+ * register_jump_label_key - register a jump label key
+ * @key -  key value associated with a a jump label
+ *
+ * Code that wants to use a jump label must first register the key before
+ * before calling jump_label_update on it.
+ */
+
+void register_jump_label_key(unsigned long key)
+{
+	struct jump_label_entry *entry;
+
+	jump_label_lock();
+	entry = get_jump_label_entry((jump_label_t)key);
+	if (!entry)
+		entry = add_jump_label_entry((jump_label_t)key, 0, NULL);
+	if (!entry)
+		goto out;
+	if (entry->registered)
+		WARN(1, KERN_ERR "jump label: re-register key: %lu !\n", key);
+	else
+		entry->registered = 1;
+out:
+	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(register_jump_label_key);
+
+/***
+ * unregister_jump_label_key - unregister a jump label key
+ * @key -  key value associated with a a jump label
+ *
+ * Code that no longer needs to make use of jump label (such as a module)
+ * needs to call this to clean up state.
+ */
+
+void unregister_jump_label_key(unsigned long key)
+{
+	struct jump_label_entry *entry;
+
+	jump_label_lock();
+	entry = get_jump_label_entry((jump_label_t)key);
+	if (!entry) {
+		jump_label_unlock();
+		WARN(1, KERN_ERR "jump label: remove unknown key: %lu\n", key);
+		return;
+	}
+	if (entry->nr_entries || (!hlist_empty(&entry->modules)))
+		WARN(1, KERN_ERR "jump label: remove used key: %lu\n", key);
+	else {
+		entry->registered = 0;
+		hlist_del(&entry->hlist);
+		kfree(entry);
+	}
+	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(unregister_jump_label_key);
+
 #ifdef CONFIG_MODULES
 
 static int module_conflict(void *start, void *end)
@@ -285,6 +362,7 @@ static __init int init_jump_label(void)
 		arch_jump_label_text_poke_early(iter->code);
 		iter++;
 	}
+	registered = 1;
 	jump_label_unlock();
 	return ret;
 }
@@ -309,6 +387,41 @@ add_jump_label_module_entry(struct jump_label_entry *entry,
 	return e;
 }
 
+static void update_jump_label_module(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) {
+			if (!e->enabled)
+				continue;
+			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--) {
+					arch_jump_label_transform(iter,
+							JUMP_LABEL_ENABLE);
+					iter++;
+				}
+			}
+		}
+	}
+}
+
 static int add_jump_label_module(struct module *mod)
 {
 	struct jump_entry *iter, *iter_begin;
@@ -342,6 +455,9 @@ static int add_jump_label_module(struct module *mod)
 		if (IS_ERR(module_entry))
 			return PTR_ERR(module_entry);
 	}
+	/* update new entries to the correct state */
+	update_jump_label_module(mod);
+
 	return 0;
 }
 
@@ -368,10 +484,6 @@ static void remove_jump_label_module(struct module *mod)
 					kfree(e_module);
 				}
 			}
-			if (hlist_empty(&e->modules) && (e->nr_entries == 0)) {
-				hlist_del(&e->hlist);
-				kfree(e);
-			}
 		}
 	}
 }
@@ -463,7 +575,7 @@ void jump_label_apply_nops(struct module *mod)
 
 struct notifier_block jump_label_module_nb = {
 	.notifier_call = jump_label_module_notify,
-	.priority = 0,
+	.priority = 1, /* higher than tracepoints */
 };
 
 static __init int init_jump_label_module(void)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index d6073a5..00e4074 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -575,6 +575,33 @@ void tracepoint_iter_reset(struct tracepoint_iter *iter)
 }
 EXPORT_SYMBOL_GPL(tracepoint_iter_reset);
 
+
+static void jump_label_reg_unreg(struct tracepoint *begin,
+				 struct tracepoint *end, int reg)
+{
+	struct tracepoint *iter;
+	if (!begin)
+		return;
+
+	mutex_lock(&tracepoints_mutex);
+	for (iter = begin; iter < end; iter++) {
+		if (reg)
+			register_jump_label_key((unsigned long)&iter->state);
+		else
+			unregister_jump_label_key((unsigned long)&iter->state);
+	}
+	mutex_unlock(&tracepoints_mutex);
+}
+#define jump_label_register(begin, end) jump_label_reg_unreg(begin, end, 1);
+#define jump_label_unregister(begin, end) jump_label_reg_unreg(begin, end, 0);
+
+static int __init init_jump_label_register(void)
+{
+	jump_label_register(__start___tracepoints, __stop___tracepoints);
+	return 0;
+}
+core_initcall(init_jump_label_register);
+
 #ifdef CONFIG_MODULES
 
 int tracepoint_module_notify(struct notifier_block *self,
@@ -584,9 +611,16 @@ int tracepoint_module_notify(struct notifier_block *self,
 
 	switch (val) {
 	case MODULE_STATE_COMING:
+		jump_label_register(mod->tracepoints,
+			mod->tracepoints + mod->num_tracepoints);
+		tracepoint_update_probe_range(mod->tracepoints,
+			mod->tracepoints + mod->num_tracepoints);
+		break;
 	case MODULE_STATE_GOING:
 		tracepoint_update_probe_range(mod->tracepoints,
 			mod->tracepoints + mod->num_tracepoints);
+		jump_label_unregister(mod->tracepoints,
+			mod->tracepoints + mod->num_tracepoints);
 		break;
 	}
 	return 0;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e925c7b..5ddcb3f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -79,6 +79,24 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
 	return buf;
 }
 
+static void jump_label_reg_unreg(struct ddebug_table *dt, int reg)
+{
+	int i;
+	if (!dt)
+		return;
+
+	for (i = 0 ; i < dt->num_ddebugs ; i++) {
+		struct _ddebug *dp = &dt->ddebugs[i];
+		if (reg)
+			register_jump_label_key((unsigned long)&dp->enabled);
+		else
+			unregister_jump_label_key((unsigned long)&dp->enabled);
+	}
+}
+
+#define jump_label_register(dt) jump_label_reg_unreg(dt, 1);
+#define jump_label_unregister(dt) jump_label_reg_unreg(dt, 0);
+
 /*
  * Search the tables for _ddebug's which match the given
  * `query' and apply the `flags' and `mask' to them.  Tells
@@ -636,6 +654,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 
 	mutex_lock(&ddebug_lock);
 	list_add_tail(&dt->link, &ddebug_tables);
+	jump_label_register(dt);
 	mutex_unlock(&ddebug_lock);
 
 	if (verbose)
@@ -647,6 +666,7 @@ EXPORT_SYMBOL_GPL(ddebug_add_module);
 
 static void ddebug_table_free(struct ddebug_table *dt)
 {
+	jump_label_unregister(dt);
 	list_del_init(&dt->link);
 	kfree(dt->mod_name);
 	kfree(dt);
-- 
1.7.1

[PATCH 6/9] jump label: Move jump table to r/w section

[Steven Rostedt]

[-- Attachment #1: 0006-jump-label-Move-jump-table-to-r-w-section.patch --]
[-- Type: text/plain, Size: 1669 bytes --]

From: Jason Baron <jbaron@redhat.com>

Since we are writing the jump table it should be be in R/W kernel
section. Move it to DATA_DATA

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <e37e4c270ba893644ef3c2f0365e2448b8b3981b.1285965957.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/asm-generic/vmlinux.lds.h |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ef2af99..24e2daf 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -160,6 +160,10 @@
 	VMLINUX_SYMBOL(__start___tracepoints) = .;			\
 	*(__tracepoints)						\
 	VMLINUX_SYMBOL(__stop___tracepoints) = .;			\
+	. = ALIGN(8);							\
+	VMLINUX_SYMBOL(__start___jump_table) = .;			\
+	*(__jump_table)							\
+	VMLINUX_SYMBOL(__stop___jump_table) = .;			\
 	/* implement dynamic printk debug */				\
 	. = ALIGN(8);							\
 	VMLINUX_SYMBOL(__start___verbose) = .;                          \
@@ -220,8 +224,6 @@
 									\
 	BUG_TABLE							\
 									\
-	JUMP_TABLE							\
-									\
 	/* PCI quirks */						\
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
 		VMLINUX_SYMBOL(__start_pci_fixups_early) = .;		\
@@ -565,14 +567,6 @@
 #define BUG_TABLE
 #endif
 
-#define JUMP_TABLE							\
-	. = ALIGN(8);							\
-	__jump_table : AT(ADDR(__jump_table) - LOAD_OFFSET) {		\
-		VMLINUX_SYMBOL(__start___jump_table) = .;		\
-		*(__jump_table)						\
-		VMLINUX_SYMBOL(__stop___jump_table) = .;		\
-	}
-
 #ifdef CONFIG_PM_TRACE
 #define TRACEDATA							\
 	. = ALIGN(4);							\
-- 
1.7.1

[PATCH 7/9] jump label: Add docs

[Steven Rostedt]

[-- Attachment #1: 0007-jump-label-Add-docs.patch --]
[-- Type: text/plain, Size: 6005 bytes --]

From: Jason Baron <jbaron@redhat.com>

Add jump label docs as: Documentation/jump-label.txt

Also, fix documentation spelling error in jump_label.c

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <9a71c059f86d432ea468728281a8e570556c8a38.1285965957.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/jump-label.txt |  142 ++++++++++++++++++++++++++++++++++++++++++
 kernel/jump_label.c          |    2 +-
 2 files changed, 143 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/jump-label.txt

diff --git a/Documentation/jump-label.txt b/Documentation/jump-label.txt
new file mode 100644
index 0000000..d26df30
--- /dev/null
+++ b/Documentation/jump-label.txt
@@ -0,0 +1,142 @@
+			Jump Label
+			----------
+
+By: Jason Baron <jbaron@redhat.com>
+
+
+1) Motivation
+
+
+Currently, tracepoints are implemented using a conditional. The conditional
+check requires checking a global variable for each tracepoint. Although
+the overhead of this check is small, it increases when the memory cache comes
+under pressure (memory cache lines for these global variables may be shared
+with other memory accesses). As we increase the number of tracepoints in the
+kernel this overhead may become more of an issue. In addition, tracepoints are
+often dormant (disabled) and provide no direct kernel functionality. Thus, it
+is highly desirable to reduce their impact as much as possible. Although
+tracepoints are the original motivation for this work, other kernel code paths
+should be able to make use of the jump label optimization.
+
+
+2) Jump label description/usage
+
+
+gcc (v4.5) adds a new 'asm goto' statement that allows branching to a label.
+http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01556.html
+
+Thus, this patch set introduces an architecture specific 'JUMP_LABEL()' macro as
+follows (x86):
+
+# define JUMP_LABEL_INITIAL_NOP ".byte 0xe9 \n\t .long 0\n\t"
+
+# define JUMP_LABEL(key, label)					\
+	do {                                                    \
+		asm goto("1:"                                   \
+			JUMP_LABEL_INITIAL_NOP			\
+                        ".pushsection __jump_table,  \"a\" \n\t"\
+                        _ASM_PTR "1b, %l[" #label "], %c0 \n\t" \
+                        ".popsection \n\t"                      \
+                        : :  "i" (key) :  : label);		\
+        } while (0)
+
+
+For architectures that have not yet introduced jump label support it's simply:
+
+#define JUMP_LABEL(key, label)			\
+	if (unlikely(*key))			\
+		goto label;
+
+which then can be used as:
+
+	....
+        JUMP_LABEL(key, trace_label);
+        printk("not doing tracing\n");
+	return;
+trace_label:
+        printk("doing tracing: %d\n", file);
+	....
+
+The 'key' argument is thus a pointer to a conditional argument that can be used
+if the optimization is not enabled. Otherwise, this address serves as a unique
+key to identify the particular instance of the jump label.
+
+Thus, when tracing is disabled, we simply have a no-op followed by a jump around
+the dormant (disabled) tracing code. The 'JUMP_LABEL()' macro produces a
+'jump_table', which has the following format:
+
+[instruction address] [jump target] [key]
+
+Thus, to enable a tracepoint, we simply patch the 'instruction address' with
+a jump to the 'jump target.'
+
+The calls to enable and disable a jump label are: enable_jump_label(key) and
+disable_jump_label(key).
+
+
+3) Architecture interface
+
+
+There are a few functions and macros that architectures must implement in order
+to take advantage of this optimization. As previously mentioned, if there is no
+architecture support, we simply fall back to a traditional, load, test, and
+jump sequence.
+
+* add "HAVE_ARCH_JUMP_LABEL" to arch/<arch>/Kconfig to indicate support
+
+* #define JUMP_LABEL_NOP_SIZE, arch/x86/include/asm/jump_label.h
+
+* #define "JUMP_LABEL(key, label)", arch/x86/include/asm/jump_label.h
+
+* void arch_jump_label_transform(struct jump_entry *entry, enum jump_label_type type)
+	see: arch/x86/kernel/jump_label.c
+
+* void arch_jump_label_text_poke_early(jump_label_t addr)
+	see: arch/x86/kernel/jump_label.c
+
+* finally add a definition for "struct jump_entry".
+	see: arch/x86/include/asm/jump_label.h
+
+
+4) Jump label analysis (x86)
+
+
+I've tested the performance of using 'get_cycles()' calls around the
+tracepoint call sites. For an Intel Core 2 Quad cpu (in cycles, averages):
+
+		idle		after tbench run
+		----		----------------
+old code	 32		  88
+new code	  2		   4
+
+
+The performance improvement can be reproduced reliably on both Intel and AMD
+hardware.
+
+In terms of code analysis, the current code for the disabled case is a 'cmpl'
+followed by a 'je' around the tracepoint code. So:
+
+cmpl - 83 3d 0e 77 87 00 00 - 7 bytes
+je   - 74 3e                - 2 bytes
+
+total of 9 instruction bytes.
+
+The new code is a 'nopl' followed by a 'jmp'. Thus:
+
+nopl - 0f 1f 44 00 00 - 5 bytes
+jmp  - eb 3e          - 2 bytes
+
+total of 7 instruction bytes.
+
+So, the new code also accounts for 2 less bytes in the instruction cache per tracepoint.
+
+
+5) Acknowledgements
+
+
+Thanks to Roland McGrath and Richard Henderson for helping come up with the
+initial 'asm goto' and jump label design.
+
+Thanks to Mathieu Desnoyers and H. Peter Anvin for calling attention to this
+issue, and outlining the requirements of a solution. Mathieu also implemented a
+solution in the form of the "Immediate Values" work.
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b30ef81..ebae902 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -193,7 +193,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 			arch_jump_label_transform(iter, type);
 		iter++;
 	}
-	/* eanble/disable jump labels in modules */
+	/* enable/disable jump labels in modules */
 	hlist_for_each_entry(e_module, module_node, &(entry->modules),
 						hlist) {
 		count = e_module->nr_entries;
-- 
1.7.1

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

[Steven Rostedt]

[-- Attachment #1: 0008-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 382cd23..c3562cf 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

[PATCH 9/9] jump label: Add MIPS support.

[Steven Rostedt]

[-- Attachment #1: 0009-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 5526faa..9429df3 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 06f8482..db4feb9 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

Re: [PATCH 0/9] [GIT PULL] jump label: various updates

[Steven Rostedt]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> Ingo,
> 
> Please pull the latest tip/perf/jump-label tree, which can be found at:

Hold on, I forgot to add Jason Baron's fix for compiling against < 4.5.

I'll add that and push it out as jump-label-2

-- Steve

> 
>   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.
> 
> Jason Baron (6):
>       jump label/x86: Move arch_init_ideal_nop5 later
>       jump label: Fix module __init section race
>       jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex
>       jump label: Add register_jump_label_key/unregister_jump_label_key
>       jump label: Move jump table to r/w section
>       jump label: Add docs
> 
> Steven Rostedt (1):
>       tracing/x86: No need to disable interrupts when calling arch_init_ideal_nop5
> 
> ----
>  Documentation/jump-label.txt        |  142 ++++++++++++++++++++++
>  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 |    1 +
>  arch/x86/include/asm/jump_label.h   |    1 +
>  arch/x86/kernel/alternative.c       |    5 +
>  arch/x86/kernel/setup.c             |    6 -
>  include/asm-generic/vmlinux.lds.h   |   14 +--
>  include/linux/jump_label.h          |   15 +++
>  kernel/jump_label.c                 |  222 ++++++++++++++++++++++++++++++-----
>  kernel/kprobes.c                    |    6 +
>  kernel/trace/ftrace.c               |    4 +-
>  kernel/tracepoint.c                 |   34 ++++++
>  lib/dynamic_debug.c                 |   20 +++
>  17 files changed, 526 insertions(+), 51 deletions(-)

Re: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex

[Peter Zijlstra]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> +void jump_label_lock(void)
> +{
> +       mutex_lock(&jump_label_mutex);
> +}
> +

> +++ b/kernel/kprobes.c
> @@ -1145,13 +1145,16 @@ int __kprobes register_kprobe(struct kprobe *p)
>                 return ret;
>  
>         preempt_disable();
> +       jump_label_lock();

How exactly does that work?

>         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(); 

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Peter Zijlstra]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
>  
>  static inline void jump_label_lock(void) {}
>  static inline void jump_label_unlock(void) {}
> +static inline void register_jump_label_key(unsigned long key) {}
> +static inline void unregister_jump_label_key(unsigned long key) {}
> 
shouldn't we call that jump_label_register_key() and co?

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Peter Zijlstra]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> 
> 2)
> 
> If a tracepoint is defined in the core kernel code, but the usage
> of the tracepoint is confined to a module, the current jump label
> code does not create a entry in its table until the module is
> loaded. Thus, if the tracepoint is enabled before the module
> is loaded, we would miss the enablement of the jump label.
> 
> I'm not sure if there any tracepoints which currently fall into
> this category (the bkl could fall into this category at some point
> if its only used in modules). However, I do think its an important
> case to address to make sure that jump label behave in a consistent
> way with how consumers of the tracepoints might expect.
> 
> This case is implemented by introducing:
> 
> void register_jump_label_key(unsigned long key);
> void unregister_jump_label_key(unsigned long key);
> 
> So basically any jump label that we want to use in the system must
> first be registered, then it can be enabled/disabled, and then
> finally it can be unregistered. For core kernel jump labels, I would
> only expect them to be registered and never unregistered. However,
> a jump label may be unregistred when modules are removed.
> 
> Although, this introduces some more work for consumers wanting
> to use jump labels, the tracepoint and dynamic debug consumer code
> seems fairly contained, at least to me. 

Urgh, this sucks.. :-(

So now we have to actually track all JUMP_LABEL() sites and call
register muck on them.. even though we already track them through the
special data section.

Is there really no way around this?

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

[Peter Zijlstra]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> +
> +#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
> + 

How often is this called, do we really need to avoid that call? It looks
like init code.

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

[Steven Rostedt]

On Fri, 2010-10-15 at 23:06 +0200, Peter Zijlstra wrote:
> On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > +
> > +#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
> > + 
> 
> How often is this called, do we really need to avoid that call? It looks
> like init code.

Yeah, that's a good point.

What's the harm in doing a stub in the mips code here?

-- Steve

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Steven Rostedt]

On Fri, 2010-10-15 at 23:03 +0200, Peter Zijlstra wrote:

> Urgh, this sucks.. :-(
> 
> So now we have to actually track all JUMP_LABEL() sites and call
> register muck on them.. even though we already track them through the
> special data section.
> 
> Is there really no way around this?

I'll take a look to see if we can monkey with magic and automate it.

-- Steve "pulling out his macro voodoo handlers" Rostedt

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Peter Zijlstra]

On Fri, 2010-10-15 at 17:09 -0400, Steven Rostedt wrote:
> On Fri, 2010-10-15 at 23:03 +0200, Peter Zijlstra wrote:
> 
> > Urgh, this sucks.. :-(
> > 
> > So now we have to actually track all JUMP_LABEL() sites and call
> > register muck on them.. even though we already track them through the
> > special data section.
> > 
> > Is there really no way around this?
> 
> I'll take a look to see if we can monkey with magic and automate it.

So the problem is something like:

core kernel:

 jump_label_enable()

module:

 JUMP_LABEL()

And then because we don't have a proper __jump_table section, the
jump_label_enable() won't properly work?

Why not let jump_label_enable() add a dummy entry with the enabled bit
and once you load the module merge the real entry into it.

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

[David Daney]

On 10/15/2010 02:08 PM, Steven Rostedt wrote:
> On Fri, 2010-10-15 at 23:06 +0200, Peter Zijlstra wrote:
>> On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
>>> +
>>> +#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
>>> +
>>
>> How often is this called, do we really need to avoid that call? It looks
>> like init code.
>
> Yeah, that's a good point.
>
> What's the harm in doing a stub in the mips code here?
>

Really the SPARC code (and any architectures added in the future with a 
well defined optimal NOP) should be doing nothing here as well.

So I could turn the question around and ask:  How many empty definitions 
of this thing do you want in the tree?

I will defer to Steven.  If he wants to push the empty stubs into all 
the architectures, I will update the MIPS patch and he can remove/revert 
the 'jump label: Make arch_jump_label_text_poke_early() optional' patch.

Let me know which option you would prefer.

David Daney

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

[Steven Rostedt]

On Fri, 2010-10-15 at 14:22 -0700, David Daney wrote:

> Really the SPARC code (and any architectures added in the future with a 
> well defined optimal NOP) should be doing nothing here as well.
> 
> So I could turn the question around and ask:  How many empty definitions 
> of this thing do you want in the tree?
> 
> I will defer to Steven.  If he wants to push the empty stubs into all 
> the architectures, I will update the MIPS patch and he can remove/revert 
> the 'jump label: Make arch_jump_label_text_poke_early() optional' patch.
> 
> Let me know which option you would prefer.

On IRC, Thomas Gleixner mentioned a better solution (something I've done
with ftrace). Just add a "weak" stub function. No need to get Kconfig
involved.

-- Steve

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

[David Daney]

On 10/15/2010 02:35 PM, Steven Rostedt wrote:
> On Fri, 2010-10-15 at 14:22 -0700, David Daney wrote:
>
>> Really the SPARC code (and any architectures added in the future with a
>> well defined optimal NOP) should be doing nothing here as well.
>>
>> So I could turn the question around and ask:  How many empty definitions
>> of this thing do you want in the tree?
>>
>> I will defer to Steven.  If he wants to push the empty stubs into all
>> the architectures, I will update the MIPS patch and he can remove/revert
>> the 'jump label: Make arch_jump_label_text_poke_early() optional' patch.
>>
>> Let me know which option you would prefer.
>
> On IRC, Thomas Gleixner mentioned a better solution (something I've done
> with ftrace). Just add a "weak" stub function. No need to get Kconfig
> involved.
>

Do you want to write that patch, or shall I?


David Daney

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

[Steven Rostedt]

On Fri, 2010-10-15 at 14:38 -0700, David Daney wrote:
> On 10/15/2010 02:35 PM, Steven Rostedt wrote:
> > On Fri, 2010-10-15 at 14:22 -0700, David Daney wrote:
> >
> >> Really the SPARC code (and any architectures added in the future with a
> >> well defined optimal NOP) should be doing nothing here as well.
> >>
> >> So I could turn the question around and ask:  How many empty definitions
> >> of this thing do you want in the tree?
> >>
> >> I will defer to Steven.  If he wants to push the empty stubs into all
> >> the architectures, I will update the MIPS patch and he can remove/revert
> >> the 'jump label: Make arch_jump_label_text_poke_early() optional' patch.
> >>
> >> Let me know which option you would prefer.
> >
> > On IRC, Thomas Gleixner mentioned a better solution (something I've done
> > with ftrace). Just add a "weak" stub function. No need to get Kconfig
> > involved.
> >
> 
> Do you want to write that patch, or shall I?

I can do it since I need to clean up some of the patch set anyway.

-- Steve

Re: [PATCH 0/9] [GIT PULL] jump label: various updates

[Steven Rostedt]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> Ingo,
> 
> Please pull the latest tip/perf/jump-label tree, which can be found at:

OK, don't pull. With feedback from Peter, I'm going to do some clean ups
and such.

He was still in Japan when the original patch set went out, and this is
the first time he's seen them.

-- Steve

Re: [PATCH 3/9] jump label: Fix module __init section race

[Steven Rostedt]

On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:

> --- 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))

Peter, I know you hated this, but the alternative is either:

				if (iter->key &&
				    kernel_text_address(iter->code))

or break 80 chars. All three seem bad (although I don't mind breaking 80
chars for this). But I'll leave it for now.

-- Steve

>  					arch_jump_label_transform(iter, type);
>  				iter++;
>  			}
> @@ -366,6 +367,39 @@ static void remove_jump_label_module(struct module *mod)
>  	}
>  }

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Steven Rostedt]

On Fri, 2010-10-15 at 22:58 +0200, Peter Zijlstra wrote:
> On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> >  
> >  static inline void jump_label_lock(void) {}
> >  static inline void jump_label_unlock(void) {}
> > +static inline void register_jump_label_key(unsigned long key) {}
> > +static inline void unregister_jump_label_key(unsigned long key) {}
> > 
> shouldn't we call that jump_label_register_key() and co?

I believe a lot of code has register/unregister first in its name.

-- Steve

Re: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex

[Steven Rostedt]

On Fri, 2010-10-15 at 22:55 +0200, Peter Zijlstra wrote:
> On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > +void jump_label_lock(void)
> > +{
> > +       mutex_lock(&jump_label_mutex);
> > +}
> > +
> 
> > +++ b/kernel/kprobes.c
> > @@ -1145,13 +1145,16 @@ int __kprobes register_kprobe(struct kprobe *p)
> >                 return ret;
> >  
> >         preempt_disable();
> > +       jump_label_lock();
> 
> How exactly does that work?

Ouch! Good catch!

I'll do the James Bottomley and blame jet lag ;-)

-- Steve

> 
> >         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(); 

Re: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex

[Steven Rostedt]

On Fri, 2010-10-15 at 22:55 +0200, Peter Zijlstra wrote:
> On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > +void jump_label_lock(void)
> > +{
> > +       mutex_lock(&jump_label_mutex);
> > +}
> > +
> 
> > +++ b/kernel/kprobes.c
> > @@ -1145,13 +1145,16 @@ int __kprobes register_kprobe(struct kprobe *p)
> >                 return ret;
> >  
> >         preempt_disable();
> > +       jump_label_lock();
> 
> How exactly does that work?
> 

This patch should fix it (compiled tested only). If all agree, I'll add
it to the series.

-- Steve

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d45d72e..0dbd328 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1144,17 +1144,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;
@@ -1168,10 +1164,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.
@@ -1179,11 +1174,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);
@@ -1225,6 +1220,11 @@ out:
 		module_put(probed_mod);
 
 	return ret;
+
+fail_with_jump_label:
+	preempt_enable();
+	jump_label_unlock();
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(register_kprobe);
 

Re: [PATCH 4/9] jump label: Fix deadlock b/w jump_label_mutex vs. text_mutex

[Steven Rostedt]

On Fri, 2010-10-15 at 22:25 -0400, Steven Rostedt wrote:
> On Fri, 2010-10-15 at 22:55 +0200, Peter Zijlstra wrote:
> > On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > > +void jump_label_lock(void)
> > > +{
> > > +       mutex_lock(&jump_label_mutex);
> > > +}
> > > +
> > 
> > > +++ b/kernel/kprobes.c
> > > @@ -1145,13 +1145,16 @@ int __kprobes register_kprobe(struct kprobe *p)
> > >                 return ret;
> > >  
> > >         preempt_disable();
> > > +       jump_label_lock();
> > 
> > How exactly does that work?
> > 
> 
> This patch should fix it (compiled tested only). If all agree, I'll add
> it to the series.

Well, I just did more than compile test it. I booted first without this
patch, and added a kprobe and lockdep spewed its guts about this
condition. Then I applied the patch, compiled and rebooted, tried the
probe again, and lockdep stayed quiet :-)

-- Steve


> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d45d72e..0dbd328 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1144,17 +1144,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;
> @@ -1168,10 +1164,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.
> @@ -1179,11 +1174,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);
> @@ -1225,6 +1220,11 @@ out:
>  		module_put(probed_mod);
>  
>  	return ret;
> +
> +fail_with_jump_label:
> +	preempt_enable();
> +	jump_label_unlock();
> +	return -EINVAL;
>  }
>  EXPORT_SYMBOL_GPL(register_kprobe);
>  
> 

Re: [PATCH 3/9] jump label: Fix module __init section race

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> 
> > --- 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))
> 
> Peter, I know you hated this, but the alternative is either:
> 
> 				if (iter->key &&
> 				    kernel_text_address(iter->code))
> 
> or break 80 chars. All three seem bad (although I don't mind breaking 
> 80 chars for this). But I'll leave it for now.

No, the proper alternative is to move the body of the iteration into a 
jump_label_update_entry() inline function ...

Nobody is forcing you to start yet another iteration 4 indentations 
deep. We have functions for a reason.

Thanks,

	Ingo

Re: [PATCH 3/9] jump label: Fix module __init section race

[Steven Rostedt]

On Sat, 2010-10-16 at 08:23 +0200, Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > 
> > > --- 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))
> > 
> > Peter, I know you hated this, but the alternative is either:
> > 
> > 				if (iter->key &&
> > 				    kernel_text_address(iter->code))
> > 
> > or break 80 chars. All three seem bad (although I don't mind breaking 
> > 80 chars for this). But I'll leave it for now.
> 
> No, the proper alternative is to move the body of the iteration into a 
> jump_label_update_entry() inline function ...
> 
> Nobody is forcing you to start yet another iteration 4 indentations 
> deep. We have functions for a reason.

Jason,

Could you send me another version of this patch with the second
iteration wrapped in a function.

Thanks,

-- Steve

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Peter Zijlstra]

On Fri, 2010-10-15 at 23:13 +0200, Peter Zijlstra wrote:
> On Fri, 2010-10-15 at 17:09 -0400, Steven Rostedt wrote:
> > On Fri, 2010-10-15 at 23:03 +0200, Peter Zijlstra wrote:
> > 
> > > Urgh, this sucks.. :-(
> > > 
> > > So now we have to actually track all JUMP_LABEL() sites and call
> > > register muck on them.. even though we already track them through the
> > > special data section.
> > > 
> > > Is there really no way around this?
> > 
> > I'll take a look to see if we can monkey with magic and automate it.
> 
> So the problem is something like:
> 
> core kernel:
> 
>  jump_label_enable()
> 
> module:
> 
>  JUMP_LABEL()
> 
> And then because we don't have a proper __jump_table section, the
> jump_label_enable() won't properly work?
> 
> Why not let jump_label_enable() add a dummy entry with the enabled bit
> and once you load the module merge the real entry into it.

Or actually use the value of the key pointer.. it would mean either
standardizing the size (int/atomic_t would work), or using a version of
the fallback JUMP_LABEL implementation to sort out the type issues.

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Jason Baron]

On Mon, Oct 18, 2010 at 02:05:21PM +0200, Peter Zijlstra wrote:
> On Fri, 2010-10-15 at 23:13 +0200, Peter Zijlstra wrote:
> > On Fri, 2010-10-15 at 17:09 -0400, Steven Rostedt wrote:
> > > On Fri, 2010-10-15 at 23:03 +0200, Peter Zijlstra wrote:
> > > 
> > > > Urgh, this sucks.. :-(
> > > > 
> > > > So now we have to actually track all JUMP_LABEL() sites and call
> > > > register muck on them.. even though we already track them through the
> > > > special data section.
> > > > 
> > > > Is there really no way around this?
> > > 
> > > I'll take a look to see if we can monkey with magic and automate it.
> > 
> > So the problem is something like:
> > 
> > core kernel:
> > 
> >  jump_label_enable()
> > 
> > module:
> > 
> >  JUMP_LABEL()
> > 
> > And then because we don't have a proper __jump_table section, the
> > jump_label_enable() won't properly work?
> > 
> > Why not let jump_label_enable() add a dummy entry with the enabled bit
> > and once you load the module merge the real entry into it.
> 
> Or actually use the value of the key pointer.. it would mean either
> standardizing the size (int/atomic_t would work), or using a version of
> the fallback JUMP_LABEL implementation to sort out the type issues.


So I initially implmented this as 'jump_label_enable()' would add a new
entry for the key, if it didn't already exist. However, I was concerned
about the case where module 'a' defined the key variable, and then
module 'b' did the enable/disable, and then module 'a' was removed and
thus the key value could be re-used, and module's 'b' key would mean
something different.

However, I'm not sure that is possible - since module 'b' would have
symbol dependency on module 'a', and thus module 'a' could not be
unloaded before module 'b'.

Thus, when a module is freed, I think we can scan all the keys and check
if any key is contained within the text section of the module that is
about to be freed. If so, we simply remove that key entry. does this
make sense?

thanks,

-Jason

Re: [PATCH 5/9] jump label: Add register_jump_label_key/unregister_jump_label_key

[Peter Zijlstra]

On Mon, 2010-10-18 at 10:03 -0400, Jason Baron wrote:
> On Mon, Oct 18, 2010 at 02:05:21PM +0200, Peter Zijlstra wrote:
> > On Fri, 2010-10-15 at 23:13 +0200, Peter Zijlstra wrote:
> > > On Fri, 2010-10-15 at 17:09 -0400, Steven Rostedt wrote:
> > > > On Fri, 2010-10-15 at 23:03 +0200, Peter Zijlstra wrote:
> > > > 
> > > > > Urgh, this sucks.. :-(
> > > > > 
> > > > > So now we have to actually track all JUMP_LABEL() sites and call
> > > > > register muck on them.. even though we already track them through the
> > > > > special data section.
> > > > > 
> > > > > Is there really no way around this?
> > > > 
> > > > I'll take a look to see if we can monkey with magic and automate it.
> > > 
> > > So the problem is something like:
> > > 
> > > core kernel:
> > > 
> > >  jump_label_enable()
> > > 
> > > module:
> > > 
> > >  JUMP_LABEL()
> > > 
> > > And then because we don't have a proper __jump_table section, the
> > > jump_label_enable() won't properly work?
> > > 
> > > Why not let jump_label_enable() add a dummy entry with the enabled bit
> > > and once you load the module merge the real entry into it.
> > 
> > Or actually use the value of the key pointer.. it would mean either
> > standardizing the size (int/atomic_t would work), or using a version of
> > the fallback JUMP_LABEL implementation to sort out the type issues.
> 
> 
> So I initially implmented this as 'jump_label_enable()' would add a new
> entry for the key, if it didn't already exist. However, I was concerned
> about the case where module 'a' defined the key variable, and then
> module 'b' did the enable/disable, and then module 'a' was removed and
> thus the key value could be re-used, and module's 'b' key would mean
> something different.
> 
> However, I'm not sure that is possible - since module 'b' would have
> symbol dependency on module 'a', and thus module 'a' could not be
> unloaded before module 'b'.

Right, if the key variable is part of A and B uses it, then A should not
be allowed to be unloaded.

> Thus, when a module is freed, I think we can scan all the keys and check
> if any key is contained within the text section of the module that is
> about to be freed. If so, we simply remove that key entry. does this
> make sense?

Yep.

Re: [PATCH 3/9] jump label: Fix module __init section race

[Jason Baron]

On Sat, Oct 16, 2010 at 12:23:19PM -0400, Steven Rostedt wrote:
> On Sat, 2010-10-16 at 08:23 +0200, Ingo Molnar wrote:
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > On Fri, 2010-10-15 at 16:09 -0400, Steven Rostedt wrote:
> > > 
> > > > --- 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))
> > > 
> > > Peter, I know you hated this, but the alternative is either:
> > > 
> > > 				if (iter->key &&
> > > 				    kernel_text_address(iter->code))
> > > 
> > > or break 80 chars. All three seem bad (although I don't mind breaking 
> > > 80 chars for this). But I'll leave it for now.
> > 
> > No, the proper alternative is to move the body of the iteration into a 
> > jump_label_update_entry() inline function ...
> > 
> > Nobody is forcing you to start yet another iteration 4 indentations 
> > deep. We have functions for a reason.
> 
> Jason,
> 
> Could you send me another version of this patch with the second
> iteration wrapped in a function.
> 
> Thanks,
> 
> -- Steve
> 
> 

sure. I going to re-spin this whole series - there are a bunch of
changes. Hopefully, I can drop the whole register/unregister notion as
Peter suggested.

thanks,

-Jason

[tip:perf/urgent] jump label: Make arch_jump_label_text_poke_early() optional

[tip-bot for Steven Rostedt]

Commit-ID:  95bcd683fb694a3e2d0538bf486430a0dfbb4111
Gitweb:     http://git.kernel.org/tip/95bcd683fb694a3e2d0538bf486430a0dfbb4111
Author:     Steven Rostedt <srostedt@redhat.com>
AuthorDate: Fri, 29 Oct 2010 11:02:43 -0400
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Fri, 29 Oct 2010 12:56:13 -0400

jump label: Make arch_jump_label_text_poke_early() optional

Some archs do not need to do anything special for jump labels on
startup (like MIPS).  This patch adds a weak function stub for
arch_jump_label_text_poke_early();

Cc: Jason Baron <jbaron@redhat.com>
Cc: David Miller <davem@davemloft.net>
Cc: David Daney <ddaney@caviumnetworks.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <1286218615-24011-2-git-send-email-ddaney@caviumnetworks.com>
LKML-Reference: <20101015201037.703989993@goodmis.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/jump_label.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 12cce78..3b79bd9 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -270,6 +270,13 @@ out:
 	return conflict;
 }
 
+/*
+ * Not all archs need this.
+ */
+void __weak arch_jump_label_text_poke_early(jump_label_t addr)
+{
+}
+
 static __init int init_jump_label(void)
 {
 	int ret;