* [PATCH 01/11] jump label: Make dynamic no-op selection available outside of ftrace
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 02/11] jump label: Make text_poke_early() globally visible Steven Rostedt
` (12 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/11] jump label: Make text_poke_early() globally visible
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
2010-09-23 3:49 ` [PATCH 01/11] jump label: Make dynamic no-op selection available outside of ftrace Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 03/11] jump label: Base patch for jump label Steven Rostedt
` (11 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
2010-09-23 3:49 ` [PATCH 01/11] jump label: Make dynamic no-op selection available outside of ftrace Steven Rostedt
2010-09-23 3:49 ` [PATCH 02/11] jump label: Make text_poke_early() globally visible Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 14:37 ` Mathieu Desnoyers
2010-09-23 3:49 ` [PATCH 04/11] jump label: Initialize workqueue tracepoints *before* they are registered Steven Rostedt
` (10 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 3:49 ` [PATCH 03/11] jump label: Base patch for jump label Steven Rostedt
@ 2010-09-23 14:37 ` Mathieu Desnoyers
2010-09-23 15:39 ` Jason Baron
0 siblings, 1 reply; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 14:37 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Andi Kleen, Jason Baron, David Miller, Paul E. McKenney,
Rusty Russell
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 14:37 ` Mathieu Desnoyers
@ 2010-09-23 15:39 ` Jason Baron
2010-09-23 15:48 ` Mathieu Desnoyers
2010-09-23 16:52 ` Steven Rostedt
0 siblings, 2 replies; 29+ messages in thread
From: Jason Baron @ 2010-09-23 15:39 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
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
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 15:39 ` Jason Baron
@ 2010-09-23 15:48 ` Mathieu Desnoyers
2010-09-23 18:40 ` Jason Baron
2010-09-23 16:52 ` Steven Rostedt
1 sibling, 1 reply; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 15:48 UTC (permalink / raw)
To: Jason Baron
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 15:48 ` Mathieu Desnoyers
@ 2010-09-23 18:40 ` Jason Baron
2010-09-23 18:55 ` Mathieu Desnoyers
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Jason Baron @ 2010-09-23 18:40 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 18:40 ` Jason Baron
@ 2010-09-23 18:55 ` Mathieu Desnoyers
2010-09-23 19:08 ` Mathieu Desnoyers
2010-09-23 19:11 ` Jason Baron
2010-09-24 0:44 ` Rusty Russell
2010-09-24 13:54 ` Steven Rostedt
2 siblings, 2 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 18:55 UTC (permalink / raw)
To: Jason Baron
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell, Masami Hiramatsu
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 18:55 ` Mathieu Desnoyers
@ 2010-09-23 19:08 ` Mathieu Desnoyers
2010-09-23 19:11 ` Jason Baron
1 sibling, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 19:08 UTC (permalink / raw)
To: Jason Baron
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell, Masami Hiramatsu
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 18:55 ` Mathieu Desnoyers
2010-09-23 19:08 ` Mathieu Desnoyers
@ 2010-09-23 19:11 ` Jason Baron
1 sibling, 0 replies; 29+ messages in thread
From: Jason Baron @ 2010-09-23 19:11 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell, Masami Hiramatsu
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 18:40 ` Jason Baron
2010-09-23 18:55 ` Mathieu Desnoyers
@ 2010-09-24 0:44 ` Rusty Russell
2010-09-24 13:54 ` Steven Rostedt
2 siblings, 0 replies; 29+ messages in thread
From: Rusty Russell @ 2010-09-24 0:44 UTC (permalink / raw)
To: Jason Baron
Cc: Mathieu Desnoyers, Steven Rostedt, linux-kernel, Ingo Molnar,
Andrew Morton, Frederic Weisbecker, Andi Kleen, David Miller,
Paul E. McKenney
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 18:40 ` Jason Baron
2010-09-23 18:55 ` Mathieu Desnoyers
2010-09-24 0:44 ` Rusty Russell
@ 2010-09-24 13:54 ` Steven Rostedt
2010-09-24 20:54 ` Mathieu Desnoyers
2 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-09-24 13:54 UTC (permalink / raw)
To: Jason Baron
Cc: Mathieu Desnoyers, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-24 13:54 ` Steven Rostedt
@ 2010-09-24 20:54 ` Mathieu Desnoyers
2010-09-24 21:45 ` Jason Baron
0 siblings, 1 reply; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-09-24 20:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: Jason Baron, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-24 20:54 ` Mathieu Desnoyers
@ 2010-09-24 21:45 ` Jason Baron
0 siblings, 0 replies; 29+ messages in thread
From: Jason Baron @ 2010-09-24 21:45 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 15:39 ` Jason Baron
2010-09-23 15:48 ` Mathieu Desnoyers
@ 2010-09-23 16:52 ` Steven Rostedt
2010-09-23 17:09 ` Mathieu Desnoyers
1 sibling, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 16:52 UTC (permalink / raw)
To: Jason Baron
Cc: Mathieu Desnoyers, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
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
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 03/11] jump label: Base patch for jump label
2010-09-23 16:52 ` Steven Rostedt
@ 2010-09-23 17:09 ` Mathieu Desnoyers
0 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 17:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: Jason Baron, linux-kernel, Ingo Molnar, Andrew Morton,
Frederic Weisbecker, Andi Kleen, David Miller, Paul E. McKenney,
Rusty Russell
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 04/11] jump label: Initialize workqueue tracepoints *before* they are registered
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (2 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 03/11] jump label: Base patch for jump label Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 05/11] jump label: Add jump_label_text_reserved() to reserve jump points Steven Rostedt
` (9 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 05/11] jump label: Add jump_label_text_reserved() to reserve jump points
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (3 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 04/11] jump label: Initialize workqueue tracepoints *before* they are registered Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 06/11] jump label: Tracepoint support for jump labels Steven Rostedt
` (8 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller,
Masami Hiramatsu
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 06/11] jump label: Tracepoint support for jump labels
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (4 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 05/11] jump label: Add jump_label_text_reserved() to reserve jump points Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 07/11] jump label: Convert dynamic debug to use " Steven Rostedt
` (7 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/11] jump label: Convert dynamic debug to use jump labels
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (5 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 06/11] jump label: Tracepoint support for jump labels Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 08/11] jump label: x86 support Steven Rostedt
` (6 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/11] jump label: x86 support
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (6 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 07/11] jump label: Convert dynamic debug to use " Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 09/11] jump label: Add sparc64 support Steven Rostedt
` (5 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/11] jump label: Add sparc64 support
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (7 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 08/11] jump label: x86 support Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 10/11] jump label: Remove duplicate structure for x86 Steven Rostedt
` (4 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/11] jump label: Remove duplicate structure for x86
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (8 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 09/11] jump label: Add sparc64 support Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 3:49 ` [PATCH 11/11] jump label/x86/sparc64: Remove !CC_OPTIMIZE_FOR_SIZE config conditions Steven Rostedt
` (3 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/11] jump label/x86/sparc64: Remove !CC_OPTIMIZE_FOR_SIZE config conditions
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (9 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 10/11] jump label: Remove duplicate structure for x86 Steven Rostedt
@ 2010-09-23 3:49 ` Steven Rostedt
2010-09-23 4:06 ` [GIT PULL] jump label: add jump label code Steven Rostedt
` (2 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 3:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
[-- 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [GIT PULL] jump label: add jump label code
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (10 preceding siblings ...)
2010-09-23 3:49 ` [PATCH 11/11] jump label/x86/sparc64: Remove !CC_OPTIMIZE_FOR_SIZE config conditions Steven Rostedt
@ 2010-09-23 4:06 ` Steven Rostedt
2010-09-23 6:32 ` Ingo Molnar
2010-09-23 12:42 ` Steven Rostedt
2010-09-24 9:02 ` [tip:perf/core] jump label: Fix GCC feature check when distcc is used tip-bot for Ingo Molnar
13 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 4:06 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
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
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [GIT PULL] jump label: add jump label code
2010-09-23 4:06 ` [GIT PULL] jump label: add jump label code Steven Rostedt
@ 2010-09-23 6:32 ` Ingo Molnar
0 siblings, 0 replies; 29+ messages in thread
From: Ingo Molnar @ 2010-09-23 6:32 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller,
Peter Zijlstra, Thomas Gleixner
* 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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [GIT PULL] jump label: add jump label code
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (11 preceding siblings ...)
2010-09-23 4:06 ` [GIT PULL] jump label: add jump label code Steven Rostedt
@ 2010-09-23 12:42 ` Steven Rostedt
2010-09-24 9:02 ` [tip:perf/core] jump label: Fix GCC feature check when distcc is used tip-bot for Ingo Molnar
13 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-09-23 12:42 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker,
Mathieu Desnoyers, Andi Kleen, Jason Baron, David Miller
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
^ permalink raw reply [flat|nested] 29+ messages in thread* [tip:perf/core] jump label: Fix GCC feature check when distcc is used
2010-09-23 3:49 [GIT PULL] jump label: add jump label code Steven Rostedt
` (12 preceding siblings ...)
2010-09-23 12:42 ` Steven Rostedt
@ 2010-09-24 9:02 ` tip-bot for Ingo Molnar
13 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Ingo Molnar @ 2010-09-24 9:02 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, rostedt, jbaron, tglx, mingo
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"
^ permalink raw reply related [flat|nested] 29+ messages in thread