[GIT PULL] jump label: add jump label code

[GIT PULL] jump label: add jump label code

[Steven Rostedt]

Ingo,

I finally got around to doing some basic tests. The thing that
took the longest, was setting up all my distcc computers with gcc 2.5.1.
I also hit a little glitch in one of my tests that would trigger
RCU stalls, but that's another story.

Anyway, I took Jason's patches and applied them, with a few
formatting clean ups (basically, shortened function prototypes).

I added two patches at the end of the series to remove the duplicate
structure and also to remove the dependency on !CC_OPTIMIZE_FOR_SIZE.

Andi,

I saw your patches but have not had time to take a deeper look at
them. If you want, you can rebase them against this tree.

Thanks,

Please pull the latest tip/perf/core tree, which can be found at:

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


David S. Miller (1):
      jump label: Add sparc64 support

Jason Baron (8):
      jump label: Make dynamic no-op selection available outside of ftrace
      jump label: Make text_poke_early() globally visible
      jump label: Base patch for jump label
      jump label: Initialize workqueue tracepoints *before* they are registered
      jump label: Add jump_label_text_reserved() to reserve jump points
      jump label: Tracepoint support for jump labels
      jump label: Convert dynamic debug to use jump labels
      jump label: x86 support

Steven Rostedt (2):
      jump label: Remove duplicate structure for x86
      jump label/x86/sparc64: Remove !CC_OPTIMIZE_FOR_SIZE config conditions

----
 Makefile                            |    5 +
 arch/Kconfig                        |    3 +
 arch/sparc/Kconfig                  |    1 +
 arch/sparc/include/asm/jump_label.h |   32 +++
 arch/sparc/kernel/Makefile          |    2 +
 arch/sparc/kernel/jump_label.c      |   47 ++++
 arch/sparc/kernel/module.c          |    6 +
 arch/x86/Kconfig                    |    1 +
 arch/x86/include/asm/alternative.h  |   11 +
 arch/x86/include/asm/jump_label.h   |   37 +++
 arch/x86/kernel/Makefile            |    2 +-
 arch/x86/kernel/alternative.c       |   68 ++++++-
 arch/x86/kernel/ftrace.c            |   63 +-----
 arch/x86/kernel/jump_label.c        |   50 ++++
 arch/x86/kernel/kprobes.c           |    3 +-
 arch/x86/kernel/module.c            |    3 +
 arch/x86/kernel/setup.c             |    6 +
 include/asm-generic/vmlinux.lds.h   |   10 +
 include/linux/dynamic_debug.h       |   39 ++--
 include/linux/jump_label.h          |   64 ++++++
 include/linux/module.h              |    5 +-
 include/linux/tracepoint.h          |    5 +-
 kernel/Makefile                     |    2 +-
 kernel/jump_label.c                 |  429 +++++++++++++++++++++++++++++++++++
 kernel/kprobes.c                    |    4 +-
 kernel/module.c                     |    6 +
 kernel/trace/trace_workqueue.c      |   10 +-
 kernel/tracepoint.c                 |   14 +-
 lib/dynamic_debug.c                 |   42 +----
 scripts/Makefile.lib                |   11 +-
 scripts/basic/Makefile              |    2 +-
 scripts/basic/hash.c                |   64 ------
 scripts/gcc-goto.sh                 |    5 +
 33 files changed, 843 insertions(+), 209 deletions(-)

[PATCH 01/11] jump label: Make dynamic no-op selection available outside of ftrace

[Steven Rostedt]

[-- Attachment #1: 0001-jump-label-Make-dynamic-no-op-selection-available-ou.patch --]
[-- Type: text/plain, Size: 6638 bytes --]

From: Jason Baron <jbaron@redhat.com>

Move Steve's code for finding the best 5-byte no-op from ftrace.c to
alternative.c. The idea is that other consumers (in this case jump label)
want to make use of that code.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <96259ae74172dcac99c0020c249743c523a92e18.1284733808.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/include/asm/alternative.h |    8 ++++
 arch/x86/kernel/alternative.c      |   64 ++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/ftrace.c           |   63 +----------------------------------
 arch/x86/kernel/setup.c            |    6 +++
 4 files changed, 79 insertions(+), 62 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index bc6abb7..27a35b6 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -180,4 +180,12 @@ static inline void apply_paravirt(struct paravirt_patch_site *start,
 extern void *text_poke(void *addr, const void *opcode, size_t len);
 extern void *text_poke_smp(void *addr, const void *opcode, size_t len);
 
+#if defined(CONFIG_DYNAMIC_FTRACE)
+#define IDEAL_NOP_SIZE_5 5
+extern unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
+extern void arch_init_ideal_nop5(void);
+#else
+static inline void arch_init_ideal_nop5(void) {}
+#endif
+
 #endif /* _ASM_X86_ALTERNATIVE_H */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index f65ab8b..1849d80 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -641,3 +641,67 @@ void *__kprobes text_poke_smp(void *addr, const void *opcode, size_t len)
 	return addr;
 }
 
+#if defined(CONFIG_DYNAMIC_FTRACE)
+
+unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
+
+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.
+	 *
+	 * TODO: check the cpuid to determine the best nop.
+	 */
+	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;
+	}
+
+}
+#endif
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index cd37469..3afb33f 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -257,14 +257,9 @@ do_ftrace_mod_code(unsigned long ip, void *new_code)
 	return mod_code_status;
 }
 
-
-
-
-static unsigned char ftrace_nop[MCOUNT_INSN_SIZE];
-
 static unsigned char *ftrace_nop_replace(void)
 {
-	return ftrace_nop;
+	return ideal_nop5;
 }
 
 static int
@@ -338,62 +333,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 
 int __init ftrace_dyn_arch_init(void *data)
 {
-	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.
-	 *
-	 * TODO: check the cpuid to determine the best nop.
-	 */
-	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(ftrace_nop, ftrace_test_p6nop, MCOUNT_INSN_SIZE);
-		break;
-	case 1:
-		pr_info("converting mcount calls to 66 66 66 66 90\n");
-		memcpy(ftrace_nop, ftrace_test_nop5, MCOUNT_INSN_SIZE);
-		break;
-	case 2:
-		pr_info("converting mcount calls to jmp . + 5\n");
-		memcpy(ftrace_nop, ftrace_test_jmp, MCOUNT_INSN_SIZE);
-		break;
-	}
-
 	/* The return code is retured via data */
 	*(unsigned long *)data = 0;
 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c3a4fbb..00e1678 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -112,6 +112,7 @@
 #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.
@@ -726,6 +727,7 @@ 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));
@@ -1071,6 +1073,10 @@ 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 02/11] jump label: Make text_poke_early() globally visible

[Steven Rostedt]

