* [PATCH 1/3] uprobes/core: Make instruction tables volatile.
@ 2012-02-22 9:15 Srikar Dronamraju
2012-02-22 9:15 ` [PATCH 2/3] uprobes/core: remove uprobe_opcode_sz Srikar Dronamraju
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Srikar Dronamraju @ 2012-02-22 9:15 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Linus Torvalds, Oleg Nesterov, LKML,
Christoph Hellwig, Steven Rostedt, Thomas Gleixner,
Masami Hiramatsu, Anton Arapov, Ananth N Mavinakayanahalli,
Jim Keniston, Jiri Olsa, Josh Stone
From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Some versions of gcc spits a warning about the asm operand for test_bit and
also causes the first long of the instruction table to be output.
Fix is similar to 7115e3fc on arch/x86/kernel/kprobes.c
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/x86/kernel/uprobes.c | 61 ++++++++++++++++++++++++---------------------
1 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index cf2a184..13d616d 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -53,34 +53,12 @@
(bc##UL << 0xc)|(bd##UL << 0xd)|(be##UL << 0xe)|(bf##UL << 0xf)) \
<< (row % 32))
-#ifdef CONFIG_X86_64
-static u32 good_insns_64[256 / 32] = {
- /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
- /* ---------------------------------------------- */
- W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
- W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
- W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
- W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
- W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
- W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
- W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
- W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
- W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
- W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
- W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
- W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
- W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
- W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
- W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
- W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
- /* ---------------------------------------------- */
- /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
-};
-#endif
-
-/* Good-instruction tables for 32-bit apps */
-
-static u32 good_insns_32[256 / 32] = {
+/*
+ * Good-instruction tables for 32-bit apps. This is non-const and volatile
+ * to keep gcc from statically optimizing it out, as variable_test_bit makes
+ * some versions of gcc to think only *(unsigned long*) is used.
+ */
+static volatile u32 good_insns_32[256 / 32] = {
/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
/* ---------------------------------------------- */
W(0x00, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0) | /* 00 */
@@ -104,7 +82,7 @@ static u32 good_insns_32[256 / 32] = {
};
/* Using this for both 64-bit and 32-bit apps */
-static u32 good_2byte_insns[256 / 32] = {
+static volatile u32 good_2byte_insns[256 / 32] = {
/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
/* ---------------------------------------------- */
W(0x00, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1) | /* 00 */
@@ -127,6 +105,31 @@ static u32 good_2byte_insns[256 / 32] = {
/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
};
+#ifdef CONFIG_X86_64
+/* Good-instruction tables for 64-bit apps */
+static volatile u32 good_insns_64[256 / 32] = {
+ /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
+ /* ---------------------------------------------- */
+ W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
+ W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
+ W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
+ W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
+ W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
+ W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
+ W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
+ W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
+ W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
+ W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
+ W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
+ W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
+ W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
+ W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
+ W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
+ W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
+ /* ---------------------------------------------- */
+ /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
+};
+#endif
#undef W
/*
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] uprobes/core: remove uprobe_opcode_sz
2012-02-22 9:15 [PATCH 1/3] uprobes/core: Make instruction tables volatile Srikar Dronamraju
@ 2012-02-22 9:15 ` Srikar Dronamraju
2012-02-22 16:05 ` [tip:perf/uprobes] uprobes/core: Remove uprobe_opcode_sz tip-bot for Srikar Dronamraju
2012-02-22 9:16 ` [PATCH 3/3] uprobes/core: move insn to arch specific structure Srikar Dronamraju
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Srikar Dronamraju @ 2012-02-22 9:15 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Linus Torvalds, Oleg Nesterov, LKML,
Christoph Hellwig, Steven Rostedt, Thomas Gleixner,
Masami Hiramatsu, Anton Arapov, Ananth N Mavinakayanahalli,
Jim Keniston, Jiri Olsa, Josh Stone
From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
uprobe_opcode_sz refers to the smallest instruction size for that
architecture. UPROBES_BKPT_INSN_SIZE refers to the size of the breakpoint
instruction for that architecture.
For now we are assuming that both uprobe_opcode_sz and
UPROBES_BKPT_INSN_SIZE are the same for all archs and hence removing
uprobe_opcode_sz in favour of UPROBES_BKPT_INSN_SIZE.
However if we have to support architectures where the smallest instruction
size is different from the size of breakpoint instruction, we may have to
re-introduce uprobe_opcode_sz.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
include/linux/uprobes.h | 2 --
kernel/uprobes.c | 6 +++---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 64e45f1..fd45b70 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -37,8 +37,6 @@ struct uprobe_arch_info {};
#define MAX_UINSN_BYTES 4
#endif
-#define uprobe_opcode_sz sizeof(uprobe_opcode_t)
-
/* flags that denote/change uprobes behaviour */
/* Have a copy of original instruction */
diff --git a/kernel/uprobes.c b/kernel/uprobes.c
index 884817f..ee496ad 100644
--- a/kernel/uprobes.c
+++ b/kernel/uprobes.c
@@ -244,8 +244,8 @@ static int write_opcode(struct mm_struct *mm, struct uprobe *uprobe,
/* poke the new insn in, ASSUMES we don't cross page boundary */
vaddr &= ~PAGE_MASK;
- BUG_ON(vaddr + uprobe_opcode_sz > PAGE_SIZE);
- memcpy(vaddr_new + vaddr, &opcode, uprobe_opcode_sz);
+ BUG_ON(vaddr + UPROBES_BKPT_INSN_SIZE > PAGE_SIZE);
+ memcpy(vaddr_new + vaddr, &opcode, UPROBES_BKPT_INSN_SIZE);
kunmap_atomic(vaddr_new);
kunmap_atomic(vaddr_old);
@@ -293,7 +293,7 @@ static int read_opcode(struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_
lock_page(page);
vaddr_new = kmap_atomic(page);
vaddr &= ~PAGE_MASK;
- memcpy(opcode, vaddr_new + vaddr, uprobe_opcode_sz);
+ memcpy(opcode, vaddr_new + vaddr, UPROBES_BKPT_INSN_SIZE);
kunmap_atomic(vaddr_new);
unlock_page(page);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] uprobes/core: move insn to arch specific structure.
2012-02-22 9:15 [PATCH 1/3] uprobes/core: Make instruction tables volatile Srikar Dronamraju
2012-02-22 9:15 ` [PATCH 2/3] uprobes/core: remove uprobe_opcode_sz Srikar Dronamraju
@ 2012-02-22 9:16 ` Srikar Dronamraju
2012-02-22 16:06 ` [tip:perf/uprobes] uprobes/core: Move " tip-bot for Srikar Dronamraju
2012-02-22 9:47 ` [PATCH 1/3] uprobes/core: Make instruction tables volatile Ingo Molnar
2012-02-22 16:04 ` [tip:perf/uprobes] " tip-bot for Srikar Dronamraju
3 siblings, 1 reply; 8+ messages in thread
From: Srikar Dronamraju @ 2012-02-22 9:16 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Linus Torvalds, Oleg Nesterov, LKML,
Christoph Hellwig, Steven Rostedt, Thomas Gleixner,
Masami Hiramatsu, Anton Arapov, Ananth N Mavinakayanahalli,
Jim Keniston, Jiri Olsa, Josh Stone
From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Few cleanups suggested by Ingo Molnar.
- Rename struct uprobe_arch_info to struct arch_uprobe.
- Move insn from struct uprobe to struct arch_uprobe.
- Make arch specific uprobe functions to accept struct arch_uprobe instead of
struct uprobe.
- Move struct uprobe to kernel/uprobes.c from include/linux/uprobes.h
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/x86/include/asm/uprobes.h | 6 ++--
arch/x86/kernel/uprobes.c | 60 ++++++++++++++++++++--------------------
include/linux/uprobes.h | 23 +--------------
kernel/uprobes.c | 38 +++++++++++++++++--------
4 files changed, 61 insertions(+), 66 deletions(-)
diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
index 072df39..f7ce310 100644
--- a/arch/x86/include/asm/uprobes.h
+++ b/arch/x86/include/asm/uprobes.h
@@ -31,13 +31,13 @@ typedef u8 uprobe_opcode_t;
#define UPROBES_BKPT_INSN 0xcc
#define UPROBES_BKPT_INSN_SIZE 1
-struct uprobe_arch_info {
+struct arch_uprobe {
u16 fixups;
+ u8 insn[MAX_UINSN_BYTES];
#ifdef CONFIG_X86_64
unsigned long rip_rela_target_address;
#endif
};
-struct uprobe;
-extern int arch_uprobes_analyze_insn(struct mm_struct *mm, struct uprobe *uprobe);
+extern int arch_uprobes_analyze_insn(struct mm_struct *mm, struct arch_uprobe *arch_uprobe);
#endif /* _ASM_UPROBES_H */
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 13d616d..04dfcef 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -200,9 +200,9 @@ static bool is_prefix_bad(struct insn *insn)
return false;
}
-static int validate_insn_32bits(struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_32bits(struct arch_uprobe *auprobe, struct insn *insn)
{
- insn_init(insn, uprobe->insn, false);
+ insn_init(insn, auprobe->insn, false);
/* Skip good instruction prefixes; reject "bad" ones. */
insn_get_opcode(insn);
@@ -222,11 +222,11 @@ static int validate_insn_32bits(struct uprobe *uprobe, struct insn *insn)
/*
* Figure out which fixups post_xol() will need to perform, and annotate
- * uprobe->arch_info.fixups accordingly. To start with,
- * uprobe->arch_info.fixups is either zero or it reflects rip-related
+ * arch_uprobe->fixups accordingly. To start with,
+ * arch_uprobe->fixups is either zero or it reflects rip-related
* fixups.
*/
-static void prepare_fixups(struct uprobe *uprobe, struct insn *insn)
+static void prepare_fixups(struct arch_uprobe *auprobe, struct insn *insn)
{
bool fix_ip = true, fix_call = false; /* defaults */
int reg;
@@ -269,17 +269,17 @@ static void prepare_fixups(struct uprobe *uprobe, struct insn *insn)
break;
}
if (fix_ip)
- uprobe->arch_info.fixups |= UPROBES_FIX_IP;
+ auprobe->fixups |= UPROBES_FIX_IP;
if (fix_call)
- uprobe->arch_info.fixups |= UPROBES_FIX_CALL;
+ auprobe->fixups |= UPROBES_FIX_CALL;
}
#ifdef CONFIG_X86_64
/*
- * If uprobe->insn doesn't use rip-relative addressing, return
+ * If arch_uprobe->insn doesn't use rip-relative addressing, return
* immediately. Otherwise, rewrite the instruction so that it accesses
* its memory operand indirectly through a scratch register. Set
- * uprobe->arch_info.fixups and uprobe->arch_info.rip_rela_target_address
+ * arch_uprobe->fixups and arch_uprobe->rip_rela_target_address
* accordingly. (The contents of the scratch register will be saved
* before we single-step the modified instruction, and restored
* afterward.)
@@ -297,7 +297,7 @@ static void prepare_fixups(struct uprobe *uprobe, struct insn *insn)
* - There's never a SIB byte.
* - The displacement is always 4 bytes.
*/
-static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static void handle_riprel_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
u8 *cursor;
u8 reg;
@@ -305,7 +305,7 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
if (mm->context.ia32_compat)
return;
- uprobe->arch_info.rip_rela_target_address = 0x0;
+ auprobe->rip_rela_target_address = 0x0;
if (!insn_rip_relative(insn))
return;
@@ -315,7 +315,7 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
* we want to encode rax/rcx, not r8/r9.
*/
if (insn->rex_prefix.nbytes) {
- cursor = uprobe->insn + insn_offset_rex_prefix(insn);
+ cursor = auprobe->insn + insn_offset_rex_prefix(insn);
*cursor &= 0xfe; /* Clearing REX.B bit */
}
@@ -324,7 +324,7 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
* displacement. Beyond the displacement, for some instructions,
* is the immediate operand.
*/
- cursor = uprobe->insn + insn_offset_modrm(insn);
+ cursor = auprobe->insn + insn_offset_modrm(insn);
insn_get_length(insn);
/*
@@ -341,18 +341,18 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
* is NOT the register operand, so we use %rcx (register
* #1) for the scratch register.
*/
- uprobe->arch_info.fixups = UPROBES_FIX_RIP_CX;
+ auprobe->fixups = UPROBES_FIX_RIP_CX;
/* Change modrm from 00 000 101 to 00 000 001. */
*cursor = 0x1;
} else {
/* Use %rax (register #0) for the scratch register. */
- uprobe->arch_info.fixups = UPROBES_FIX_RIP_AX;
+ auprobe->fixups = UPROBES_FIX_RIP_AX;
/* Change modrm from 00 xxx 101 to 00 xxx 000 */
*cursor = (reg << 3);
}
/* Target address = address of next instruction + (signed) offset */
- uprobe->arch_info.rip_rela_target_address = (long)insn->length + insn->displacement.value;
+ auprobe->rip_rela_target_address = (long)insn->length + insn->displacement.value;
/* Displacement field is gone; slide immediate field (if any) over. */
if (insn->immediate.nbytes) {
@@ -362,9 +362,9 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
return;
}
-static int validate_insn_64bits(struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_64bits(struct arch_uprobe *auprobe, struct insn *insn)
{
- insn_init(insn, uprobe->insn, true);
+ insn_init(insn, auprobe->insn, true);
/* Skip good instruction prefixes; reject "bad" ones. */
insn_get_opcode(insn);
@@ -381,42 +381,42 @@ static int validate_insn_64bits(struct uprobe *uprobe, struct insn *insn)
return -ENOTSUPP;
}
-static int validate_insn_bits(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_bits(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
if (mm->context.ia32_compat)
- return validate_insn_32bits(uprobe, insn);
- return validate_insn_64bits(uprobe, insn);
+ return validate_insn_32bits(auprobe, insn);
+ return validate_insn_64bits(auprobe, insn);
}
#else /* 32-bit: */
-static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static void handle_riprel_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
/* No RIP-relative addressing on 32-bit */
}
-static int validate_insn_bits(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_bits(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
- return validate_insn_32bits(uprobe, insn);
+ return validate_insn_32bits(auprobe, insn);
}
#endif /* CONFIG_X86_64 */
/**
* arch_uprobes_analyze_insn - instruction analysis including validity and fixups.
* @mm: the probed address space.
- * @uprobe: the probepoint information.
+ * @arch_uprobe: the probepoint information.
* Return 0 on success or a -ve number on error.
*/
-int arch_uprobes_analyze_insn(struct mm_struct *mm, struct uprobe *uprobe)
+int arch_uprobes_analyze_insn(struct mm_struct *mm, struct arch_uprobe *auprobe)
{
int ret;
struct insn insn;
- uprobe->arch_info.fixups = 0;
- ret = validate_insn_bits(mm, uprobe, &insn);
+ auprobe->fixups = 0;
+ ret = validate_insn_bits(mm, auprobe, &insn);
if (ret != 0)
return ret;
- handle_riprel_insn(mm, uprobe, &insn);
- prepare_fixups(uprobe, &insn);
+ handle_riprel_insn(mm, auprobe, &insn);
+ prepare_fixups(auprobe, &insn);
return 0;
}
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index fd45b70..9c6be62 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -29,12 +29,6 @@
struct vm_area_struct;
#ifdef CONFIG_ARCH_SUPPORTS_UPROBES
#include <asm/uprobes.h>
-#else
-
-typedef u8 uprobe_opcode_t;
-struct uprobe_arch_info {};
-
-#define MAX_UINSN_BYTES 4
#endif
/* flags that denote/change uprobes behaviour */
@@ -56,22 +50,9 @@ struct uprobe_consumer {
struct uprobe_consumer *next;
};
-struct uprobe {
- struct rb_node rb_node; /* node in the rb tree */
- atomic_t ref;
- struct rw_semaphore consumer_rwsem;
- struct list_head pending_list;
- struct uprobe_arch_info arch_info;
- struct uprobe_consumer *consumers;
- struct inode *inode; /* Also hold a ref to inode */
- loff_t offset;
- int flags;
- u8 insn[MAX_UINSN_BYTES];
-};
-
#ifdef CONFIG_UPROBES
-extern int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr);
-extern int __weak set_orig_insn(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr, bool verify);
+extern int __weak set_bkpt(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr);
+extern int __weak set_orig_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr, bool verify);
extern bool __weak is_bkpt_insn(uprobe_opcode_t *insn);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *consumer);
extern void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *consumer);
diff --git a/kernel/uprobes.c b/kernel/uprobes.c
index ee496ad..0867302 100644
--- a/kernel/uprobes.c
+++ b/kernel/uprobes.c
@@ -65,6 +65,18 @@ struct vma_info {
loff_t vaddr;
};
+struct uprobe {
+ struct rb_node rb_node; /* node in the rb tree */
+ atomic_t ref;
+ struct rw_semaphore consumer_rwsem;
+ struct list_head pending_list;
+ struct uprobe_consumer *consumers;
+ struct inode *inode; /* Also hold a ref to inode */
+ loff_t offset;
+ int flags;
+ struct arch_uprobe arch;
+};
+
/*
* valid_vma: Verify if the specified vma is an executable vma
* Relax restrictions while unregistering: vm_flags might have
@@ -180,7 +192,7 @@ bool __weak is_bkpt_insn(uprobe_opcode_t *insn)
/*
* write_opcode - write the opcode at a given virtual address.
* @mm: the probed process address space.
- * @uprobe: the breakpointing information.
+ * @arch_uprobe: the breakpointing information.
* @vaddr: the virtual address to store the opcode.
* @opcode: opcode to be written at @vaddr.
*
@@ -190,13 +202,14 @@ bool __weak is_bkpt_insn(uprobe_opcode_t *insn)
* For mm @mm, write the opcode at @vaddr.
* Return 0 (success) or a negative errno.
*/
-static int write_opcode(struct mm_struct *mm, struct uprobe *uprobe,
+static int write_opcode(struct mm_struct *mm, struct arch_uprobe *auprobe,
unsigned long vaddr, uprobe_opcode_t opcode)
{
struct page *old_page, *new_page;
struct address_space *mapping;
void *vaddr_old, *vaddr_new;
struct vm_area_struct *vma;
+ struct uprobe *uprobe;
loff_t addr;
int ret;
@@ -216,6 +229,7 @@ static int write_opcode(struct mm_struct *mm, struct uprobe *uprobe,
if (!valid_vma(vma, is_bkpt_insn(&opcode)))
goto put_out;
+ uprobe = container_of(auprobe, struct uprobe, arch);
mapping = uprobe->inode->i_mapping;
if (mapping != vma->vm_file->f_mapping)
goto put_out;
@@ -326,7 +340,7 @@ static int is_bkpt_at_addr(struct mm_struct *mm, unsigned long vaddr)
* For mm @mm, store the breakpoint instruction at @vaddr.
* Return 0 (success) or a negative errno.
*/
-int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr)
+int __weak set_bkpt(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr)
{
int result;
@@ -337,7 +351,7 @@ int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long v
if (result)
return result;
- return write_opcode(mm, uprobe, vaddr, UPROBES_BKPT_INSN);
+ return write_opcode(mm, auprobe, vaddr, UPROBES_BKPT_INSN);
}
/**
@@ -351,7 +365,7 @@ int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long v
* Return 0 (success) or a negative errno.
*/
int __weak
-set_orig_insn(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr, bool verify)
+set_orig_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr, bool verify)
{
if (verify) {
int result;
@@ -363,7 +377,7 @@ set_orig_insn(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr,
if (result != 1)
return result;
}
- return write_opcode(mm, uprobe, vaddr, *(uprobe_opcode_t *)uprobe->insn);
+ return write_opcode(mm, auprobe, vaddr, *(uprobe_opcode_t *)auprobe->insn);
}
static int match_uprobe(struct uprobe *l, struct uprobe *r)
@@ -593,13 +607,13 @@ static int copy_insn(struct uprobe *uprobe, struct vm_area_struct *vma, unsigned
/* Instruction at the page-boundary; copy bytes in second page */
if (nbytes < bytes) {
- if (__copy_insn(mapping, vma, uprobe->insn + nbytes,
+ if (__copy_insn(mapping, vma, uprobe->arch.insn + nbytes,
bytes - nbytes, uprobe->offset + nbytes))
return -ENOMEM;
bytes = nbytes;
}
- return __copy_insn(mapping, vma, uprobe->insn, bytes, uprobe->offset);
+ return __copy_insn(mapping, vma, uprobe->arch.insn, bytes, uprobe->offset);
}
static int install_breakpoint(struct mm_struct *mm, struct uprobe *uprobe,
@@ -625,23 +639,23 @@ static int install_breakpoint(struct mm_struct *mm, struct uprobe *uprobe,
if (ret)
return ret;
- if (is_bkpt_insn((uprobe_opcode_t *)uprobe->insn))
+ if (is_bkpt_insn((uprobe_opcode_t *)uprobe->arch.insn))
return -EEXIST;
- ret = arch_uprobes_analyze_insn(mm, uprobe);
+ ret = arch_uprobes_analyze_insn(mm, &uprobe->arch);
if (ret)
return ret;
uprobe->flags |= UPROBES_COPY_INSN;
}
- ret = set_bkpt(mm, uprobe, addr);
+ ret = set_bkpt(mm, &uprobe->arch, addr);
return ret;
}
static void remove_breakpoint(struct mm_struct *mm, struct uprobe *uprobe, loff_t vaddr)
{
- set_orig_insn(mm, uprobe, (unsigned long)vaddr, true);
+ set_orig_insn(mm, &uprobe->arch, (unsigned long)vaddr, true);
}
static void delete_uprobe(struct uprobe *uprobe)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] uprobes/core: Make instruction tables volatile.
2012-02-22 9:15 [PATCH 1/3] uprobes/core: Make instruction tables volatile Srikar Dronamraju
2012-02-22 9:15 ` [PATCH 2/3] uprobes/core: remove uprobe_opcode_sz Srikar Dronamraju
2012-02-22 9:16 ` [PATCH 3/3] uprobes/core: move insn to arch specific structure Srikar Dronamraju
@ 2012-02-22 9:47 ` Ingo Molnar
2012-02-22 10:04 ` Srikar Dronamraju
2012-02-22 16:04 ` [tip:perf/uprobes] " tip-bot for Srikar Dronamraju
3 siblings, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2012-02-22 9:47 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Peter Zijlstra, Linus Torvalds, Oleg Nesterov, LKML,
Christoph Hellwig, Steven Rostedt, Thomas Gleixner,
Masami Hiramatsu, Anton Arapov, Ananth N Mavinakayanahalli,
Jim Keniston, Jiri Olsa, Josh Stone
* Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>
> Some versions of gcc spits a warning about the asm operand for test_bit and
> also causes the first long of the instruction table to be output.
>
> Fix is similar to 7115e3fc on arch/x86/kernel/kprobes.c
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/uprobes.c | 61 ++++++++++++++++++++++++---------------------
> 1 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index cf2a184..13d616d 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
> @@ -53,34 +53,12 @@
> (bc##UL << 0xc)|(bd##UL << 0xd)|(be##UL << 0xe)|(bf##UL << 0xf)) \
> << (row % 32))
>
> -#ifdef CONFIG_X86_64
> -static u32 good_insns_64[256 / 32] = {
> - /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> - /* ---------------------------------------------- */
> - W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
> - W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
> - W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
> - W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
> - W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
> - W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
> - W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
> - W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
> - W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
> - W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
> - W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
> - W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
> - W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
> - W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
> - W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
> - W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
> - /* ---------------------------------------------- */
> - /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> -};
> -#endif
> -
> -/* Good-instruction tables for 32-bit apps */
> -
> -static u32 good_insns_32[256 / 32] = {
> +/*
> + * Good-instruction tables for 32-bit apps. This is non-const and volatile
> + * to keep gcc from statically optimizing it out, as variable_test_bit makes
> + * some versions of gcc to think only *(unsigned long*) is used.
> + */
> +static volatile u32 good_insns_32[256 / 32] = {
> /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> /* ---------------------------------------------- */
> W(0x00, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0) | /* 00 */
> @@ -104,7 +82,7 @@ static u32 good_insns_32[256 / 32] = {
> };
>
> /* Using this for both 64-bit and 32-bit apps */
> -static u32 good_2byte_insns[256 / 32] = {
> +static volatile u32 good_2byte_insns[256 / 32] = {
> /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> /* ---------------------------------------------- */
> W(0x00, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1) | /* 00 */
> @@ -127,6 +105,31 @@ static u32 good_2byte_insns[256 / 32] = {
> /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> };
>
> +#ifdef CONFIG_X86_64
> +/* Good-instruction tables for 64-bit apps */
> +static volatile u32 good_insns_64[256 / 32] = {
> + /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> + /* ---------------------------------------------- */
> + W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
> + W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
> + W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
> + W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
> + W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
> + W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
> + W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
> + W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
> + W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
> + W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
> + W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
> + W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
> + W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
> + W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
> + W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
> + W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
> + /* ---------------------------------------------- */
> + /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> +};
> +#endif
Hm, why was the table moved around? This makes it hard to see
whether it's just a single volatile that is added, or something
more.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] uprobes/core: Make instruction tables volatile.
2012-02-22 9:47 ` [PATCH 1/3] uprobes/core: Make instruction tables volatile Ingo Molnar
@ 2012-02-22 10:04 ` Srikar Dronamraju
0 siblings, 0 replies; 8+ messages in thread
From: Srikar Dronamraju @ 2012-02-22 10:04 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Linus Torvalds, Oleg Nesterov, LKML,
Christoph Hellwig, Steven Rostedt, Thomas Gleixner,
Masami Hiramatsu, Anton Arapov, Ananth N Mavinakayanahalli,
Jim Keniston, Jiri Olsa, Josh Stone
> * Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
>
> > From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> >
> > Some versions of gcc spits a warning about the asm operand for test_bit and
> > also causes the first long of the instruction table to be output.
> >
> > Fix is similar to 7115e3fc on arch/x86/kernel/kprobes.c
> >
> > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> > arch/x86/kernel/uprobes.c | 61 ++++++++++++++++++++++++---------------------
> > 1 files changed, 32 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> > index cf2a184..13d616d 100644
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -53,34 +53,12 @@
> > (bc##UL << 0xc)|(bd##UL << 0xd)|(be##UL << 0xe)|(bf##UL << 0xf)) \
> > << (row % 32))
> >
> > -#ifdef CONFIG_X86_64
> > -static u32 good_insns_64[256 / 32] = {
> > - /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > - /* ---------------------------------------------- */
> > - W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
> > - W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
> > - W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
> > - W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
> > - W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
> > - W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
> > - W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
> > - W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
> > - W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
> > - W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
> > - W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
> > - W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
> > - W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
> > - W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
> > - W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
> > - W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
> > - /* ---------------------------------------------- */
> > - /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > -};
> > -#endif
> > -
> > -/* Good-instruction tables for 32-bit apps */
> > -
> > -static u32 good_insns_32[256 / 32] = {
> > +/*
> > + * Good-instruction tables for 32-bit apps. This is non-const and volatile
> > + * to keep gcc from statically optimizing it out, as variable_test_bit makes
> > + * some versions of gcc to think only *(unsigned long*) is used.
> > + */
> > +static volatile u32 good_insns_32[256 / 32] = {
> > /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > /* ---------------------------------------------- */
> > W(0x00, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0) | /* 00 */
> > @@ -104,7 +82,7 @@ static u32 good_insns_32[256 / 32] = {
> > };
> >
> > /* Using this for both 64-bit and 32-bit apps */
> > -static u32 good_2byte_insns[256 / 32] = {
> > +static volatile u32 good_2byte_insns[256 / 32] = {
> > /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > /* ---------------------------------------------- */
> > W(0x00, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1) | /* 00 */
> > @@ -127,6 +105,31 @@ static u32 good_2byte_insns[256 / 32] = {
> > /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > };
> >
> > +#ifdef CONFIG_X86_64
> > +/* Good-instruction tables for 64-bit apps */
> > +static volatile u32 good_insns_64[256 / 32] = {
> > + /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > + /* ---------------------------------------------- */
> > + W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
> > + W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
> > + W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
> > + W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
> > + W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
> > + W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
> > + W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
> > + W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
> > + W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
> > + W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
> > + W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
> > + W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
> > + W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
> > + W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
> > + W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
> > + W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
> > + /* ---------------------------------------------- */
> > + /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
> > +};
> > +#endif
>
> Hm, why was the table moved around? This makes it hard to see
> whether it's just a single volatile that is added, or something
> more.
>
Without the table moving, the volatile was first used under #ifdef
CONFIG_X86_64. I was wondering if I add a comment in #ifdef people
looking at it might assume that its a problem only on x86_64 only.
So I moved the CONFIG_X86_64 specific table down and added the comment
for volatile at the first instance its used.
--
Thanks and Regards
Srikar
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/uprobes] uprobes/core: Make instruction tables volatile
2012-02-22 9:15 [PATCH 1/3] uprobes/core: Make instruction tables volatile Srikar Dronamraju
` (2 preceding siblings ...)
2012-02-22 9:47 ` [PATCH 1/3] uprobes/core: Make instruction tables volatile Ingo Molnar
@ 2012-02-22 16:04 ` tip-bot for Srikar Dronamraju
3 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Srikar Dronamraju @ 2012-02-22 16:04 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, torvalds, peterz, jolsa, anton, jkenisto, rostedt, tglx,
oleg, hpa, linux-kernel, hch, ananth, jistone,
masami.hiramatsu.pt, srikar, mingo
Commit-ID: 04a3d984d32e47983770d314cdb4e4d8f38fccb7
Gitweb: http://git.kernel.org/tip/04a3d984d32e47983770d314cdb4e4d8f38fccb7
Author: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
AuthorDate: Wed, 22 Feb 2012 14:45:35 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 22 Feb 2012 11:26:07 +0100
uprobes/core: Make instruction tables volatile
Some versions of gcc spits a warning about the asm operand for
test_bit and also causes the first long of the instruction table
to be output.
Fix is similar to 7115e3fc on arch/x86/kernel/kprobes.c
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Anton Arapov <anton@redhat.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Stone <jistone@redhat.com>
Link: http://lkml.kernel.org/r/20120222091535.15880.12502.sendpatchset@srdronam.in.ibm.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/uprobes.c | 61 +++++++++++++++++++++++---------------------
1 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index cf2a184..13d616d 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -53,34 +53,12 @@
(bc##UL << 0xc)|(bd##UL << 0xd)|(be##UL << 0xe)|(bf##UL << 0xf)) \
<< (row % 32))
-#ifdef CONFIG_X86_64
-static u32 good_insns_64[256 / 32] = {
- /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
- /* ---------------------------------------------- */
- W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
- W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
- W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
- W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
- W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
- W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
- W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
- W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
- W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
- W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
- W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
- W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
- W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
- W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
- W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
- W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
- /* ---------------------------------------------- */
- /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
-};
-#endif
-
-/* Good-instruction tables for 32-bit apps */
-
-static u32 good_insns_32[256 / 32] = {
+/*
+ * Good-instruction tables for 32-bit apps. This is non-const and volatile
+ * to keep gcc from statically optimizing it out, as variable_test_bit makes
+ * some versions of gcc to think only *(unsigned long*) is used.
+ */
+static volatile u32 good_insns_32[256 / 32] = {
/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
/* ---------------------------------------------- */
W(0x00, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0) | /* 00 */
@@ -104,7 +82,7 @@ static u32 good_insns_32[256 / 32] = {
};
/* Using this for both 64-bit and 32-bit apps */
-static u32 good_2byte_insns[256 / 32] = {
+static volatile u32 good_2byte_insns[256 / 32] = {
/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
/* ---------------------------------------------- */
W(0x00, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1) | /* 00 */
@@ -127,6 +105,31 @@ static u32 good_2byte_insns[256 / 32] = {
/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
};
+#ifdef CONFIG_X86_64
+/* Good-instruction tables for 64-bit apps */
+static volatile u32 good_insns_64[256 / 32] = {
+ /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
+ /* ---------------------------------------------- */
+ W(0x00, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 00 */
+ W(0x10, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 10 */
+ W(0x20, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) | /* 20 */
+ W(0x30, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0) , /* 30 */
+ W(0x40, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) | /* 40 */
+ W(0x50, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 50 */
+ W(0x60, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* 60 */
+ W(0x70, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 70 */
+ W(0x80, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* 80 */
+ W(0x90, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* 90 */
+ W(0xa0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) | /* a0 */
+ W(0xb0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* b0 */
+ W(0xc0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0) | /* c0 */
+ W(0xd0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1) , /* d0 */
+ W(0xe0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0) | /* e0 */
+ W(0xf0, 0, 0, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1) /* f0 */
+ /* ---------------------------------------------- */
+ /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */
+};
+#endif
#undef W
/*
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip:perf/uprobes] uprobes/core: Remove uprobe_opcode_sz
2012-02-22 9:15 ` [PATCH 2/3] uprobes/core: remove uprobe_opcode_sz Srikar Dronamraju
@ 2012-02-22 16:05 ` tip-bot for Srikar Dronamraju
0 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Srikar Dronamraju @ 2012-02-22 16:05 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, torvalds, peterz, jolsa, anton, jkenisto, rostedt, tglx,
oleg, hpa, linux-kernel, hch, ananth, jistone,
masami.hiramatsu.pt, srikar, mingo
Commit-ID: 96379f60075c75b261328aa7830ef8aa158247ac
Gitweb: http://git.kernel.org/tip/96379f60075c75b261328aa7830ef8aa158247ac
Author: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
AuthorDate: Wed, 22 Feb 2012 14:45:49 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 22 Feb 2012 11:26:08 +0100
uprobes/core: Remove uprobe_opcode_sz
uprobe_opcode_sz refers to the smallest instruction size for
that architecture. UPROBES_BKPT_INSN_SIZE refers to the size of
the breakpoint instruction for that architecture.
For now we are assuming that both uprobe_opcode_sz and
UPROBES_BKPT_INSN_SIZE are the same for all archs and hence
removing uprobe_opcode_sz in favour of UPROBES_BKPT_INSN_SIZE.
However if we have to support architectures where the smallest
instruction size is different from the size of breakpoint
instruction, we may have to re-introduce uprobe_opcode_sz.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Anton Arapov <anton@redhat.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Stone <jistone@redhat.com>
Link: http://lkml.kernel.org/r/20120222091549.15880.67020.sendpatchset@srdronam.in.ibm.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
include/linux/uprobes.h | 2 --
kernel/events/uprobes.c | 6 +++---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 64e45f1..fd45b70 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -37,8 +37,6 @@ struct uprobe_arch_info {};
#define MAX_UINSN_BYTES 4
#endif
-#define uprobe_opcode_sz sizeof(uprobe_opcode_t)
-
/* flags that denote/change uprobes behaviour */
/* Have a copy of original instruction */
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 884817f..ee496ad 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -244,8 +244,8 @@ static int write_opcode(struct mm_struct *mm, struct uprobe *uprobe,
/* poke the new insn in, ASSUMES we don't cross page boundary */
vaddr &= ~PAGE_MASK;
- BUG_ON(vaddr + uprobe_opcode_sz > PAGE_SIZE);
- memcpy(vaddr_new + vaddr, &opcode, uprobe_opcode_sz);
+ BUG_ON(vaddr + UPROBES_BKPT_INSN_SIZE > PAGE_SIZE);
+ memcpy(vaddr_new + vaddr, &opcode, UPROBES_BKPT_INSN_SIZE);
kunmap_atomic(vaddr_new);
kunmap_atomic(vaddr_old);
@@ -293,7 +293,7 @@ static int read_opcode(struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_
lock_page(page);
vaddr_new = kmap_atomic(page);
vaddr &= ~PAGE_MASK;
- memcpy(opcode, vaddr_new + vaddr, uprobe_opcode_sz);
+ memcpy(opcode, vaddr_new + vaddr, UPROBES_BKPT_INSN_SIZE);
kunmap_atomic(vaddr_new);
unlock_page(page);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip:perf/uprobes] uprobes/core: Move insn to arch specific structure
2012-02-22 9:16 ` [PATCH 3/3] uprobes/core: move insn to arch specific structure Srikar Dronamraju
@ 2012-02-22 16:06 ` tip-bot for Srikar Dronamraju
0 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Srikar Dronamraju @ 2012-02-22 16:06 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, torvalds, peterz, jolsa, anton, jkenisto, rostedt, tglx,
oleg, hpa, linux-kernel, hch, ananth, jistone,
masami.hiramatsu.pt, srikar, mingo
Commit-ID: 3ff54efdfaace9e9b2b7c1959a865be6b91de96c
Gitweb: http://git.kernel.org/tip/3ff54efdfaace9e9b2b7c1959a865be6b91de96c
Author: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
AuthorDate: Wed, 22 Feb 2012 14:46:02 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 22 Feb 2012 11:26:09 +0100
uprobes/core: Move insn to arch specific structure
Few cleanups suggested by Ingo Molnar.
- Rename struct uprobe_arch_info to struct arch_uprobe.
- Move insn from struct uprobe to struct arch_uprobe.
- Make arch specific uprobe functions to accept struct arch_uprobe
instead of struct uprobe.
- Move struct uprobe to kernel/uprobes.c from include/linux/uprobes.h
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Anton Arapov <anton@redhat.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Stone <jistone@redhat.com>
Link: http://lkml.kernel.org/r/20120222091602.15880.40249.sendpatchset@srdronam.in.ibm.com
[ Made various small improvements ]
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/include/asm/uprobes.h | 6 ++--
arch/x86/kernel/uprobes.c | 60 ++++++++++++++++++++--------------------
include/linux/uprobes.h | 23 +--------------
kernel/events/uprobes.c | 38 +++++++++++++++++--------
4 files changed, 61 insertions(+), 66 deletions(-)
diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
index 072df39..f7ce310 100644
--- a/arch/x86/include/asm/uprobes.h
+++ b/arch/x86/include/asm/uprobes.h
@@ -31,13 +31,13 @@ typedef u8 uprobe_opcode_t;
#define UPROBES_BKPT_INSN 0xcc
#define UPROBES_BKPT_INSN_SIZE 1
-struct uprobe_arch_info {
+struct arch_uprobe {
u16 fixups;
+ u8 insn[MAX_UINSN_BYTES];
#ifdef CONFIG_X86_64
unsigned long rip_rela_target_address;
#endif
};
-struct uprobe;
-extern int arch_uprobes_analyze_insn(struct mm_struct *mm, struct uprobe *uprobe);
+extern int arch_uprobes_analyze_insn(struct mm_struct *mm, struct arch_uprobe *arch_uprobe);
#endif /* _ASM_UPROBES_H */
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 13d616d..04dfcef 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -200,9 +200,9 @@ static bool is_prefix_bad(struct insn *insn)
return false;
}
-static int validate_insn_32bits(struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_32bits(struct arch_uprobe *auprobe, struct insn *insn)
{
- insn_init(insn, uprobe->insn, false);
+ insn_init(insn, auprobe->insn, false);
/* Skip good instruction prefixes; reject "bad" ones. */
insn_get_opcode(insn);
@@ -222,11 +222,11 @@ static int validate_insn_32bits(struct uprobe *uprobe, struct insn *insn)
/*
* Figure out which fixups post_xol() will need to perform, and annotate
- * uprobe->arch_info.fixups accordingly. To start with,
- * uprobe->arch_info.fixups is either zero or it reflects rip-related
+ * arch_uprobe->fixups accordingly. To start with,
+ * arch_uprobe->fixups is either zero or it reflects rip-related
* fixups.
*/
-static void prepare_fixups(struct uprobe *uprobe, struct insn *insn)
+static void prepare_fixups(struct arch_uprobe *auprobe, struct insn *insn)
{
bool fix_ip = true, fix_call = false; /* defaults */
int reg;
@@ -269,17 +269,17 @@ static void prepare_fixups(struct uprobe *uprobe, struct insn *insn)
break;
}
if (fix_ip)
- uprobe->arch_info.fixups |= UPROBES_FIX_IP;
+ auprobe->fixups |= UPROBES_FIX_IP;
if (fix_call)
- uprobe->arch_info.fixups |= UPROBES_FIX_CALL;
+ auprobe->fixups |= UPROBES_FIX_CALL;
}
#ifdef CONFIG_X86_64
/*
- * If uprobe->insn doesn't use rip-relative addressing, return
+ * If arch_uprobe->insn doesn't use rip-relative addressing, return
* immediately. Otherwise, rewrite the instruction so that it accesses
* its memory operand indirectly through a scratch register. Set
- * uprobe->arch_info.fixups and uprobe->arch_info.rip_rela_target_address
+ * arch_uprobe->fixups and arch_uprobe->rip_rela_target_address
* accordingly. (The contents of the scratch register will be saved
* before we single-step the modified instruction, and restored
* afterward.)
@@ -297,7 +297,7 @@ static void prepare_fixups(struct uprobe *uprobe, struct insn *insn)
* - There's never a SIB byte.
* - The displacement is always 4 bytes.
*/
-static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static void handle_riprel_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
u8 *cursor;
u8 reg;
@@ -305,7 +305,7 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
if (mm->context.ia32_compat)
return;
- uprobe->arch_info.rip_rela_target_address = 0x0;
+ auprobe->rip_rela_target_address = 0x0;
if (!insn_rip_relative(insn))
return;
@@ -315,7 +315,7 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
* we want to encode rax/rcx, not r8/r9.
*/
if (insn->rex_prefix.nbytes) {
- cursor = uprobe->insn + insn_offset_rex_prefix(insn);
+ cursor = auprobe->insn + insn_offset_rex_prefix(insn);
*cursor &= 0xfe; /* Clearing REX.B bit */
}
@@ -324,7 +324,7 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
* displacement. Beyond the displacement, for some instructions,
* is the immediate operand.
*/
- cursor = uprobe->insn + insn_offset_modrm(insn);
+ cursor = auprobe->insn + insn_offset_modrm(insn);
insn_get_length(insn);
/*
@@ -341,18 +341,18 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
* is NOT the register operand, so we use %rcx (register
* #1) for the scratch register.
*/
- uprobe->arch_info.fixups = UPROBES_FIX_RIP_CX;
+ auprobe->fixups = UPROBES_FIX_RIP_CX;
/* Change modrm from 00 000 101 to 00 000 001. */
*cursor = 0x1;
} else {
/* Use %rax (register #0) for the scratch register. */
- uprobe->arch_info.fixups = UPROBES_FIX_RIP_AX;
+ auprobe->fixups = UPROBES_FIX_RIP_AX;
/* Change modrm from 00 xxx 101 to 00 xxx 000 */
*cursor = (reg << 3);
}
/* Target address = address of next instruction + (signed) offset */
- uprobe->arch_info.rip_rela_target_address = (long)insn->length + insn->displacement.value;
+ auprobe->rip_rela_target_address = (long)insn->length + insn->displacement.value;
/* Displacement field is gone; slide immediate field (if any) over. */
if (insn->immediate.nbytes) {
@@ -362,9 +362,9 @@ static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, stru
return;
}
-static int validate_insn_64bits(struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_64bits(struct arch_uprobe *auprobe, struct insn *insn)
{
- insn_init(insn, uprobe->insn, true);
+ insn_init(insn, auprobe->insn, true);
/* Skip good instruction prefixes; reject "bad" ones. */
insn_get_opcode(insn);
@@ -381,42 +381,42 @@ static int validate_insn_64bits(struct uprobe *uprobe, struct insn *insn)
return -ENOTSUPP;
}
-static int validate_insn_bits(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_bits(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
if (mm->context.ia32_compat)
- return validate_insn_32bits(uprobe, insn);
- return validate_insn_64bits(uprobe, insn);
+ return validate_insn_32bits(auprobe, insn);
+ return validate_insn_64bits(auprobe, insn);
}
#else /* 32-bit: */
-static void handle_riprel_insn(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static void handle_riprel_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
/* No RIP-relative addressing on 32-bit */
}
-static int validate_insn_bits(struct mm_struct *mm, struct uprobe *uprobe, struct insn *insn)
+static int validate_insn_bits(struct mm_struct *mm, struct arch_uprobe *auprobe, struct insn *insn)
{
- return validate_insn_32bits(uprobe, insn);
+ return validate_insn_32bits(auprobe, insn);
}
#endif /* CONFIG_X86_64 */
/**
* arch_uprobes_analyze_insn - instruction analysis including validity and fixups.
* @mm: the probed address space.
- * @uprobe: the probepoint information.
+ * @arch_uprobe: the probepoint information.
* Return 0 on success or a -ve number on error.
*/
-int arch_uprobes_analyze_insn(struct mm_struct *mm, struct uprobe *uprobe)
+int arch_uprobes_analyze_insn(struct mm_struct *mm, struct arch_uprobe *auprobe)
{
int ret;
struct insn insn;
- uprobe->arch_info.fixups = 0;
- ret = validate_insn_bits(mm, uprobe, &insn);
+ auprobe->fixups = 0;
+ ret = validate_insn_bits(mm, auprobe, &insn);
if (ret != 0)
return ret;
- handle_riprel_insn(mm, uprobe, &insn);
- prepare_fixups(uprobe, &insn);
+ handle_riprel_insn(mm, auprobe, &insn);
+ prepare_fixups(auprobe, &insn);
return 0;
}
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index fd45b70..9c6be62 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -29,12 +29,6 @@
struct vm_area_struct;
#ifdef CONFIG_ARCH_SUPPORTS_UPROBES
#include <asm/uprobes.h>
-#else
-
-typedef u8 uprobe_opcode_t;
-struct uprobe_arch_info {};
-
-#define MAX_UINSN_BYTES 4
#endif
/* flags that denote/change uprobes behaviour */
@@ -56,22 +50,9 @@ struct uprobe_consumer {
struct uprobe_consumer *next;
};
-struct uprobe {
- struct rb_node rb_node; /* node in the rb tree */
- atomic_t ref;
- struct rw_semaphore consumer_rwsem;
- struct list_head pending_list;
- struct uprobe_arch_info arch_info;
- struct uprobe_consumer *consumers;
- struct inode *inode; /* Also hold a ref to inode */
- loff_t offset;
- int flags;
- u8 insn[MAX_UINSN_BYTES];
-};
-
#ifdef CONFIG_UPROBES
-extern int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr);
-extern int __weak set_orig_insn(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr, bool verify);
+extern int __weak set_bkpt(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr);
+extern int __weak set_orig_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr, bool verify);
extern bool __weak is_bkpt_insn(uprobe_opcode_t *insn);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *consumer);
extern void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *consumer);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index ee496ad..13f1b59 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -65,6 +65,18 @@ struct vma_info {
loff_t vaddr;
};
+struct uprobe {
+ struct rb_node rb_node; /* node in the rb tree */
+ atomic_t ref;
+ struct rw_semaphore consumer_rwsem;
+ struct list_head pending_list;
+ struct uprobe_consumer *consumers;
+ struct inode *inode; /* Also hold a ref to inode */
+ loff_t offset;
+ int flags;
+ struct arch_uprobe arch;
+};
+
/*
* valid_vma: Verify if the specified vma is an executable vma
* Relax restrictions while unregistering: vm_flags might have
@@ -180,7 +192,7 @@ bool __weak is_bkpt_insn(uprobe_opcode_t *insn)
/*
* write_opcode - write the opcode at a given virtual address.
* @mm: the probed process address space.
- * @uprobe: the breakpointing information.
+ * @arch_uprobe: the breakpointing information.
* @vaddr: the virtual address to store the opcode.
* @opcode: opcode to be written at @vaddr.
*
@@ -190,13 +202,14 @@ bool __weak is_bkpt_insn(uprobe_opcode_t *insn)
* For mm @mm, write the opcode at @vaddr.
* Return 0 (success) or a negative errno.
*/
-static int write_opcode(struct mm_struct *mm, struct uprobe *uprobe,
+static int write_opcode(struct mm_struct *mm, struct arch_uprobe *auprobe,
unsigned long vaddr, uprobe_opcode_t opcode)
{
struct page *old_page, *new_page;
struct address_space *mapping;
void *vaddr_old, *vaddr_new;
struct vm_area_struct *vma;
+ struct uprobe *uprobe;
loff_t addr;
int ret;
@@ -216,6 +229,7 @@ static int write_opcode(struct mm_struct *mm, struct uprobe *uprobe,
if (!valid_vma(vma, is_bkpt_insn(&opcode)))
goto put_out;
+ uprobe = container_of(auprobe, struct uprobe, arch);
mapping = uprobe->inode->i_mapping;
if (mapping != vma->vm_file->f_mapping)
goto put_out;
@@ -326,7 +340,7 @@ static int is_bkpt_at_addr(struct mm_struct *mm, unsigned long vaddr)
* For mm @mm, store the breakpoint instruction at @vaddr.
* Return 0 (success) or a negative errno.
*/
-int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr)
+int __weak set_bkpt(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr)
{
int result;
@@ -337,7 +351,7 @@ int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long v
if (result)
return result;
- return write_opcode(mm, uprobe, vaddr, UPROBES_BKPT_INSN);
+ return write_opcode(mm, auprobe, vaddr, UPROBES_BKPT_INSN);
}
/**
@@ -351,7 +365,7 @@ int __weak set_bkpt(struct mm_struct *mm, struct uprobe *uprobe, unsigned long v
* Return 0 (success) or a negative errno.
*/
int __weak
-set_orig_insn(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr, bool verify)
+set_orig_insn(struct mm_struct *mm, struct arch_uprobe *auprobe, unsigned long vaddr, bool verify)
{
if (verify) {
int result;
@@ -363,7 +377,7 @@ set_orig_insn(struct mm_struct *mm, struct uprobe *uprobe, unsigned long vaddr,
if (result != 1)
return result;
}
- return write_opcode(mm, uprobe, vaddr, *(uprobe_opcode_t *)uprobe->insn);
+ return write_opcode(mm, auprobe, vaddr, *(uprobe_opcode_t *)auprobe->insn);
}
static int match_uprobe(struct uprobe *l, struct uprobe *r)
@@ -593,13 +607,13 @@ static int copy_insn(struct uprobe *uprobe, struct vm_area_struct *vma, unsigned
/* Instruction at the page-boundary; copy bytes in second page */
if (nbytes < bytes) {
- if (__copy_insn(mapping, vma, uprobe->insn + nbytes,
+ if (__copy_insn(mapping, vma, uprobe->arch.insn + nbytes,
bytes - nbytes, uprobe->offset + nbytes))
return -ENOMEM;
bytes = nbytes;
}
- return __copy_insn(mapping, vma, uprobe->insn, bytes, uprobe->offset);
+ return __copy_insn(mapping, vma, uprobe->arch.insn, bytes, uprobe->offset);
}
static int install_breakpoint(struct mm_struct *mm, struct uprobe *uprobe,
@@ -625,23 +639,23 @@ static int install_breakpoint(struct mm_struct *mm, struct uprobe *uprobe,
if (ret)
return ret;
- if (is_bkpt_insn((uprobe_opcode_t *)uprobe->insn))
+ if (is_bkpt_insn((uprobe_opcode_t *)uprobe->arch.insn))
return -EEXIST;
- ret = arch_uprobes_analyze_insn(mm, uprobe);
+ ret = arch_uprobes_analyze_insn(mm, &uprobe->arch);
if (ret)
return ret;
uprobe->flags |= UPROBES_COPY_INSN;
}
- ret = set_bkpt(mm, uprobe, addr);
+ ret = set_bkpt(mm, &uprobe->arch, addr);
return ret;
}
static void remove_breakpoint(struct mm_struct *mm, struct uprobe *uprobe, loff_t vaddr)
{
- set_orig_insn(mm, uprobe, (unsigned long)vaddr, true);
+ set_orig_insn(mm, &uprobe->arch, (unsigned long)vaddr, true);
}
static void delete_uprobe(struct uprobe *uprobe)
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-02-22 16:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-22 9:15 [PATCH 1/3] uprobes/core: Make instruction tables volatile Srikar Dronamraju
2012-02-22 9:15 ` [PATCH 2/3] uprobes/core: remove uprobe_opcode_sz Srikar Dronamraju
2012-02-22 16:05 ` [tip:perf/uprobes] uprobes/core: Remove uprobe_opcode_sz tip-bot for Srikar Dronamraju
2012-02-22 9:16 ` [PATCH 3/3] uprobes/core: move insn to arch specific structure Srikar Dronamraju
2012-02-22 16:06 ` [tip:perf/uprobes] uprobes/core: Move " tip-bot for Srikar Dronamraju
2012-02-22 9:47 ` [PATCH 1/3] uprobes/core: Make instruction tables volatile Ingo Molnar
2012-02-22 10:04 ` Srikar Dronamraju
2012-02-22 16:04 ` [tip:perf/uprobes] " tip-bot for Srikar Dronamraju
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox