Skip to contentSkip to conversation

(yet another) kprobes x86 code unification and boosters

PATCH In review Masami Hiramatsu 3 participants 4 messages Dec 17, 2007

lore
Share & tools
lore
#0 lore raw

cover (yet another) kprobes x86 code unification and boosters

Hello all,

I developed a series of patches which unifies kprobes code on x86
and introduces boosters on x86-64. These patches can be applied to 2.6.24-rc4-mm1.

The purpose of this patchset is unifying kprobes_[32|64].[c|h] to kprobes.[c|h] for simplifying code maintenance.

I know these patches are conflicting with Harvey's patch. We need to solve that.

kprobe-booster and kretprobe-booster were explained in:
http://marc.info/?l=linux-kernel&m=113862526017068&w=2

Currently, these patches do unification as following order.
1. Clean up and fix bugs in kprobes[1/6, 2/6].
2. Introduce kprobe-booster and kretprobe-booster for x86-64[3/6, 4/6]. (x86-32 kprobes already has same functionalities)
3. Prepare unification[5/6].
4. Unify kprobes code[6/6].

If you have any comment, please let me know.

Best Regards,

Signature · 7 lines
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: [email protected], [email protected]
#1 lore raw
Quoted 11 lines · Masami Hiramatsu

Hello all,

I developed a series of patches which unifies kprobes code on x86 and introduces boosters on x86-64. These patches can be applied to 2.6.24-rc4-mm1.

The purpose of this patchset is unifying kprobes_[32|64].[c|h] to kprobes.[c|h] for simplifying code maintenance.

I know these patches are conflicting with Harvey's patch. We need to solve that.

your series fixes the 64-bit crash that i was seeing, so i've picked it up. Please work it out with Harvey which cleanups of him are not included yet.

Ingo

#2 lore raw
Quoted 11 lines · Ingo Molnar
Quoted 9 lines · Masami Hiramatsu

Hello all,

I developed a series of patches which unifies kprobes code on x86 and introduces boosters on x86-64. These patches can be applied to 2.6.24-rc4-mm1.

The purpose of this patchset is unifying kprobes_[32|64].[c|h] to kprobes.[c|h] for simplifying code maintenance.

Quoted 7 lines
Quoted 2 lines

I know these patches are conflicting with Harvey's patch. We need to solve that.

your series fixes the 64-bit crash that i was seeing, so i've picked it up. Please work it out with Harvey which cleanups of him are not included yet.

I thought the jprobe_saved_sp looked fishy on X86_64 in my unified set but I went bug-for-bug compatible I guess :-) If you look at my cleaned up set this was one of the main source of remaining ifdefs.

Harvey

#3 lore raw
In reply to Ingo Molnar

Hi,

Quoted 11 lines · Ingo Molnar
Quoted 9 lines · Masami Hiramatsu

Hello all,

I developed a series of patches which unifies kprobes code on x86 and introduces boosters on x86-64. These patches can be applied to 2.6.24-rc4-mm1.

The purpose of this patchset is unifying kprobes_[32|64].[c|h] to kprobes.[c|h] for simplifying code maintenance.

Quoted 6 lines
Quoted 2 lines

I know these patches are conflicting with Harvey's patch. We need to solve that.

your series fixes the 64-bit crash that i was seeing, so i've picked it up. Please work it out with Harvey which cleanups of him are not included yet.

Absolutely sure.
I compared my patch and Harvey's.
These directions are almost same.

Harvey, I have found some differences and I'd like to fix that with you. I think following comments and style cleanups in your patch are good to me.

Quoted 30 lines
(quoted diff) +3 -3
@@ -156,7 +157,7 @@ twobyte_has_modrm[256 / sizeof(unsigned long)] = {
 #undef RF
 
 /* insert a jmp code */
-static __always_inline void set_jmp_op(void *from, void *to)
+static inline void set_jmp_op(void *from, void *to)
 {
 	struct __arch_jmp_op {
 		char op;
@@ -170,7 +171,7 @@ static __always_inline void set_jmp_op(void *from, void *to)
 /*
  * returns non-zero if opcodes can be boosted.
  */
-static __always_inline int can_boost(kprobe_opcode_t *opcodes)
+static inline int can_boost(kprobe_opcode_t *opcodes)
 {
 	kprobe_opcode_t opcode;
 	kprobe_opcode_t *orig_opcodes = opcodes;
@@ -734,7 +740,7 @@ static int __kprobes post_kprobe_handler(struct pt_regs *regs)
 	regs->flags |= kcb->kprobe_saved_flags;
 	trace_hardirqs_fixup_flags(regs->flags);
 
-	/*Restore back the original saved kprobes variables and continue. */
+	/* Restore the original saved kprobes variables and continue. */
 	if (kcb->kprobe_status == KPROBE_REENTER) {
 		restore_previous_kprobe(kcb);
 		goto out;

Review int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs) · line 872

Quoted 8 lines
(quoted diff) +1 -1
@@ -860,7 +866,7 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
 	addr = (unsigned long)(kcb->jprobe_saved_sp);
 
 	/*
-	 * TBD: As Linus pointed out, gcc assumes that the callee
+	 * As Linus pointed out, gcc assumes that the callee
 	 * owns the argument space and could overwrite it, e.g.
 	 * tailcall optimization. So, to be absolutely safe
 	 * we also save and restore enough stack bytes to cover

And also, if you can unify x86/mm/extable_*.c and introduce fixup_exception() to 64-bit, it is very helpful to remove ifdefs from kprobe_fault_handler().

Thank you,

Signature · 7 lines
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: [email protected], [email protected]