[-- Attachment #1: 0002-jump-label-Make-text_poke_early-globally-visible.patch --]
[-- Type: text/plain, Size: 2174 bytes --]

From: Jason Baron <jbaron@redhat.com>

Make text_poke_early available outside of alternative.c. The jump label
patchset wants to make use of it in order to set up the optimal no-op
sequences at run-time.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <04cfddf2ba77bcabfc3e524f1849d871d6a1cf9d.1284733808.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/include/asm/alternative.h |    2 ++
 arch/x86/kernel/alternative.c      |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 27a35b6..634bf78 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -160,6 +160,8 @@ static inline void apply_paravirt(struct paravirt_patch_site *start,
 #define __parainstructions_end	NULL
 #endif
 
+extern void *text_poke_early(void *addr, const void *opcode, size_t len);
+
 /*
  * Clear and restore the kernel write-protection flag on the local CPU.
  * Allows the kernel to edit read-only pages.
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 1849d80..083bd01 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -195,7 +195,7 @@ static void __init_or_module add_nops(void *insns, unsigned int len)
 
 extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
 extern s32 __smp_locks[], __smp_locks_end[];
-static void *text_poke_early(void *addr, const void *opcode, size_t len);
+void *text_poke_early(void *addr, const void *opcode, size_t len);
 
 /* Replace instructions with better alternatives for this CPU type.
    This runs before SMP is initialized to avoid SMP problems with
@@ -522,7 +522,7 @@ void __init alternative_instructions(void)
  * instructions. And on the local CPU you need to be protected again NMI or MCE
  * handlers seeing an inconsistent instruction while you patch.
  */
-static void *__init_or_module text_poke_early(void *addr, const void *opcode,
+void *__init_or_module text_poke_early(void *addr, const void *opcode,
 					      size_t len)
 {
 	unsigned long flags;
-- 
1.7.1

[PATCH 03/11] jump label: Base patch for jump label

[Steven Rostedt]

[-- Attachment #1: 0003-jump-label-Base-patch-for-jump-label.patch --]
[-- Type: text/plain, Size: 17237 bytes --]

From: Jason Baron <jbaron@redhat.com>

base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
assembly gcc mechanism, we can now branch to labels from an 'asm goto'
statment. This allows us to create a 'no-op' fastpath, which can subsequently
be patched with a jump to the slowpath code. This is useful for code which
might be rarely used, but which we'd like to be able to call, if needed.
Tracepoints are the current usecase that these are being implemented for.

Acked-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <ee8b3595967989fdaf84e698dc7447d315ce972a.1284733808.git.jbaron@redhat.com>

[ cleaned up some formating ]

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Makefile                           |    5 +
 arch/Kconfig                       |    3 +
 arch/x86/include/asm/alternative.h |    3 +-
 arch/x86/kernel/alternative.c      |    2 +-
 include/asm-generic/vmlinux.lds.h  |   10 +
 include/linux/jump_label.h         |   58 ++++++
 include/linux/module.h             |    5 +-
 kernel/Makefile                    |    2 +-
 kernel/jump_label.c                |  346 ++++++++++++++++++++++++++++++++++++
 kernel/kprobes.c                   |    1 +
 kernel/module.c                    |    6 +
 scripts/gcc-goto.sh                |    5 +
 12 files changed, 442 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/jump_label.h
 create mode 100644 kernel/jump_label.c
 create mode 100644 scripts/gcc-goto.sh

diff --git a/Makefile b/Makefile
index 92ab33f..a906378 100644
--- a/Makefile
+++ b/Makefile
@@ -591,6 +591,11 @@ KBUILD_CFLAGS	+= $(call cc-option,-fno-strict-overflow)
 # conserve stack if available
 KBUILD_CFLAGS   += $(call cc-option,-fconserve-stack)
 
+# check for 'asm goto'
+ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC)), y)
+	KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
+endif
+
 # Add user supplied CPPFLAGS, AFLAGS and CFLAGS as the last assignments
 # But warn user when we do so
 warn-assign = \
diff --git a/arch/Kconfig b/arch/Kconfig
index 4877a8c..1462d84 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -158,4 +158,7 @@ config HAVE_PERF_EVENTS_NMI
 	  subsystem.  Also has support for calculating CPU cycle events
 	  to determine how many clock cycles in a given period.
 
+config HAVE_ARCH_JUMP_LABEL
+	bool
+
 source "kernel/gcov/Kconfig"
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 634bf78..76561d2 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 #include <linux/stddef.h>
 #include <linux/stringify.h>
+#include <linux/jump_label.h>
 #include <asm/asm.h>
 
 /*
@@ -182,7 +183,7 @@ extern void *text_poke_early(void *addr, const void *opcode, size_t len);
 extern void *text_poke(void *addr, const void *opcode, size_t len);
 extern void *text_poke_smp(void *addr, const void *opcode, size_t len);
 
-#if defined(CONFIG_DYNAMIC_FTRACE)
+#if defined(CONFIG_DYNAMIC_FTRACE) || defined(HAVE_JUMP_LABEL)
 #define IDEAL_NOP_SIZE_5 5
 extern unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
 extern void arch_init_ideal_nop5(void);
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 083bd01..cb0e6d3 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -641,7 +641,7 @@ void *__kprobes text_poke_smp(void *addr, const void *opcode, size_t len)
 	return addr;
 }
 
-#if defined(CONFIG_DYNAMIC_FTRACE)
+#if defined(CONFIG_DYNAMIC_FTRACE) || defined(HAVE_JUMP_LABEL)
 
 unsigned char ideal_nop5[IDEAL_NOP_SIZE_5];
 
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8a92a17..ef2af99 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -220,6 +220,8 @@
 									\
 	BUG_TABLE							\
 									\
+	JUMP_TABLE							\
+									\
 	/* PCI quirks */						\
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
 		VMLINUX_SYMBOL(__start_pci_fixups_early) = .;		\
@@ -563,6 +565,14 @@
 #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);							\
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
new file mode 100644
index 0000000..de58656
--- /dev/null
+++ b/include/linux/jump_label.h
@@ -0,0 +1,58 @@
+#ifndef _LINUX_JUMP_LABEL_H
+#define _LINUX_JUMP_LABEL_H
+
+#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_HAVE_ARCH_JUMP_LABEL)
+# include <asm/jump_label.h>
+# define HAVE_JUMP_LABEL
+#endif
+
+enum jump_label_type {
+	JUMP_LABEL_ENABLE,
+	JUMP_LABEL_DISABLE
+};
+
+struct module;
+
+#ifdef HAVE_JUMP_LABEL
+
+extern struct jump_entry __start___jump_table[];
+extern struct jump_entry __stop___jump_table[];
+
+extern void arch_jump_label_transform(struct jump_entry *entry,
+				 enum jump_label_type type);
+extern void jump_label_update(unsigned long key, enum jump_label_type type);
+extern void jump_label_apply_nops(struct module *mod);
+extern void arch_jump_label_text_poke_early(jump_label_t addr);
+
+#define enable_jump_label(key) \
+	jump_label_update((unsigned long)key, JUMP_LABEL_ENABLE);
+
+#define disable_jump_label(key) \
+	jump_label_update((unsigned long)key, JUMP_LABEL_DISABLE);
+
+#else
+
+#define JUMP_LABEL(key, label)			\
+do {						\
+	if (unlikely(*key))			\
+		goto label;			\
+} while (0)
+
+#define enable_jump_label(cond_var)	\
+do {					\
+       *(cond_var) = 1;			\
+} while (0)
+
+#define disable_jump_label(cond_var)	\
+do {					\
+       *(cond_var) = 0;			\
+} while (0)
+
+static inline int jump_label_apply_nops(struct module *mod)
+{
+	return 0;
+}
+
+#endif
+
+#endif
diff --git a/include/linux/module.h b/include/linux/module.h
index 8a6b9fd..403ac26 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -350,7 +350,10 @@ struct module
 	struct tracepoint *tracepoints;
 	unsigned int num_tracepoints;
 #endif
-
+#ifdef HAVE_JUMP_LABEL
+	struct jump_entry *jump_entries;
+	unsigned int num_jump_entries;
+#endif
 #ifdef CONFIG_TRACING
 	const char **trace_bprintk_fmt_start;
 	unsigned int num_trace_bprintk_fmt;
diff --git a/kernel/Makefile b/kernel/Makefile
index 0b72d1a..d52b473 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -10,7 +10,7 @@ obj-y     = sched.o fork.o exec_domain.o panic.o printk.o \
 	    kthread.o wait.o kfifo.o sys_ni.o posix-cpu-timers.o mutex.o \
 	    hrtimer.o rwsem.o nsproxy.o srcu.o semaphore.o \
 	    notifier.o ksysfs.o pm_qos_params.o sched_clock.o cred.o \
-	    async.o range.o
+	    async.o range.o jump_label.o
 obj-$(CONFIG_HAVE_EARLY_RES) += early_res.o
 obj-y += groups.o
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
new file mode 100644
index 0000000..460fd40
--- /dev/null
+++ b/kernel/jump_label.c
@@ -0,0 +1,346 @@
+/*
+ * jump label support
+ *
+ * Copyright (C) 2009 Jason Baron <jbaron@redhat.com>
+ *
+ */
+#include <linux/jump_label.h>
+#include <linux/memory.h>
+#include <linux/uaccess.h>
+#include <linux/module.h>
+#include <linux/list.h>
+#include <linux/jhash.h>
+#include <linux/slab.h>
+#include <linux/sort.h>
+#include <linux/err.h>
+
+#ifdef HAVE_JUMP_LABEL
+
+#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];
+
+/* mutex to protect coming/going of the the jump_label table */
+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;
+};
+
+struct jump_label_module_entry {
+	struct hlist_node hlist;
+	struct jump_entry *table;
+	int nr_entries;
+	struct module *mod;
+};
+
+static int jump_label_cmp(const void *a, const void *b)
+{
+	const struct jump_entry *jea = a;
+	const struct jump_entry *jeb = b;
+
+	if (jea->key < jeb->key)
+		return -1;
+
+	if (jea->key > jeb->key)
+		return 1;
+
+	return 0;
+}
+
+static void
+sort_jump_label_entries(struct jump_entry *start, struct jump_entry *stop)
+{
+	unsigned long size;
+
+	size = (((unsigned long)stop - (unsigned long)start)
+					/ sizeof(struct jump_entry));
+	sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
+}
+
+static struct jump_label_entry *get_jump_label_entry(jump_label_t key)
+{
+	struct hlist_head *head;
+	struct hlist_node *node;
+	struct jump_label_entry *e;
+	u32 hash = jhash((void *)&key, sizeof(jump_label_t), 0);
+
+	head = &jump_label_table[hash & (JUMP_LABEL_TABLE_SIZE - 1)];
+	hlist_for_each_entry(e, node, head, hlist) {
+		if (key == e->key)
+			return e;
+	}
+	return NULL;
+}
+
+static struct jump_label_entry *
+add_jump_label_entry(jump_label_t key, int nr_entries, struct jump_entry *table)
+{
+	struct hlist_head *head;
+	struct jump_label_entry *e;
+	u32 hash;
+
+	e = get_jump_label_entry(key);
+	if (e)
+		return ERR_PTR(-EEXIST);
+
+	e = kmalloc(sizeof(struct jump_label_entry), GFP_KERNEL);
+	if (!e)
+		return ERR_PTR(-ENOMEM);
+
+	hash = jhash((void *)&key, sizeof(jump_label_t), 0);
+	head = &jump_label_table[hash & (JUMP_LABEL_TABLE_SIZE - 1)];
+	e->key = key;
+	e->table = table;
+	e->nr_entries = nr_entries;
+	INIT_HLIST_HEAD(&(e->modules));
+	hlist_add_head(&e->hlist, head);
+	return e;
+}
+
+static int
+build_jump_label_hashtable(struct jump_entry *start, struct jump_entry *stop)
+{
+	struct jump_entry *iter, *iter_begin;
+	struct jump_label_entry *entry;
+	int count;
+
+	sort_jump_label_entries(start, stop);
+	iter = start;
+	while (iter < stop) {
+		entry = get_jump_label_entry(iter->key);
+		if (!entry) {
+			iter_begin = iter;
+			count = 0;
+			while ((iter < stop) &&
+				(iter->key == iter_begin->key)) {
+				iter++;
+				count++;
+			}
+			entry = add_jump_label_entry(iter_begin->key,
+							count, iter_begin);
+			if (IS_ERR(entry))
+				return PTR_ERR(entry);
+		 } else {
+			WARN_ONCE(1, KERN_ERR "build_jump_hashtable: unexpected entry!\n");
+			return -1;
+		}
+	}
+	return 0;
+}
+
+/***
+ * jump_label_update - update jump label text
+ * @key -  key value associated with a a jump label
+ * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
+ *
+ * Will enable/disable the jump for jump label @key, depending on the
+ * value of @type.
+ *
+ */
+
+void jump_label_update(unsigned long key, enum jump_label_type type)
+{
+	struct jump_entry *iter;
+	struct jump_label_entry *entry;
+	struct hlist_node *module_node;
+	struct jump_label_module_entry *e_module;
+	int count;
+
+	mutex_lock(&jump_label_mutex);
+	entry = get_jump_label_entry((jump_label_t)key);
+	if (entry) {
+		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))
+					arch_jump_label_transform(iter, type);
+				iter++;
+			}
+		}
+	}
+	mutex_unlock(&jump_label_mutex);
+}
+
+static __init int init_jump_label(void)
+{
+	int ret;
+	struct jump_entry *iter_start = __start___jump_table;
+	struct jump_entry *iter_stop = __stop___jump_table;
+	struct jump_entry *iter;
+
+	mutex_lock(&jump_label_mutex);
+	ret = build_jump_label_hashtable(__start___jump_table,
+					 __stop___jump_table);
+	iter = iter_start;
+	while (iter < iter_stop) {
+		arch_jump_label_text_poke_early(iter->code);
+		iter++;
+	}
+	mutex_unlock(&jump_label_mutex);
+	return ret;
+}
+early_initcall(init_jump_label);
+
+#ifdef CONFIG_MODULES
+
+static struct jump_label_module_entry *
+add_jump_label_module_entry(struct jump_label_entry *entry,
+			    struct jump_entry *iter_begin,
+			    int count, struct module *mod)
+{
+	struct jump_label_module_entry *e;
+
+	e = kmalloc(sizeof(struct jump_label_module_entry), GFP_KERNEL);
+	if (!e)
+		return ERR_PTR(-ENOMEM);
+	e->mod = mod;
+	e->nr_entries = count;
+	e->table = iter_begin;
+	hlist_add_head(&e->hlist, &entry->modules);
+	return e;
+}
+
+static int add_jump_label_module(struct module *mod)
+{
+	struct jump_entry *iter, *iter_begin;
+	struct jump_label_entry *entry;
+	struct jump_label_module_entry *module_entry;
+	int count;
+
+	/* if the module doesn't have jump label entries, just return */
+	if (!mod->num_jump_entries)
+		return 0;
+
+	sort_jump_label_entries(mod->jump_entries,
+				mod->jump_entries + mod->num_jump_entries);
+	iter = mod->jump_entries;
+	while (iter < mod->jump_entries + mod->num_jump_entries) {
+		entry = get_jump_label_entry(iter->key);
+		iter_begin = iter;
+		count = 0;
+		while ((iter < mod->jump_entries + mod->num_jump_entries) &&
+			(iter->key == iter_begin->key)) {
+				iter++;
+				count++;
+		}
+		if (!entry) {
+			entry = add_jump_label_entry(iter_begin->key, 0, NULL);
+			if (IS_ERR(entry))
+				return PTR_ERR(entry);
+		}
+		module_entry = add_jump_label_module_entry(entry, iter_begin,
+							   count, mod);
+		if (IS_ERR(module_entry))
+			return PTR_ERR(module_entry);
+	}
+	return 0;
+}
+
+static void remove_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;
+	int i;
+
+	/* 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) {
+					hlist_del(&e_module->hlist);
+					kfree(e_module);
+				}
+			}
+			if (hlist_empty(&e->modules) && (e->nr_entries == 0)) {
+				hlist_del(&e->hlist);
+				kfree(e);
+			}
+		}
+	}
+}
+
+static int
+jump_label_module_notify(struct notifier_block *self, unsigned long val,
+			 void *data)
+{
+	struct module *mod = data;
+	int ret = 0;
+
+	switch (val) {
+	case MODULE_STATE_COMING:
+		mutex_lock(&jump_label_mutex);
+		ret = add_jump_label_module(mod);
+		if (ret)
+			remove_jump_label_module(mod);
+		mutex_unlock(&jump_label_mutex);
+		break;
+	case MODULE_STATE_GOING:
+		mutex_lock(&jump_label_mutex);
+		remove_jump_label_module(mod);
+		mutex_unlock(&jump_label_mutex);
+		break;
+	}
+	return ret;
+}
+
+/***
+ * apply_jump_label_nops - patch module jump labels with arch_get_jump_label_nop()
+ * @mod: module to patch
+ *
+ * Allow for run-time selection of the optimal nops. Before the module
+ * loads patch these with arch_get_jump_label_nop(), which is specified by
+ * the arch specific jump label code.
+ */
+void jump_label_apply_nops(struct module *mod)
+{
+	struct jump_entry *iter;
+
+	/* if the module doesn't have jump label entries, just return */
+	if (!mod->num_jump_entries)
+		return;
+
+	iter = mod->jump_entries;
+	while (iter < mod->jump_entries + mod->num_jump_entries) {
+		arch_jump_label_text_poke_early(iter->code);
+		iter++;
+	}
+}
+
+struct notifier_block jump_label_module_nb = {
+	.notifier_call = jump_label_module_notify,
+	.priority = 0,
+};
+
+static __init int init_jump_label_module(void)
+{
+	return register_module_notifier(&jump_label_module_nb);
+}
+early_initcall(init_jump_label_module);
+
+#endif /* CONFIG_MODULES */
+
+#endif
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 6dd5359..18904e4 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -47,6 +47,7 @@
 #include <linux/memory.h>
 #include <linux/ftrace.h>
 #include <linux/cpu.h>
+#include <linux/jump_label.h>
 
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
diff --git a/kernel/module.c b/kernel/module.c
index d0b5f8d..eba1341 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -55,6 +55,7 @@
 #include <linux/async.h>
 #include <linux/percpu.h>
 #include <linux/kmemleak.h>
+#include <linux/jump_label.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
@@ -2308,6 +2309,11 @@ static void find_module_sections(struct module *mod, struct load_info *info)
 					sizeof(*mod->tracepoints),
 					&mod->num_tracepoints);
 #endif
+#ifdef HAVE_JUMP_LABEL
+	mod->jump_entries = section_objs(info, "__jump_table",
+					sizeof(*mod->jump_entries),
+					&mod->num_jump_entries);
+#endif
 #ifdef CONFIG_EVENT_TRACING
 	mod->trace_events = section_objs(info, "_ftrace_events",
 					 sizeof(*mod->trace_events),
diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
new file mode 100644
index 0000000..8e82424
--- /dev/null
+++ b/scripts/gcc-goto.sh
@@ -0,0 +1,5 @@
+#!/bin/sh
+# Test for gcc 'asm goto' suport
+# Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
+
+echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $1 -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
-- 
1.7.1

[PATCH 04/11] jump label: Initialize workqueue tracepoints *before* they are registered

[Steven Rostedt]

[-- Attachment #1: 0004-jump-label-Initialize-workqueue-tracepoints-before-t.patch --]
[-- Type: text/plain, Size: 1229 bytes --]

From: Jason Baron <jbaron@redhat.com>

Initialize the workqueue data structures *before* they are registered
so that they are ready for callbacks.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <e3a3383fc370ac7086625bebe89d9480d7caf372.1284733808.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_workqueue.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_workqueue.c b/kernel/trace/trace_workqueue.c
index a7cc379..209b379 100644
--- a/kernel/trace/trace_workqueue.c
+++ b/kernel/trace/trace_workqueue.c
@@ -263,6 +263,11 @@ int __init trace_workqueue_early_init(void)
 {
 	int ret, cpu;
 
+	for_each_possible_cpu(cpu) {
+		spin_lock_init(&workqueue_cpu_stat(cpu)->lock);
+		INIT_LIST_HEAD(&workqueue_cpu_stat(cpu)->list);
+	}
+
 	ret = register_trace_workqueue_insertion(probe_workqueue_insertion, NULL);
 	if (ret)
 		goto out;
@@ -279,11 +284,6 @@ int __init trace_workqueue_early_init(void)
 	if (ret)
 		goto no_creation;
 
-	for_each_possible_cpu(cpu) {
-		spin_lock_init(&workqueue_cpu_stat(cpu)->lock);
-		INIT_LIST_HEAD(&workqueue_cpu_stat(cpu)->list);
-	}
-
 	return 0;
 
 no_creation:
-- 
1.7.1

[PATCH 05/11] jump label: Add jump_label_text_reserved() to reserve jump points

[Steven Rostedt]

[-- Attachment #1: 0005-jump-label-Add-jump_label_text_reserved-to-reserve-j.patch --]
[-- Type: text/plain, Size: 5112 bytes --]

From: Jason Baron <jbaron@redhat.com>

Add a jump_label_text_reserved(void *start, void *end), so that other
pieces of code that want to modify kernel text, can first verify that
jump label has not reserved the instruction.

Acked-by: Masami Hiramatsu <mhiramat@redhat.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <06236663a3a7b1c1f13576bb9eccb6d9c17b7bfe.1284733808.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/kprobes.c  |    3 +-
 include/linux/jump_label.h |    8 ++++-
 kernel/jump_label.c        |   83 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/kprobes.c           |    3 +-
 4 files changed, 94 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index e05952a..1cbd54c 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -1218,7 +1218,8 @@ static int __kprobes copy_optimized_instructions(u8 *dest, u8 *src)
 	}
 	/* Check whether the address range is reserved */
 	if (ftrace_text_reserved(src, src + len - 1) ||
-	    alternatives_text_reserved(src, src + len - 1))
+	    alternatives_text_reserved(src, src + len - 1) ||
+	    jump_label_text_reserved(src, src + len - 1))
 		return -EBUSY;
 
 	return len;
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index de58656..b72cd9f 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -20,9 +20,10 @@ extern struct jump_entry __stop___jump_table[];
 
 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);
 extern void jump_label_update(unsigned long key, enum jump_label_type type);
 extern void jump_label_apply_nops(struct module *mod);
-extern void arch_jump_label_text_poke_early(jump_label_t addr);
+extern int jump_label_text_reserved(void *start, void *end);
 
 #define enable_jump_label(key) \
 	jump_label_update((unsigned long)key, JUMP_LABEL_ENABLE);
@@ -53,6 +54,11 @@ static inline int jump_label_apply_nops(struct module *mod)
 	return 0;
 }
 
+static inline int jump_label_text_reserved(void *start, void *end)
+{
+	return 0;
+}
+
 #endif
 
 #endif
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 460fd40..7be868b 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -177,6 +177,89 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 	mutex_unlock(&jump_label_mutex);
 }
 
+static int addr_conflict(struct jump_entry *entry, void *start, void *end)
+{
+	if (entry->code <= (unsigned long)end &&
+		entry->code + JUMP_LABEL_NOP_SIZE > (unsigned long)start)
+		return 1;
+
+	return 0;
+}
+
+#ifdef CONFIG_MODULES
+
+static int module_conflict(void *start, void *end)
+{
+	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;
+	int conflict = 0;
+
+	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) {
+				count = e_module->nr_entries;
+				iter = e_module->table;
+				while (count--) {
+					if (addr_conflict(iter, start, end)) {
+						conflict = 1;
+						goto out;
+					}
+					iter++;
+				}
+			}
+		}
+	}
+out:
+	return conflict;
+}
+
+#endif
+
+/***
+ * jump_label_text_reserved - check if addr range is reserved
+ * @start: start text addr
+ * @end: end text addr
+ *
+ * checks if the text addr located between @start and @end
+ * 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.
+ *
+ * returns 1 if there is an overlap, 0 otherwise
+ */
+int jump_label_text_reserved(void *start, void *end)
+{
+	struct jump_entry *iter;
+	struct jump_entry *iter_start = __start___jump_table;
+	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)) {
+			conflict = 1;
+			goto out;
+		}
+		iter++;
+	}
+
+	/* now check modules */
+#ifdef CONFIG_MODULES
+	conflict = module_conflict(start, end);
+#endif
+out:
+	mutex_unlock(&jump_label_mutex);
+	return conflict;
+}
+
 static __init int init_jump_label(void)
 {
 	int ret;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 18904e4..ec4210c 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1147,7 +1147,8 @@ int __kprobes register_kprobe(struct kprobe *p)
 	preempt_disable();
 	if (!kernel_text_address((unsigned long) p->addr) ||
 	    in_kprobes_functions((unsigned long) p->addr) ||
-	    ftrace_text_reserved(p->addr, p->addr)) {
+	    ftrace_text_reserved(p->addr, p->addr) ||
+	    jump_label_text_reserved(p->addr, p->addr)) {
 		preempt_enable();
 		return -EINVAL;
 	}
-- 
1.7.1

[PATCH 06/11] jump label: Tracepoint support for jump labels

[Steven Rostedt]

[-- Attachment #1: 0006-jump-label-Tracepoint-support-for-jump-labels.patch --]
[-- Type: text/plain, Size: 2264 bytes --]

From: Jason Baron <jbaron@redhat.com>

Make use of the jump label infrastructure for tracepoints.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <a9ba2056e2c9cf332c3c300b577463ce66ff23a8.1284733808.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/tracepoint.h |    5 ++++-
 kernel/tracepoint.c        |   14 ++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 103d1b6..a4a90b6 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/types.h>
 #include <linux/rcupdate.h>
+#include <linux/jump_label.h>
 
 struct module;
 struct tracepoint;
@@ -145,7 +146,9 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,
 	extern struct tracepoint __tracepoint_##name;			\
 	static inline void trace_##name(proto)				\
 	{								\
-		if (unlikely(__tracepoint_##name.state))		\
+		JUMP_LABEL(&__tracepoint_##name.state, do_trace);	\
+		return;							\
+do_trace:								\
 			__DO_TRACE(&__tracepoint_##name,		\
 				TP_PROTO(data_proto),			\
 				TP_ARGS(data_args));			\
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index c77f3ec..d6073a5 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -25,6 +25,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/jump_label.h>
 
 extern struct tracepoint __start___tracepoints[];
 extern struct tracepoint __stop___tracepoints[];
@@ -263,7 +264,13 @@ static void set_tracepoint(struct tracepoint_entry **entry,
 	 * is used.
 	 */
 	rcu_assign_pointer(elem->funcs, (*entry)->funcs);
-	elem->state = active;
+	if (!elem->state && active) {
+		enable_jump_label(&elem->state);
+		elem->state = active;
+	} else if (elem->state && !active) {
+		disable_jump_label(&elem->state);
+		elem->state = active;
+	}
 }
 
 /*
@@ -277,7 +284,10 @@ static void disable_tracepoint(struct tracepoint *elem)
 	if (elem->unregfunc && elem->state)
 		elem->unregfunc();
 
-	elem->state = 0;
+	if (elem->state) {
+		disable_jump_label(&elem->state);
+		elem->state = 0;
+	}
 	rcu_assign_pointer(elem->funcs, NULL);
 }
 
-- 
1.7.1

[PATCH 07/11] jump label: Convert dynamic debug to use jump labels

[Steven Rostedt]

[-- Attachment #1: 0007-jump-label-Convert-dynamic-debug-to-use-jump-labels.patch --]
[-- Type: text/plain, Size: 8756 bytes --]

From: Jason Baron <jbaron@redhat.com>

Convert the 'dynamic debug' infrastructure to use jump labels.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <b77627358cea3e27d7be4386f45f66219afb8452.1284733808.git.jbaron@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/dynamic_debug.h |   39 +++++++++++++-----------
 lib/dynamic_debug.c           |   42 ++-------------------------
 scripts/Makefile.lib          |   11 +------
 scripts/basic/Makefile        |    2 +-
 scripts/basic/hash.c          |   64 -----------------------------------------
 5 files changed, 26 insertions(+), 132 deletions(-)
 delete mode 100644 scripts/basic/hash.c

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 52c0da4..bef3cda 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -1,6 +1,8 @@
 #ifndef _DYNAMIC_DEBUG_H
 #define _DYNAMIC_DEBUG_H
 
+#include <linux/jump_label.h>
+
 /* dynamic_printk_enabled, and dynamic_printk_enabled2 are bitmasks in which
  * bit n is set to 1 if any modname hashes into the bucket n, 0 otherwise. They
  * use independent hash functions, to reduce the chance of false positives.
@@ -22,8 +24,6 @@ struct _ddebug {
 	const char *function;
 	const char *filename;
 	const char *format;
-	char primary_hash;
-	char secondary_hash;
 	unsigned int lineno:24;
 	/*
  	 * The flags field controls the behaviour at the callsite.
@@ -33,6 +33,7 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_PRINT   (1<<0)  /* printk() a message using the format */
 #define _DPRINTK_FLAGS_DEFAULT 0
 	unsigned int flags:8;
+	char enabled;
 } __attribute__((aligned(8)));
 
 
@@ -42,33 +43,35 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 #if defined(CONFIG_DYNAMIC_DEBUG)
 extern int ddebug_remove_module(const char *mod_name);
 
-#define __dynamic_dbg_enabled(dd)  ({	     \
-	int __ret = 0;							     \
-	if (unlikely((dynamic_debug_enabled & (1LL << DEBUG_HASH)) &&	     \
-			(dynamic_debug_enabled2 & (1LL << DEBUG_HASH2))))   \
-				if (unlikely(dd.flags))			     \
-					__ret = 1;			     \
-	__ret; })
-
 #define dynamic_pr_debug(fmt, ...) do {					\
+	__label__ do_printk;						\
+	__label__ out;							\
 	static struct _ddebug descriptor				\
 	__used								\
 	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
-		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
-	if (__dynamic_dbg_enabled(descriptor))				\
-		printk(KERN_DEBUG pr_fmt(fmt),	##__VA_ARGS__);		\
+	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
+		_DPRINTK_FLAGS_DEFAULT };				\
+	JUMP_LABEL(&descriptor.enabled, do_printk);			\
+	goto out;							\
+do_printk:								\
+	printk(KERN_DEBUG pr_fmt(fmt),	##__VA_ARGS__);			\
+out:	;								\
 	} while (0)
 
 
 #define dynamic_dev_dbg(dev, fmt, ...) do {				\
+	__label__ do_printk;						\
+	__label__ out;							\
 	static struct _ddebug descriptor				\
 	__used								\
 	__attribute__((section("__verbose"), aligned(8))) =		\
-	{ KBUILD_MODNAME, __func__, __FILE__, fmt, DEBUG_HASH,	\
-		DEBUG_HASH2, __LINE__, _DPRINTK_FLAGS_DEFAULT };	\
-	if (__dynamic_dbg_enabled(descriptor))				\
-		dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
+	{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__,		\
+		_DPRINTK_FLAGS_DEFAULT };				\
+	JUMP_LABEL(&descriptor.enabled, do_printk);			\
+	goto out;							\
+do_printk:								\
+	dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);		\
+out:	;								\
 	} while (0)
 
 #else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 02afc25..e925c7b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -26,19 +26,11 @@
 #include <linux/dynamic_debug.h>
 #include <linux/debugfs.h>
 #include <linux/slab.h>
+#include <linux/jump_label.h>
 
 extern struct _ddebug __start___verbose[];
 extern struct _ddebug __stop___verbose[];
 
-/* dynamic_debug_enabled, and dynamic_debug_enabled2 are bitmasks in which
- * bit n is set to 1 if any modname hashes into the bucket n, 0 otherwise. They
- * use independent hash functions, to reduce the chance of false positives.
- */
-long long dynamic_debug_enabled;
-EXPORT_SYMBOL_GPL(dynamic_debug_enabled);
-long long dynamic_debug_enabled2;
-EXPORT_SYMBOL_GPL(dynamic_debug_enabled2);
-
 struct ddebug_table {
 	struct list_head link;
 	char *mod_name;
@@ -88,26 +80,6 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
 }
 
 /*
- * must be called with ddebug_lock held
- */
-
-static int disabled_hash(char hash, bool first_table)
-{
-	struct ddebug_table *dt;
-	char table_hash_value;
-
-	list_for_each_entry(dt, &ddebug_tables, link) {
-		if (first_table)
-			table_hash_value = dt->ddebugs->primary_hash;
-		else
-			table_hash_value = dt->ddebugs->secondary_hash;
-		if (dt->num_enabled && (hash == table_hash_value))
-			return 0;
-	}
-	return 1;
-}
-
-/*
  * Search the tables for _ddebug's which match the given
  * `query' and apply the `flags' and `mask' to them.  Tells
  * the user which ddebug's were changed, or whether none
@@ -170,17 +142,9 @@ static void ddebug_change(const struct ddebug_query *query,
 				dt->num_enabled++;
 			dp->flags = newflags;
 			if (newflags) {
-				dynamic_debug_enabled |=
-						(1LL << dp->primary_hash);
-				dynamic_debug_enabled2 |=
-						(1LL << dp->secondary_hash);
+				enable_jump_label(&dp->enabled);
 			} else {
-				if (disabled_hash(dp->primary_hash, true))
-					dynamic_debug_enabled &=
-						~(1LL << dp->primary_hash);
-				if (disabled_hash(dp->secondary_hash, false))
-					dynamic_debug_enabled2 &=
-						~(1LL << dp->secondary_hash);
+				disable_jump_label(&dp->enabled);
 			}
 			if (verbose)
 				printk(KERN_INFO
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 54fd1b7..7bfcf1a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -101,14 +101,6 @@ basename_flags = -D"KBUILD_BASENAME=KBUILD_STR($(call name-fix,$(basetarget)))"
 modname_flags  = $(if $(filter 1,$(words $(modname))),\
                  -D"KBUILD_MODNAME=KBUILD_STR($(call name-fix,$(modname)))")
 
-#hash values
-ifdef CONFIG_DYNAMIC_DEBUG
-debug_flags = -D"DEBUG_HASH=$(shell ./scripts/basic/hash djb2 $(@D)$(modname))"\
-              -D"DEBUG_HASH2=$(shell ./scripts/basic/hash r5 $(@D)$(modname))"
-else
-debug_flags =
-endif
-
 orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(KBUILD_SUBDIR_CCFLAGS) \
                  $(ccflags-y) $(CFLAGS_$(basetarget).o)
 _c_flags       = $(filter-out $(CFLAGS_REMOVE_$(basetarget).o), $(orig_c_flags))
@@ -152,8 +144,7 @@ endif
 
 c_flags        = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
 		 $(__c_flags) $(modkern_cflags)                           \
-		 -D"KBUILD_STR(s)=\#s" $(basename_flags) $(modname_flags) \
-		  $(debug_flags)
+		 -D"KBUILD_STR(s)=\#s" $(basename_flags) $(modname_flags)
 
 a_flags        = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
 		 $(__a_flags) $(modkern_aflags)
diff --git a/scripts/basic/Makefile b/scripts/basic/Makefile
index 0955995..4c324a1 100644
--- a/scripts/basic/Makefile
+++ b/scripts/basic/Makefile
@@ -9,7 +9,7 @@
 # fixdep: 	 Used to generate dependency information during build process
 # docproc:	 Used in Documentation/DocBook
 
-hostprogs-y	:= fixdep docproc hash
+hostprogs-y	:= fixdep docproc
 always		:= $(hostprogs-y)
 
 # fixdep is needed to compile other host programs
diff --git a/scripts/basic/hash.c b/scripts/basic/hash.c
deleted file mode 100644
index 2ef5d3f..0000000
--- a/scripts/basic/hash.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright (C) 2008 Red Hat, Inc., Jason Baron <jbaron@redhat.com>
- *
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#define DYNAMIC_DEBUG_HASH_BITS 6
-
-static const char *program;
-
-static void usage(void)
-{
-	printf("Usage: %s <djb2|r5> <modname>\n", program);
-	exit(1);
-}
-
-/* djb2 hashing algorithm by Dan Bernstein. From:
- * http://www.cse.yorku.ca/~oz/hash.html
- */
-
-static unsigned int djb2_hash(char *str)
-{
-	unsigned long hash = 5381;
-	int c;
-
-	c = *str;
-	while (c) {
-		hash = ((hash << 5) + hash) + c;
-		c = *++str;
-	}
-	return (unsigned int)(hash & ((1 << DYNAMIC_DEBUG_HASH_BITS) - 1));
-}
-
-static unsigned int r5_hash(char *str)
-{
-	unsigned long hash = 0;
-	int c;
-
-	c = *str;
-	while (c) {
-		hash = (hash + (c << 4) + (c >> 4)) * 11;
-		c = *++str;
-	}
-	return (unsigned int)(hash & ((1 << DYNAMIC_DEBUG_HASH_BITS) - 1));
-}
-
-int main(int argc, char *argv[])
-{
-	program = argv[0];
-
-	if (argc != 3)
-		usage();
-	if (!strcmp(argv[1], "djb2"))
-		printf("%d\n", djb2_hash(argv[2]));
-	else if (!strcmp(argv[1], "r5"))
-		printf("%d\n", r5_hash(argv[2]));
-	else
-		usage();
-	exit(0);
-}
-
-- 
1.7.1

[PATCH 08/11] jump label: x86 support

[Steven Rostedt]

[-- Attachment #1: 0008-jump-label-x86-support.patch --]
[-- Type: text/plain, Size: 4637 bytes --]

From: Jason Baron <jbaron@redhat.com>

add x86 support for jump label. I'm keeping this patch separate so its clear
to arch maintainers what was required for x86 support this new feature.
Hopefully, it wouldn't be too painful for other archs.

Signed-off-by: Jason Baron <jbaron@redhat.com>
LKML-Reference: <f838f49f40fbea0254036194be66dc48b598dcea.1284733808.git.jbaron@redhat.com>

[ cleaned up some formatting ]

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/Kconfig                  |    1 +
 arch/x86/include/asm/jump_label.h |   47 ++++++++++++++++++++++++++++++++++
 arch/x86/kernel/Makefile          |    2 +-
 arch/x86/kernel/jump_label.c      |   50 +++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/module.c          |    3 ++
 5 files changed, 102 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/include/asm/jump_label.h
 create mode 100644 arch/x86/kernel/jump_label.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cea0cd9..afcd663 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -59,6 +59,7 @@ config X86
 	select ANON_INODES
 	select HAVE_ARCH_KMEMCHECK
 	select HAVE_USER_RETURN_NOTIFIER
+	select HAVE_ARCH_JUMP_LABEL if !CC_OPTIMIZE_FOR_SIZE
 
 config INSTRUCTION_DECODER
 	def_bool (KPROBES || PERF_EVENTS)
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
new file mode 100644
index 0000000..b4a2cb4
--- /dev/null
+++ b/arch/x86/include/asm/jump_label.h
@@ -0,0 +1,47 @@
+#ifndef _ASM_X86_JUMP_LABEL_H
+#define _ASM_X86_JUMP_LABEL_H
+
+#ifdef __KERNEL__
+
+#include <linux/types.h>
+#include <asm/nops.h>
+
+#define JUMP_LABEL_NOP_SIZE 5
+
+# 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)
+
+#endif /* __KERNEL__ */
+
+#ifdef CONFIG_X86_64
+
+typedef u64 jump_label_t;
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
+#else
+
+typedef u32 jump_label_t;
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
+#endif
+
+#endif
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 0925676..24fa171 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -32,7 +32,7 @@ GCOV_PROFILE_paravirt.o		:= n
 obj-y			:= process_$(BITS).o signal.o entry_$(BITS).o
 obj-y			+= traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o
 obj-y			+= time.o ioport.o ldt.o dumpstack.o
-obj-y			+= setup.o x86_init.o i8259.o irqinit.o
+obj-y			+= setup.o x86_init.o i8259.o irqinit.o jump_label.o
 obj-$(CONFIG_X86_VISWS)	+= visws_quirks.o
 obj-$(CONFIG_X86_32)	+= probe_roms_32.o
 obj-$(CONFIG_X86_32)	+= sys_i386_32.o i386_ksyms_32.o
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
new file mode 100644
index 0000000..961b6b3
--- /dev/null
+++ b/arch/x86/kernel/jump_label.c
@@ -0,0 +1,50 @@
+/*
+ * jump label x86 support
+ *
+ * Copyright (C) 2009 Jason Baron <jbaron@redhat.com>
+ *
+ */
+#include <linux/jump_label.h>
+#include <linux/memory.h>
+#include <linux/uaccess.h>
+#include <linux/module.h>
+#include <linux/list.h>
+#include <linux/jhash.h>
+#include <linux/cpu.h>
+#include <asm/kprobes.h>
+#include <asm/alternative.h>
+
+#ifdef HAVE_JUMP_LABEL
+
+union jump_code_union {
+	char code[JUMP_LABEL_NOP_SIZE];
+	struct {
+		char jump;
+		int offset;
+	} __attribute__((packed));
+};
+
+void arch_jump_label_transform(struct jump_entry *entry,
+			       enum jump_label_type type)
+{
+	union jump_code_union code;
+
+	if (type == JUMP_LABEL_ENABLE) {
+		code.jump = 0xe9;
+		code.offset = entry->target -
+				(entry->code + JUMP_LABEL_NOP_SIZE);
+	} else
+		memcpy(&code, ideal_nop5, JUMP_LABEL_NOP_SIZE);
+	get_online_cpus();
+	mutex_lock(&text_mutex);
+	text_poke_smp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+	mutex_unlock(&text_mutex);
+	put_online_cpus();
+}
+
+void arch_jump_label_text_poke_early(jump_label_t addr)
+{
+	text_poke_early((void *)addr, ideal_nop5, JUMP_LABEL_NOP_SIZE);
+}
+
+#endif
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index e0bc186..5399f58 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -239,6 +239,9 @@ int module_finalize(const Elf_Ehdr *hdr,
 		apply_paravirt(pseg, pseg + para->sh_size);
 	}
 
+	/* make jump label nops */
+	jump_label_apply_nops(me);
+
 	return module_bug_finalize(hdr, sechdrs, me);
 }
 
-- 
1.7.1

[PATCH 09/11] jump label: Add sparc64 support

[Steven Rostedt]

[-- Attachment #1: 0009-jump-label-Add-sparc64-support.patch --]
[-- Type: text/plain, Size: 4092 bytes --]

From: David S. Miller <davem@davemloft.net>

Add jump label support for sparc64.

Signed-off-by: David S. Miller <davem@davemloft.net>
LKML-Reference: <3b5b071fcdb2afb7f67cacecfa78b14c740278a7.1284733808.git.jbaron@redhat.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>

[ cleaned up some formatting ]

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sparc/Kconfig                  |    1 +
 arch/sparc/include/asm/jump_label.h |   32 +++++++++++++++++++++++
 arch/sparc/kernel/Makefile          |    2 +
 arch/sparc/kernel/jump_label.c      |   47 +++++++++++++++++++++++++++++++++++
 arch/sparc/kernel/module.c          |    6 ++++
 5 files changed, 88 insertions(+), 0 deletions(-)
 create mode 100644 arch/sparc/include/asm/jump_label.h
 create mode 100644 arch/sparc/kernel/jump_label.c

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 491e9d6..a81b04e 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -30,6 +30,7 @@ config SPARC
 	select PERF_USE_VMALLOC
 	select HAVE_DMA_ATTRS
 	select HAVE_DMA_API_DEBUG
+	select HAVE_ARCH_JUMP_LABEL if !CC_OPTIMIZE_FOR_SIZE
 
 config SPARC32
 	def_bool !64BIT
diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
new file mode 100644
index 0000000..62e66d7
--- /dev/null
+++ b/arch/sparc/include/asm/jump_label.h
@@ -0,0 +1,32 @@
+#ifndef _ASM_SPARC_JUMP_LABEL_H
+#define _ASM_SPARC_JUMP_LABEL_H
+
+#ifdef __KERNEL__
+
+#include <linux/types.h>
+#include <asm/system.h>
+
+#define JUMP_LABEL_NOP_SIZE 4
+
+#define JUMP_LABEL(key, label)					\
+	do {							\
+		asm goto("1:\n\t"				\
+			 "nop\n\t"				\
+			 "nop\n\t"				\
+			 ".pushsection __jump_table,  \"a\"\n\t"\
+			 ".word 1b, %l[" #label "], %c0\n\t"	\
+			 ".popsection \n\t"			\
+			 : :  "i" (key) :  : label);\
+	} while (0)
+
+#endif /* __KERNEL__ */
+
+typedef u32 jump_label_t;
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
+#endif
diff --git a/arch/sparc/kernel/Makefile b/arch/sparc/kernel/Makefile
index 0c2dc1f..599398f 100644
--- a/arch/sparc/kernel/Makefile
+++ b/arch/sparc/kernel/Makefile
@@ -119,3 +119,5 @@ obj-$(CONFIG_COMPAT)    += $(audit--y)
 
 pc--$(CONFIG_PERF_EVENTS) := perf_event.o
 obj-$(CONFIG_SPARC64)	+= $(pc--y)
+
+obj-$(CONFIG_SPARC64)	+= jump_label.o
diff --git a/arch/sparc/kernel/jump_label.c b/arch/sparc/kernel/jump_label.c
new file mode 100644
index 0000000..ea2dafc
--- /dev/null
+++ b/arch/sparc/kernel/jump_label.c
@@ -0,0 +1,47 @@
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/cpu.h>
+
+#include <linux/jump_label.h>
+#include <linux/memory.h>
+
+#ifdef HAVE_JUMP_LABEL
+
+void arch_jump_label_transform(struct jump_entry *entry,
+			       enum jump_label_type type)
+{
+	u32 val;
+	u32 *insn = (u32 *) (unsigned long) entry->code;
+
+	if (type == JUMP_LABEL_ENABLE) {
+		s32 off = (s32)entry->target - (s32)entry->code;
+
+#ifdef CONFIG_SPARC64
+		/* ba,pt %xcc, . + (off << 2) */
+		val = 0x10680000 | ((u32) off >> 2);
+#else
+		/* ba . + (off << 2) */
+		val = 0x10800000 | ((u32) off >> 2);
+#endif
+	} else {
+		val = 0x01000000;
+	}
+
+	get_online_cpus();
+	mutex_lock(&text_mutex);
+	*insn = val;
+	flushi(insn);
+	mutex_unlock(&text_mutex);
+	put_online_cpus();
+}
+
+void arch_jump_label_text_poke_early(jump_label_t addr)
+{
+	u32 *insn_p = (u32 *) (unsigned long) addr;
+
+	*insn_p = 0x01000000;
+	flushi(insn_p);
+}
+
+#endif
diff --git a/arch/sparc/kernel/module.c b/arch/sparc/kernel/module.c
index f848aad..ee3c7dd 100644
--- a/arch/sparc/kernel/module.c
+++ b/arch/sparc/kernel/module.c
@@ -18,6 +18,9 @@
 #include <asm/spitfire.h>
 
 #ifdef CONFIG_SPARC64
+
+#include <linux/jump_label.h>
+
 static void *module_map(unsigned long size)
 {
 	struct vm_struct *area;
@@ -227,6 +230,9 @@ int module_finalize(const Elf_Ehdr *hdr,
 		    const Elf_Shdr *sechdrs,
 		    struct module *me)
 {
+	/* make jump label nops */
+	jump_label_apply_nops(me);
+
 	/* Cheetah's I-cache is fully coherent.  */
 	if (tlb_type == spitfire) {
 		unsigned long va;
-- 
1.7.1

[PATCH 10/11] jump label: Remove duplicate structure for x86

[Steven Rostedt]

[-- Attachment #1: 0010-jump-label-Remove-duplicate-structure-for-x86.patch --]
[-- Type: text/plain, Size: 920 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The structure in the x86 jump label code uses the typedef jump_label_t,
which is defined by the #ifdef arch type. The structure does not need
to be duplicated there.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/include/asm/jump_label.h |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index b4a2cb4..f52d42e 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -23,18 +23,10 @@
 #endif /* __KERNEL__ */
 
 #ifdef CONFIG_X86_64
-
 typedef u64 jump_label_t;
-
-struct jump_entry {
-	jump_label_t code;
-	jump_label_t target;
-	jump_label_t key;
-};
-
 #else
-
 typedef u32 jump_label_t;
+#endif
 
 struct jump_entry {
 	jump_label_t code;
@@ -43,5 +35,3 @@ struct jump_entry {
 };
 
 #endif
-
-#endif
-- 
1.7.1

[PATCH 11/11] jump label/x86/sparc64: Remove !CC_OPTIMIZE_FOR_SIZE config conditions

[Steven Rostedt]

[-- Attachment #1: 0011-jump-label-x86-sparc64-Remove-CC_OPTIMIZE_FOR_SIZE-c.patch --]
[-- Type: text/plain, Size: 1372 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The !CC_OPTIMIZE_FOR_SIZE was added to enable the jump label functionality
because Jason noticed that the gcc option would not optimize the labels
and may even hurt performance.

But this is a gcc problem not a kernel one. Removing this condition should
add motivation to the gcc developers to actually fix it.

Cc: Jason Baron <jbaron@redhat.com>
Acked-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sparc/Kconfig |    2 +-
 arch/x86/Kconfig   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index a81b04e..9212cd4 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -30,7 +30,7 @@ config SPARC
 	select PERF_USE_VMALLOC
 	select HAVE_DMA_ATTRS
 	select HAVE_DMA_API_DEBUG
-	select HAVE_ARCH_JUMP_LABEL if !CC_OPTIMIZE_FOR_SIZE
+	select HAVE_ARCH_JUMP_LABEL
 
 config SPARC32
 	def_bool !64BIT
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index afcd663..b431a08 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -59,7 +59,7 @@ config X86
 	select ANON_INODES
 	select HAVE_ARCH_KMEMCHECK
 	select HAVE_USER_RETURN_NOTIFIER
-	select HAVE_ARCH_JUMP_LABEL if !CC_OPTIMIZE_FOR_SIZE
+	select HAVE_ARCH_JUMP_LABEL
 
 config INSTRUCTION_DECODER
 	def_bool (KPROBES || PERF_EVENTS)
-- 
1.7.1

Re: [GIT PULL] jump label: add jump label code

[Steven Rostedt]

Hmm, interesting that the Subject is missing the [PATCH 00/11].

Ah! doing a test, I see that "quilt mail" will not add that, if you
delete the space after "Subject:".  Yes, I develop in git, but send out
patches in quilt. That's just the way I am.

-- Steve

Re: [GIT PULL] jump label: add jump label code

[Ingo Molnar]

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

> Hmm, interesting that the Subject is missing the [PATCH 00/11].
> 
> Ah! doing a test, I see that "quilt mail" will not add that, if you 
> delete the space after "Subject:".  Yes, I develop in git, but send 
> out patches in quilt. That's just the way I am.

The lack of 00/11 doesnt even look bad - the threading of the emails you 
sent was correct and i didnt even notice that the 00/11 is missing. The 
numbering of the rest is what matters, and proper threading.

( The more annoying variants are the recursive-nested Git sends - and
  the totally unthreaded random-lkml-order-of-the-day patch dumps. )

Thanks,

	Ingo

Re: [GIT PULL] jump label: add jump label code

[Steven Rostedt]

On Wed, 2010-09-22 at 23:49 -0400, Steven Rostedt wrote:

> Jason Baron (8):
>       jump label: Make dynamic no-op selection available outside of ftrace
>       jump label: Make text_poke_early() globally visible
>       jump label: Base patch for jump label
>       jump label: Initialize workqueue tracepoints *before* they are registered
>       jump label: Add jump_label_text_reserved() to reserve jump points
>       jump label: Tracepoint support for jump labels
>       jump label: Convert dynamic debug to use jump labels
>       jump label: x86 support

Jason,

If you noticed, I left out your last patch: the documentation one. I did
this because there was enough comments to require a resubmit. Could you
just send that one patch, with the corrections, by itself?

Thanks,

-- Steve

Re: [PATCH 03/11] jump label: Base patch for jump label

[Mathieu Desnoyers]

* Steven Rostedt (rostedt@goodmis.org) wrote:
> From: Jason Baron <jbaron@redhat.com>
> 
> base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
> assembly gcc mechanism, we can now branch to labels from an 'asm goto'
> statment. This allows us to create a 'no-op' fastpath, which can subsequently
> be patched with a jump to the slowpath code. This is useful for code which
> might be rarely used, but which we'd like to be able to call, if needed.
> Tracepoints are the current usecase that these are being implemented for.
> 
[...]
> +/***
> + * jump_label_update - update jump label text
> + * @key -  key value associated with a a jump label
> + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> + *
> + * Will enable/disable the jump for jump label @key, depending on the
> + * value of @type.
> + *
> + */
> +
> +void jump_label_update(unsigned long key, enum jump_label_type type)
> +{
> +	struct jump_entry *iter;
> +	struct jump_label_entry *entry;
> +	struct hlist_node *module_node;
> +	struct jump_label_module_entry *e_module;
> +	int count;
> +
> +	mutex_lock(&jump_label_mutex);
> +	entry = get_jump_label_entry((jump_label_t)key);
> +	if (entry) {
> +		count = entry->nr_entries;
> +		iter = entry->table;
> +		while (count--) {
> +			if (kernel_text_address(iter->code))

As I pointed out in another thread, I'm concerned about the use of
kernel_text_address without module mutex here. kernel_text_address calls
is_module_text_address(), which calls __module_text_address() with
preemption off.

__module_text_address() looks like:

struct module *__module_address(unsigned long addr)
{
        struct module *mod;

        if (addr < module_addr_min || addr > module_addr_max)
                return NULL;

        list_for_each_entry_rcu(mod, &modules, list)
                if (within_module_core(addr, mod)
                    || within_module_init(addr, mod))
                        return mod;
        return NULL;
}

struct module *__module_text_address(unsigned long addr)
{
        struct module *mod = __module_address(addr);
        if (mod) {
                /* Make sure it's within the text section. */
                if (!within(addr, mod->module_init, mod->init_text_size)
                    && !within(addr, mod->module_core, mod->core_text_size))
                        mod = NULL;
        }
        return mod;
}

So the test for the address being in the module core is already
problematic, since we hold preempt off only within
is_module_text_address(). The is_module_text_address() caller is then
free to write to this address even after the module has been unloaded
and the module unload grace period ended.

Even worse, such grace period is not waited for at module load time
within:

init_module()
       module_free(mod, mod->module_init);
       mod->module_init = NULL;
       mod->init_size = 0;
       mod->init_text_size = 0;
  (done with module_mutex held, while the module is already in the
   module list)

We'd probably have to hold the module mutex around the
is_module_text_address() call and address use (which can be a pain), or
to correctly address this part of init_module() with RCU and require
that preempt off is held across both __module_text_address() call site
and the actual use of that pointer (which does not fit with jump label,
which need to sleep, so we'd have to move module.c to a preemptable
rcu_read_lock/synchronize_rcu() C.S.).

Thoughts ?

Mathieu


> +				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))
> +					arch_jump_label_transform(iter, type);
> +				iter++;
> +			}
> +		}
> +	}
> +	mutex_unlock(&jump_label_mutex);
-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 03/11] jump label: Base patch for jump label

[Jason Baron]

On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:
> * Steven Rostedt (rostedt@goodmis.org) wrote:
> > From: Jason Baron <jbaron@redhat.com>
> > 
> > base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
> > assembly gcc mechanism, we can now branch to labels from an 'asm goto'
> > statment. This allows us to create a 'no-op' fastpath, which can subsequently
> > be patched with a jump to the slowpath code. This is useful for code which
> > might be rarely used, but which we'd like to be able to call, if needed.
> > Tracepoints are the current usecase that these are being implemented for.
> > 
> [...]
> > +/***
> > + * jump_label_update - update jump label text
> > + * @key -  key value associated with a a jump label
> > + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> > + *
> > + * Will enable/disable the jump for jump label @key, depending on the
> > + * value of @type.
> > + *
> > + */
> > +
> > +void jump_label_update(unsigned long key, enum jump_label_type type)
> > +{
> > +	struct jump_entry *iter;
> > +	struct jump_label_entry *entry;
> > +	struct hlist_node *module_node;
> > +	struct jump_label_module_entry *e_module;
> > +	int count;
> > +
> > +	mutex_lock(&jump_label_mutex);
> > +	entry = get_jump_label_entry((jump_label_t)key);
> > +	if (entry) {
> > +		count = entry->nr_entries;
> > +		iter = entry->table;
> > +		while (count--) {
> > +			if (kernel_text_address(iter->code))
> 
> As I pointed out in another thread, I'm concerned about the use of
> kernel_text_address without module mutex here. kernel_text_address calls
> is_module_text_address(), which calls __module_text_address() with
> preemption off.
> 
> __module_text_address() looks like:
> 
> struct module *__module_address(unsigned long addr)
> {
>         struct module *mod;
> 
>         if (addr < module_addr_min || addr > module_addr_max)
>                 return NULL;
> 
>         list_for_each_entry_rcu(mod, &modules, list)
>                 if (within_module_core(addr, mod)
>                     || within_module_init(addr, mod))
>                         return mod;
>         return NULL;
> }
> 
> struct module *__module_text_address(unsigned long addr)
> {
>         struct module *mod = __module_address(addr);
>         if (mod) {
>                 /* Make sure it's within the text section. */
>                 if (!within(addr, mod->module_init, mod->init_text_size)
>                     && !within(addr, mod->module_core, mod->core_text_size))
>                         mod = NULL;
>         }
>         return mod;
> }
> 
> So the test for the address being in the module core is already
> problematic, since we hold preempt off only within
> is_module_text_address(). The is_module_text_address() caller is then
> free to write to this address even after the module has been unloaded
> and the module unload grace period ended.
> 
> Even worse, such grace period is not waited for at module load time
> within:
> 
> init_module()
>        module_free(mod, mod->module_init);
>        mod->module_init = NULL;
>        mod->init_size = 0;
>        mod->init_text_size = 0;
>   (done with module_mutex held, while the module is already in the
>    module list)
> 
> We'd probably have to hold the module mutex around the
> is_module_text_address() call and address use (which can be a pain), or
> to correctly address this part of init_module() with RCU and require
> that preempt off is held across both __module_text_address() call site
> and the actual use of that pointer (which does not fit with jump label,
> which need to sleep, so we'd have to move module.c to a preemptable
> rcu_read_lock/synchronize_rcu() C.S.).
> 
> Thoughts ?
> 

I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
We can hold the rcu_read_lock() across the is_module_text_address()
check in the jump label code, and then we can do in module.c:

mod->module_init = NULL;
synchronize_rcu();
module_free(mod, mod->module_init);
.
.
.

or we could push the rcu_read_lock() further down into
is_module_address()?

thanks,

-Jason

Re: [PATCH 03/11] jump label: Base patch for jump label

[Mathieu Desnoyers]

* Jason Baron (jbaron@redhat.com) wrote:
> On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:
> > * Steven Rostedt (rostedt@goodmis.org) wrote:
> > > From: Jason Baron <jbaron@redhat.com>
> > > 
> > > base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
> > > assembly gcc mechanism, we can now branch to labels from an 'asm goto'
> > > statment. This allows us to create a 'no-op' fastpath, which can subsequently
> > > be patched with a jump to the slowpath code. This is useful for code which
> > > might be rarely used, but which we'd like to be able to call, if needed.
> > > Tracepoints are the current usecase that these are being implemented for.
> > > 
> > [...]
> > > +/***
> > > + * jump_label_update - update jump label text
> > > + * @key -  key value associated with a a jump label
> > > + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> > > + *
> > > + * Will enable/disable the jump for jump label @key, depending on the
> > > + * value of @type.
> > > + *
> > > + */
> > > +
> > > +void jump_label_update(unsigned long key, enum jump_label_type type)
> > > +{
> > > +	struct jump_entry *iter;
> > > +	struct jump_label_entry *entry;
> > > +	struct hlist_node *module_node;
> > > +	struct jump_label_module_entry *e_module;
> > > +	int count;
> > > +
> > > +	mutex_lock(&jump_label_mutex);
> > > +	entry = get_jump_label_entry((jump_label_t)key);
> > > +	if (entry) {
> > > +		count = entry->nr_entries;
> > > +		iter = entry->table;
> > > +		while (count--) {
> > > +			if (kernel_text_address(iter->code))
> > 
> > As I pointed out in another thread, I'm concerned about the use of
> > kernel_text_address without module mutex here. kernel_text_address calls
> > is_module_text_address(), which calls __module_text_address() with
> > preemption off.
> > 
> > __module_text_address() looks like:
> > 
> > struct module *__module_address(unsigned long addr)
> > {
> >         struct module *mod;
> > 
> >         if (addr < module_addr_min || addr > module_addr_max)
> >                 return NULL;
> > 
> >         list_for_each_entry_rcu(mod, &modules, list)
> >                 if (within_module_core(addr, mod)
> >                     || within_module_init(addr, mod))
> >                         return mod;
> >         return NULL;
> > }
> > 
> > struct module *__module_text_address(unsigned long addr)
> > {
> >         struct module *mod = __module_address(addr);
> >         if (mod) {
> >                 /* Make sure it's within the text section. */
> >                 if (!within(addr, mod->module_init, mod->init_text_size)
> >                     && !within(addr, mod->module_core, mod->core_text_size))
> >                         mod = NULL;
> >         }
> >         return mod;
> > }
> > 
> > So the test for the address being in the module core is already
> > problematic, since we hold preempt off only within
> > is_module_text_address(). The is_module_text_address() caller is then
> > free to write to this address even after the module has been unloaded
> > and the module unload grace period ended.
> > 
> > Even worse, such grace period is not waited for at module load time
> > within:
> > 
> > init_module()
> >        module_free(mod, mod->module_init);
> >        mod->module_init = NULL;
> >        mod->init_size = 0;
> >        mod->init_text_size = 0;
> >   (done with module_mutex held, while the module is already in the
> >    module list)
> > 
> > We'd probably have to hold the module mutex around the
> > is_module_text_address() call and address use (which can be a pain), or
> > to correctly address this part of init_module() with RCU and require
> > that preempt off is held across both __module_text_address() call site
> > and the actual use of that pointer (which does not fit with jump label,
> > which need to sleep, so we'd have to move module.c to a preemptable
> > rcu_read_lock/synchronize_rcu() C.S.).
> > 
> > Thoughts ?
> > 
> 
> I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
> We can hold the rcu_read_lock() across the is_module_text_address()
> check in the jump label code, and then we can do in module.c:
> 
> mod->module_init = NULL;
> synchronize_rcu();
> module_free(mod, mod->module_init);

Beware that you need to copy the module_init address. Also make sure you
audit the "module_free" (per-architecture) to make sure they don't use
"mod" in ways you did not foresee.

> .
> .
> .
> 
> or we could push the rcu_read_lock() further down into
> is_module_address()?

We need to pull rcu_read_lock further _up_. It needs to be held across
both is_module_address() and the actual use of the address, otherwise
the memory mapping can be removed underneath us.

You can see the rcu read lock as keeping the memory mapping alive for as
long as the rcu read lock is held.

We'd also need to add a synchronize_rcu() in module removal.

Thanks,

Mathieu

> 
> thanks,
> 
> -Jason
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 03/11] jump label: Base patch for jump label

[Steven Rostedt]

On Thu, 2010-09-23 at 11:39 -0400, Jason Baron wrote:
> On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:

> I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
> We can hold the rcu_read_lock() across the is_module_text_address()
> check in the jump label code, and then we can do in module.c:
> 
> mod->module_init = NULL;
> synchronize_rcu();
> module_free(mod, mod->module_init);

Um, isn't that last call the same as:

	module_free(mod, NULL); ?

I'll spend some time looking at this too.

-- Steve

> .
> .
> .
> 
> or we could push the rcu_read_lock() further down into
> is_module_address()?
> 
> thanks,
> 
> -Jason
> 

Re: [PATCH 03/11] jump label: Base patch for jump label

[Mathieu Desnoyers]

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Thu, 2010-09-23 at 11:39 -0400, Jason Baron wrote:
> > On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:
> 
> > I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
> > We can hold the rcu_read_lock() across the is_module_text_address()
> > check in the jump label code, and then we can do in module.c:
> > 
> > mod->module_init = NULL;
> > synchronize_rcu();
> > module_free(mod, mod->module_init);
> 
> Um, isn't that last call the same as:
> 
> 	module_free(mod, NULL); ?

Yes, this is incorrect (as I pointed out in my reply). We should do,
instead:

module_init = mod->module_init;
mod->module_init = NULL;
synchronize_rcu();
module_free(mod, module_init);

Thanks,

Mathieu

> 
> I'll spend some time looking at this too.
> 
> -- Steve
> 
> > .
> > .
> > .
> > 
> > or we could push the rcu_read_lock() further down into
> > is_module_address()?
> > 
> > thanks,
> > 
> > -Jason
> > 
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 03/11] jump label: Base patch for jump label

[Jason Baron]

On Thu, Sep 23, 2010 at 11:48:52AM -0400, Mathieu Desnoyers wrote:
> * Jason Baron (jbaron@redhat.com) wrote:
> > On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:
> > > * Steven Rostedt (rostedt@goodmis.org) wrote:
> > > > From: Jason Baron <jbaron@redhat.com>
> > > > 
> > > > base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
> > > > assembly gcc mechanism, we can now branch to labels from an 'asm goto'
> > > > statment. This allows us to create a 'no-op' fastpath, which can subsequently
> > > > be patched with a jump to the slowpath code. This is useful for code which
> > > > might be rarely used, but which we'd like to be able to call, if needed.
> > > > Tracepoints are the current usecase that these are being implemented for.
> > > > 
> > > [...]
> > > > +/***
> > > > + * jump_label_update - update jump label text
> > > > + * @key -  key value associated with a a jump label
> > > > + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> > > > + *
> > > > + * Will enable/disable the jump for jump label @key, depending on the
> > > > + * value of @type.
> > > > + *
> > > > + */
> > > > +
> > > > +void jump_label_update(unsigned long key, enum jump_label_type type)
> > > > +{
> > > > +	struct jump_entry *iter;
> > > > +	struct jump_label_entry *entry;
> > > > +	struct hlist_node *module_node;
> > > > +	struct jump_label_module_entry *e_module;
> > > > +	int count;
> > > > +
> > > > +	mutex_lock(&jump_label_mutex);
> > > > +	entry = get_jump_label_entry((jump_label_t)key);
> > > > +	if (entry) {
> > > > +		count = entry->nr_entries;
> > > > +		iter = entry->table;
> > > > +		while (count--) {
> > > > +			if (kernel_text_address(iter->code))
> > > 
> > > As I pointed out in another thread, I'm concerned about the use of
> > > kernel_text_address without module mutex here. kernel_text_address calls
> > > is_module_text_address(), which calls __module_text_address() with
> > > preemption off.
> > > 
> > > __module_text_address() looks like:
> > > 
> > > struct module *__module_address(unsigned long addr)
> > > {
> > >         struct module *mod;
> > > 
> > >         if (addr < module_addr_min || addr > module_addr_max)
> > >                 return NULL;
> > > 
> > >         list_for_each_entry_rcu(mod, &modules, list)
> > >                 if (within_module_core(addr, mod)
> > >                     || within_module_init(addr, mod))
> > >                         return mod;
> > >         return NULL;
> > > }
> > > 
> > > struct module *__module_text_address(unsigned long addr)
> > > {
> > >         struct module *mod = __module_address(addr);
> > >         if (mod) {
> > >                 /* Make sure it's within the text section. */
> > >                 if (!within(addr, mod->module_init, mod->init_text_size)
> > >                     && !within(addr, mod->module_core, mod->core_text_size))
> > >                         mod = NULL;
> > >         }
> > >         return mod;
> > > }
> > > 
> > > So the test for the address being in the module core is already
> > > problematic, since we hold preempt off only within
> > > is_module_text_address(). The is_module_text_address() caller is then
> > > free to write to this address even after the module has been unloaded
> > > and the module unload grace period ended.
> > > 
> > > Even worse, such grace period is not waited for at module load time
> > > within:
> > > 
> > > init_module()
> > >        module_free(mod, mod->module_init);
> > >        mod->module_init = NULL;
> > >        mod->init_size = 0;
> > >        mod->init_text_size = 0;
> > >   (done with module_mutex held, while the module is already in the
> > >    module list)
> > > 
> > > We'd probably have to hold the module mutex around the
> > > is_module_text_address() call and address use (which can be a pain), or
> > > to correctly address this part of init_module() with RCU and require
> > > that preempt off is held across both __module_text_address() call site
> > > and the actual use of that pointer (which does not fit with jump label,
> > > which need to sleep, so we'd have to move module.c to a preemptable
> > > rcu_read_lock/synchronize_rcu() C.S.).
> > > 
> > > Thoughts ?
> > > 
> > 
> > I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
> > We can hold the rcu_read_lock() across the is_module_text_address()
> > check in the jump label code, and then we can do in module.c:
> > 
> > mod->module_init = NULL;
> > synchronize_rcu();
> > module_free(mod, mod->module_init);
> 
> Beware that you need to copy the module_init address. Also make sure you
> audit the "module_free" (per-architecture) to make sure they don't use
> "mod" in ways you did not foresee.
> 
> > .
> > .
> > .
> > 
> > or we could push the rcu_read_lock() further down into
> > is_module_address()?
> 
> We need to pull rcu_read_lock further _up_. It needs to be held across
> both is_module_address() and the actual use of the address, otherwise
> the memory mapping can be removed underneath us.
> 
> You can see the rcu read lock as keeping the memory mapping alive for as
> long as the rcu read lock is held.
> 
> We'd also need to add a synchronize_rcu() in module removal.
> 

I agree that we this synchronization for the module __init section.

However, I believe we are ok for module removal case. free_module() is
called *after* blocking_notifier_call_chain() call. The
blocking_notifier_call_chain() is going to call back into the jump label
code, grab the jump_label_mutex and remove the reference to the module that
is about to freed. Thus, the jump label code can no longer reference it.

So I think the following patch is all that is required here (lightly
tested).

Steve, I'll re-post as a separate patch, if we agree on this fix.

thanks,

-Jason




jump label: fix __init module section race

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 free out from under
us.

We fix this potential race putting the address check *and* the jump
label update under a rcu_read_lock(), and making sure a grace period
has completed before we free the __init section.

Thanks to Mathieu Desnoyers for pointing out this race condition.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 kernel/jump_label.c |    2 ++
 kernel/module.c     |    5 ++++-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index f82878b..7830bfb 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -160,6 +160,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 			iter++;
 		}
 		/* eanble/disable jump labels in modules */
+		rcu_read_lock();
 		hlist_for_each_entry(e_module, module_node, &(entry->modules),
 							hlist) {
 			count = e_module->nr_entries;
@@ -170,6 +171,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
 				iter++;
 			}
 		}
+		rcu_read_unlock();
 	}
 	mutex_unlock(&jump_label_mutex);
 }
diff --git a/kernel/module.c b/kernel/module.c
index eba1341..09f7e9e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2692,6 +2692,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
 		unsigned long, len, const char __user *, uargs)
 {
 	struct module *mod;
+	void *init_code;
 	int ret = 0;
 
 	/* Must have permission */
@@ -2749,8 +2750,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
 	mod->symtab = mod->core_symtab;
 	mod->strtab = mod->core_strtab;
 #endif
-	module_free(mod, mod->module_init);
+	init_code = mod->module_init;
 	mod->module_init = NULL;
+	synchronize_rcu();
+	module_free(mod, init_code);
 	mod->init_size = 0;
 	mod->init_text_size = 0;
 	mutex_unlock(&module_mutex);
-- 
1.7.1

Re: [PATCH 03/11] jump label: Base patch for jump label

[Mathieu Desnoyers]

* Jason Baron (jbaron@redhat.com) wrote:
> On Thu, Sep 23, 2010 at 11:48:52AM -0400, Mathieu Desnoyers wrote:
> > * Jason Baron (jbaron@redhat.com) wrote:
> > > On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:
> > > > * Steven Rostedt (rostedt@goodmis.org) wrote:
> > > > > From: Jason Baron <jbaron@redhat.com>
> > > > > 
> > > > > base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
> > > > > assembly gcc mechanism, we can now branch to labels from an 'asm goto'
> > > > > statment. This allows us to create a 'no-op' fastpath, which can subsequently
> > > > > be patched with a jump to the slowpath code. This is useful for code which
> > > > > might be rarely used, but which we'd like to be able to call, if needed.
> > > > > Tracepoints are the current usecase that these are being implemented for.
> > > > > 
> > > > [...]
> > > > > +/***
> > > > > + * jump_label_update - update jump label text
> > > > > + * @key -  key value associated with a a jump label
> > > > > + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> > > > > + *
> > > > > + * Will enable/disable the jump for jump label @key, depending on the
> > > > > + * value of @type.
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +void jump_label_update(unsigned long key, enum jump_label_type type)
> > > > > +{
> > > > > +	struct jump_entry *iter;
> > > > > +	struct jump_label_entry *entry;
> > > > > +	struct hlist_node *module_node;
> > > > > +	struct jump_label_module_entry *e_module;
> > > > > +	int count;
> > > > > +
> > > > > +	mutex_lock(&jump_label_mutex);
> > > > > +	entry = get_jump_label_entry((jump_label_t)key);
> > > > > +	if (entry) {
> > > > > +		count = entry->nr_entries;
> > > > > +		iter = entry->table;
> > > > > +		while (count--) {
> > > > > +			if (kernel_text_address(iter->code))
> > > > 
> > > > As I pointed out in another thread, I'm concerned about the use of
> > > > kernel_text_address without module mutex here. kernel_text_address calls
> > > > is_module_text_address(), which calls __module_text_address() with
> > > > preemption off.
> > > > 
> > > > __module_text_address() looks like:
> > > > 
> > > > struct module *__module_address(unsigned long addr)
> > > > {
> > > >         struct module *mod;
> > > > 
> > > >         if (addr < module_addr_min || addr > module_addr_max)
> > > >                 return NULL;
> > > > 
> > > >         list_for_each_entry_rcu(mod, &modules, list)
> > > >                 if (within_module_core(addr, mod)
> > > >                     || within_module_init(addr, mod))
> > > >                         return mod;
> > > >         return NULL;
> > > > }
> > > > 
> > > > struct module *__module_text_address(unsigned long addr)
> > > > {
> > > >         struct module *mod = __module_address(addr);
> > > >         if (mod) {
> > > >                 /* Make sure it's within the text section. */
> > > >                 if (!within(addr, mod->module_init, mod->init_text_size)
> > > >                     && !within(addr, mod->module_core, mod->core_text_size))
> > > >                         mod = NULL;
> > > >         }
> > > >         return mod;
> > > > }
> > > > 
> > > > So the test for the address being in the module core is already
> > > > problematic, since we hold preempt off only within
> > > > is_module_text_address(). The is_module_text_address() caller is then
> > > > free to write to this address even after the module has been unloaded
> > > > and the module unload grace period ended.
> > > > 
> > > > Even worse, such grace period is not waited for at module load time
> > > > within:
> > > > 
> > > > init_module()
> > > >        module_free(mod, mod->module_init);
> > > >        mod->module_init = NULL;
> > > >        mod->init_size = 0;
> > > >        mod->init_text_size = 0;
> > > >   (done with module_mutex held, while the module is already in the
> > > >    module list)
> > > > 
> > > > We'd probably have to hold the module mutex around the
> > > > is_module_text_address() call and address use (which can be a pain), or
> > > > to correctly address this part of init_module() with RCU and require
> > > > that preempt off is held across both __module_text_address() call site
> > > > and the actual use of that pointer (which does not fit with jump label,
> > > > which need to sleep, so we'd have to move module.c to a preemptable
> > > > rcu_read_lock/synchronize_rcu() C.S.).
> > > > 
> > > > Thoughts ?
> > > > 
> > > 
> > > I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
> > > We can hold the rcu_read_lock() across the is_module_text_address()
> > > check in the jump label code, and then we can do in module.c:
> > > 
> > > mod->module_init = NULL;
> > > synchronize_rcu();
> > > module_free(mod, mod->module_init);
> > 
> > Beware that you need to copy the module_init address. Also make sure you
> > audit the "module_free" (per-architecture) to make sure they don't use
> > "mod" in ways you did not foresee.
> > 
> > > .
> > > .
> > > .
> > > 
> > > or we could push the rcu_read_lock() further down into
> > > is_module_address()?
> > 
> > We need to pull rcu_read_lock further _up_. It needs to be held across
> > both is_module_address() and the actual use of the address, otherwise
> > the memory mapping can be removed underneath us.
> > 
> > You can see the rcu read lock as keeping the memory mapping alive for as
> > long as the rcu read lock is held.
> > 
> > We'd also need to add a synchronize_rcu() in module removal.
> > 
> 
> I agree that we this synchronization for the module __init section.
> 
> However, I believe we are ok for module removal case. free_module() is
> called *after* blocking_notifier_call_chain() call. The
> blocking_notifier_call_chain() is going to call back into the jump label
> code, grab the jump_label_mutex and remove the reference to the module that
> is about to freed. Thus, the jump label code can no longer reference it.

Ah, yes, this makes sense.

> 
> So I think the following patch is all that is required here (lightly
> tested).

Some comments below,

> 
> Steve, I'll re-post as a separate patch, if we agree on this fix.
> 
> thanks,
> 
> -Jason
> 
> 
> 
> 
> jump label: fix __init module section race
> 
> 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 free out from under
> us.
> 
> We fix this potential race putting the address check *and* the jump
> label update under a rcu_read_lock(), and making sure a grace period
> has completed before we free the __init section.
> 
> Thanks to Mathieu Desnoyers for pointing out this race condition.
> 
> Signed-off-by: Jason Baron <jbaron@redhat.com>

You can put my
Reported-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> ---
>  kernel/jump_label.c |    2 ++
>  kernel/module.c     |    5 ++++-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index f82878b..7830bfb 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -160,6 +160,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
>  			iter++;
>  		}
>  		/* eanble/disable jump labels in modules */

Separate patch for typo: eanble -> enable would be appropriate.

> +		rcu_read_lock();
>  		hlist_for_each_entry(e_module, module_node, &(entry->modules),
>  							hlist) {
>  			count = e_module->nr_entries;
> @@ -170,6 +171,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
>  				iter++;
>  			}
>  		}
> +		rcu_read_unlock();

Please note that the impact of this read lock is that synchronize_rcu()
cannot be called from text_poke anymore (because text_poke is called
within a RCU read-side C.S.). So hopefully Masami did not plan to call
synchronize_rcu() from the upcoming breakpoint-based text_poke.

>  	}
>  	mutex_unlock(&jump_label_mutex);
>  }
> diff --git a/kernel/module.c b/kernel/module.c
> index eba1341..09f7e9e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2692,6 +2692,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
>  		unsigned long, len, const char __user *, uargs)
>  {
>  	struct module *mod;
> +	void *init_code;
>  	int ret = 0;
>  
>  	/* Must have permission */
> @@ -2749,8 +2750,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
>  	mod->symtab = mod->core_symtab;
>  	mod->strtab = mod->core_strtab;
>  #endif
> -	module_free(mod, mod->module_init);
> +	init_code = mod->module_init;
>  	mod->module_init = NULL;
> +	synchronize_rcu();
> +	module_free(mod, init_code);

I'm a bit concerned about the fact that "mod" is passed as parameter to
module_free(). It does not seem to be used on x86, but have you reviewed
all other architectures ? I would feel more comfortable if we removed
this parameter from module_free() if it is indeed unused everywhere.

Thanks,

Mathieu

>  	mod->init_size = 0;
>  	mod->init_text_size = 0;
>  	mutex_unlock(&module_mutex);
> -- 
> 1.7.1
> 
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 03/11] jump label: Base patch for jump label

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj@krystal.dyndns.org) wrote:
> * Jason Baron (jbaron@redhat.com) wrote:
[...]
> > jump label: fix __init module section race
> > 
> > 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 free out from under
> > us.
> > 
> > We fix this potential race putting the address check *and* the jump
> > label update under a rcu_read_lock(), and making sure a grace period
> > has completed before we free the __init section.
> > 
> > Thanks to Mathieu Desnoyers for pointing out this race condition.

Actually, there might be another way to deal with this that would not
require holding rcu read lock at all in this path, nor adding
synchronize_rcu in the module code.

This involves adding a notifier right before the init code is freed, so
the static jump code can disable all static jumps pointing to init code
area for the given module.

On the plus side, this means text_poke would be free to still use
synchronize_rcu() if it ever needs to. Also, calling a notifier chain
before freeing the module init region will hurt module load time much
less than waiting for a grace period.

Thoughts ?

Mathieu

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 03/11] jump label: Base patch for jump label

[Jason Baron]

On Thu, Sep 23, 2010 at 02:55:40PM -0400, Mathieu Desnoyers wrote:
> * Jason Baron (jbaron@redhat.com) wrote:
> > On Thu, Sep 23, 2010 at 11:48:52AM -0400, Mathieu Desnoyers wrote:
> > > * Jason Baron (jbaron@redhat.com) wrote:
> > > > On Thu, Sep 23, 2010 at 10:37:58AM -0400, Mathieu Desnoyers wrote:
> > > > > * Steven Rostedt (rostedt@goodmis.org) wrote:
> > > > > > From: Jason Baron <jbaron@redhat.com>
> > > > > > 
> > > > > > base patch to implement 'jump labeling'. Based on a new 'asm goto' inline
> > > > > > assembly gcc mechanism, we can now branch to labels from an 'asm goto'
> > > > > > statment. This allows us to create a 'no-op' fastpath, which can subsequently
> > > > > > be patched with a jump to the slowpath code. This is useful for code which
> > > > > > might be rarely used, but which we'd like to be able to call, if needed.
> > > > > > Tracepoints are the current usecase that these are being implemented for.
> > > > > > 
> > > > > [...]
> > > > > > +/***
> > > > > > + * jump_label_update - update jump label text
> > > > > > + * @key -  key value associated with a a jump label
> > > > > > + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> > > > > > + *
> > > > > > + * Will enable/disable the jump for jump label @key, depending on the
> > > > > > + * value of @type.
> > > > > > + *
> > > > > > + */
> > > > > > +
> > > > > > +void jump_label_update(unsigned long key, enum jump_label_type type)
> > > > > > +{
> > > > > > +	struct jump_entry *iter;
> > > > > > +	struct jump_label_entry *entry;
> > > > > > +	struct hlist_node *module_node;
> > > > > > +	struct jump_label_module_entry *e_module;
> > > > > > +	int count;
> > > > > > +
> > > > > > +	mutex_lock(&jump_label_mutex);
> > > > > > +	entry = get_jump_label_entry((jump_label_t)key);
> > > > > > +	if (entry) {
> > > > > > +		count = entry->nr_entries;
> > > > > > +		iter = entry->table;
> > > > > > +		while (count--) {
> > > > > > +			if (kernel_text_address(iter->code))
> > > > > 
> > > > > As I pointed out in another thread, I'm concerned about the use of
> > > > > kernel_text_address without module mutex here. kernel_text_address calls
> > > > > is_module_text_address(), which calls __module_text_address() with
> > > > > preemption off.
> > > > > 
> > > > > __module_text_address() looks like:
> > > > > 
> > > > > struct module *__module_address(unsigned long addr)
> > > > > {
> > > > >         struct module *mod;
> > > > > 
> > > > >         if (addr < module_addr_min || addr > module_addr_max)
> > > > >                 return NULL;
> > > > > 
> > > > >         list_for_each_entry_rcu(mod, &modules, list)
> > > > >                 if (within_module_core(addr, mod)
> > > > >                     || within_module_init(addr, mod))
> > > > >                         return mod;
> > > > >         return NULL;
> > > > > }
> > > > > 
> > > > > struct module *__module_text_address(unsigned long addr)
> > > > > {
> > > > >         struct module *mod = __module_address(addr);
> > > > >         if (mod) {
> > > > >                 /* Make sure it's within the text section. */
> > > > >                 if (!within(addr, mod->module_init, mod->init_text_size)
> > > > >                     && !within(addr, mod->module_core, mod->core_text_size))
> > > > >                         mod = NULL;
> > > > >         }
> > > > >         return mod;
> > > > > }
> > > > > 
> > > > > So the test for the address being in the module core is already
> > > > > problematic, since we hold preempt off only within
> > > > > is_module_text_address(). The is_module_text_address() caller is then
> > > > > free to write to this address even after the module has been unloaded
> > > > > and the module unload grace period ended.
> > > > > 
> > > > > Even worse, such grace period is not waited for at module load time
> > > > > within:
> > > > > 
> > > > > init_module()
> > > > >        module_free(mod, mod->module_init);
> > > > >        mod->module_init = NULL;
> > > > >        mod->init_size = 0;
> > > > >        mod->init_text_size = 0;
> > > > >   (done with module_mutex held, while the module is already in the
> > > > >    module list)
> > > > > 
> > > > > We'd probably have to hold the module mutex around the
> > > > > is_module_text_address() call and address use (which can be a pain), or
> > > > > to correctly address this part of init_module() with RCU and require
> > > > > that preempt off is held across both __module_text_address() call site
> > > > > and the actual use of that pointer (which does not fit with jump label,
> > > > > which need to sleep, so we'd have to move module.c to a preemptable
> > > > > rcu_read_lock/synchronize_rcu() C.S.).
> > > > > 
> > > > > Thoughts ?
> > > > > 
> > > > 
> > > > I was thinking about the rcu_read_lock/synchronize_rcu() for this race.
> > > > We can hold the rcu_read_lock() across the is_module_text_address()
> > > > check in the jump label code, and then we can do in module.c:
> > > > 
> > > > mod->module_init = NULL;
> > > > synchronize_rcu();
> > > > module_free(mod, mod->module_init);
> > > 
> > > Beware that you need to copy the module_init address. Also make sure you
> > > audit the "module_free" (per-architecture) to make sure they don't use
> > > "mod" in ways you did not foresee.
> > > 
> > > > .
> > > > .
> > > > .
> > > > 
> > > > or we could push the rcu_read_lock() further down into
> > > > is_module_address()?
> > > 
> > > We need to pull rcu_read_lock further _up_. It needs to be held across
> > > both is_module_address() and the actual use of the address, otherwise
> > > the memory mapping can be removed underneath us.
> > > 
> > > You can see the rcu read lock as keeping the memory mapping alive for as
> > > long as the rcu read lock is held.
> > > 
> > > We'd also need to add a synchronize_rcu() in module removal.
> > > 
> > 
> > I agree that we this synchronization for the module __init section.
> > 
> > However, I believe we are ok for module removal case. free_module() is
> > called *after* blocking_notifier_call_chain() call. The
> > blocking_notifier_call_chain() is going to call back into the jump label
> > code, grab the jump_label_mutex and remove the reference to the module that
> > is about to freed. Thus, the jump label code can no longer reference it.
> 
> Ah, yes, this makes sense.
> 
> > 
> > So I think the following patch is all that is required here (lightly
> > tested).
> 
> Some comments below,
> 
> > 
> > Steve, I'll re-post as a separate patch, if we agree on this fix.
> > 
> > thanks,
> > 
> > -Jason
> > 
> > 
> > 
> > 
> > jump label: fix __init module section race
> > 
> > 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 free out from under
> > us.
> > 
> > We fix this potential race putting the address check *and* the jump
> > label update under a rcu_read_lock(), and making sure a grace period
> > has completed before we free the __init section.
> > 
> > Thanks to Mathieu Desnoyers for pointing out this race condition.
> > 
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> 
> You can put my
> Reported-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> 
> > ---
> >  kernel/jump_label.c |    2 ++
> >  kernel/module.c     |    5 ++++-
> >  2 files changed, 6 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index f82878b..7830bfb 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -160,6 +160,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
> >  			iter++;
> >  		}
> >  		/* eanble/disable jump labels in modules */
> 
> Separate patch for typo: eanble -> enable would be appropriate.
> 
> > +		rcu_read_lock();
> >  		hlist_for_each_entry(e_module, module_node, &(entry->modules),
> >  							hlist) {
> >  			count = e_module->nr_entries;
> > @@ -170,6 +171,7 @@ void jump_label_update(unsigned long key, enum jump_label_type type)
> >  				iter++;
> >  			}
> >  		}
> > +		rcu_read_unlock();
> 
> Please note that the impact of this read lock is that synchronize_rcu()
> cannot be called from text_poke anymore (because text_poke is called
> within a RCU read-side C.S.). So hopefully Masami did not plan to call
> synchronize_rcu() from the upcoming breakpoint-based text_poke.
> 

ok, we can re-visit this if it becomes an issue at some future
point...thanks for mentioning it.


> >  	}
> >  	mutex_unlock(&jump_label_mutex);
> >  }
> > diff --git a/kernel/module.c b/kernel/module.c
> > index eba1341..09f7e9e 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2692,6 +2692,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
> >  		unsigned long, len, const char __user *, uargs)
> >  {
> >  	struct module *mod;
> > +	void *init_code;
> >  	int ret = 0;
> >  
> >  	/* Must have permission */
> > @@ -2749,8 +2750,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
> >  	mod->symtab = mod->core_symtab;
> >  	mod->strtab = mod->core_strtab;
> >  #endif
> > -	module_free(mod, mod->module_init);
> > +	init_code = mod->module_init;
> >  	mod->module_init = NULL;
> > +	synchronize_rcu();
> > +	module_free(mod, init_code);
> 
> I'm a bit concerned about the fact that "mod" is passed as parameter to
> module_free(). It does not seem to be used on x86, but have you reviewed
> all other architectures ? I would feel more comfortable if we removed
> this parameter from module_free() if it is indeed unused everywhere.
> 

good point.

So you're concern here is that mod->module_init is used in the
'module_free'? indeed i missed it on ia64:

void
module_free (struct module *mod, void *module_region)
{
        if (mod && mod->arch.init_unw_table &&
            module_region == mod->module_init) {
                unw_remove_unwind_table(mod->arch.init_unw_table);
                mod->arch.init_unw_table = NULL;
        }
        vfree(module_region);
}

So, I don't see why we couldn't s/mod->module_init/module_region, but
then I wonder why it wasn't written that way in the first place?

thanks,

-Jason

Re: [PATCH 03/11] jump label: Base patch for jump label

[Rusty Russell]

On Fri, 24 Sep 2010 04:10:06 am Jason Baron wrote:
> @@ -2749,8 +2750,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
>  	mod->symtab = mod->core_symtab;
>  	mod->strtab = mod->core_strtab;
>  #endif
> -	module_free(mod, mod->module_init);
> +	init_code = mod->module_init;
>  	mod->module_init = NULL;
> +	synchronize_rcu();
> +	module_free(mod, init_code);
>  	mod->init_size = 0;
>  	mod->init_text_size = 0;
>  	mutex_unlock(&module_mutex);

When a patch requires more lkml mails than it has lines, it needs a comment.

But step back for a moment: what prompts the jump label update?  Why isn't
that simply done under the module lock, obviating any complexity?

If you're frobbing kernel text all over the place, you probable want the
module lock.  You wouldn't be the first: perhaps we should rename that to
kernel_text_lock...

Apologies if that's a dumb question,
Rusty.

[tip:perf/core] jump label: Fix GCC feature check when distcc is used

[tip-bot for Ingo Molnar]

Commit-ID:  d6dad199a10423ce37b8bfec1f055c380dc4a3d5
Gitweb:     http://git.kernel.org/tip/d6dad199a10423ce37b8bfec1f055c380dc4a3d5
Author:     Ingo Molnar <mingo@elte.hu>
AuthorDate: Fri, 24 Sep 2010 09:12:25 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 24 Sep 2010 09:12:25 +0200

jump label: Fix GCC feature check when distcc is used

The following build bug occurs on distcc builds:

   CC      arch/x86/kernel/asm-offsets.s
 In file included from include/linux/module.h:24,
                  from include/linux/crypto.h:22,
                  from arch/x86/kernel/asm-offsets_64.c:9,
                  from arch/x86/kernel/asm-offsets.c:5:
 include/trace/events/module.h: In function 'trace_module_load':
 include/trace/events/module.h:18: error: expected '(' before 'goto'
 include/trace/events/module.h:18: error: expected identifier or '*' before '(' token

It triggers because distcc is invoked by turning $CC into "distcc gcc",
but gcc-goto.sh check script was using $1 not $@ to expand parameters.

Cc: Jason Baron <jbaron@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20100923034910.867858597@goodmis.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 scripts/gcc-goto.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
index 8e82424..520d16b 100644
--- a/scripts/gcc-goto.sh
+++ b/scripts/gcc-goto.sh
@@ -2,4 +2,4 @@
 # Test for gcc 'asm goto' suport
 # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
 
-echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $1 -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"

Re: [PATCH 03/11] jump label: Base patch for jump label

[Steven Rostedt]

On Thu, 2010-09-23 at 14:40 -0400, Jason Baron wrote:

> I agree that we this synchronization for the module __init section.
> 
> However, I believe we are ok for module removal case. free_module() is
> called *after* blocking_notifier_call_chain() call. The
> blocking_notifier_call_chain() is going to call back into the jump label
> code, grab the jump_label_mutex and remove the reference to the module that
> is about to freed. Thus, the jump label code can no longer reference it.
> 
> So I think the following patch is all that is required here (lightly
> tested).
> 
> Steve, I'll re-post as a separate patch, if we agree on this fix.

Ug, I'm struggling to get ready for my Tokyo trip. I'll try to look at
it on the flight. But I still need to write my presentation :-)

I may have a response back till Monday or Tuesday.

-- Steve

Re: [PATCH 03/11] jump label: Base patch for jump label

[Mathieu Desnoyers]

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Thu, 2010-09-23 at 14:40 -0400, Jason Baron wrote:
> 
> > I agree that we this synchronization for the module __init section.
> > 
> > However, I believe we are ok for module removal case. free_module() is
> > called *after* blocking_notifier_call_chain() call. The
> > blocking_notifier_call_chain() is going to call back into the jump label
> > code, grab the jump_label_mutex and remove the reference to the module that
> > is about to freed. Thus, the jump label code can no longer reference it.
> > 
> > So I think the following patch is all that is required here (lightly
> > tested).
> > 
> > Steve, I'll re-post as a separate patch, if we agree on this fix.
> 
> Ug, I'm struggling to get ready for my Tokyo trip. I'll try to look at
> it on the flight. But I still need to write my presentation :-)
> 
> I may have a response back till Monday or Tuesday.

As a note for when you review this patch, I have a strong preference for
adding a "pre-init-free notifier callback" rather than taking the rcu
read lock + adding a synchronize_rcu() in module.c. This will make our
life much easier when we end up doing modifications down in text_poke.

Thanks,

Mathieu

> 
> -- Steve
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 03/11] jump label: Base patch for jump label

[Jason Baron]

On Fri, Sep 24, 2010 at 04:54:35PM -0400, Mathieu Desnoyers wrote:
> * Steven Rostedt (rostedt@goodmis.org) wrote:
> > On Thu, 2010-09-23 at 14:40 -0400, Jason Baron wrote:
> > 
> > > I agree that we this synchronization for the module __init section.
> > > 
> > > However, I believe we are ok for module removal case. free_module() is
> > > called *after* blocking_notifier_call_chain() call. The
> > > blocking_notifier_call_chain() is going to call back into the jump label
> > > code, grab the jump_label_mutex and remove the reference to the module that
> > > is about to freed. Thus, the jump label code can no longer reference it.
> > > 
> > > So I think the following patch is all that is required here (lightly
> > > tested).
> > > 
> > > Steve, I'll re-post as a separate patch, if we agree on this fix.
> > 
> > Ug, I'm struggling to get ready for my Tokyo trip. I'll try to look at
> > it on the flight. But I still need to write my presentation :-)
> > 
> > I may have a response back till Monday or Tuesday.
> 
> As a note for when you review this patch, I have a strong preference for
> adding a "pre-init-free notifier callback" rather than taking the rcu
> read lock + adding a synchronize_rcu() in module.c. This will make our
> life much easier when we end up doing modifications down in text_poke.
> 
> Thanks,
> 
> Mathieu
> 

Yes, I think we can make this work with a "pre-init-free notifier
callback". In fact, we can use MODULE_STATE_LIVE state that is already
in module.c. I'm in the process of testing a patch using this
alternative method.

thanks,

-Jason