* Re: [-mm][PATCH 0/6] (yet another) kprobes x86 code unification and boosters
2007-12-17 16:33 ` Ingo Molnar
@ 2007-12-17 19:17 ` Harvey Harrison
2007-12-17 19:24 ` Masami Hiramatsu
1 sibling, 0 replies; 4+ messages in thread
From: Harvey Harrison @ 2007-12-17 19:17 UTC (permalink / raw)
To: Ingo Molnar
Cc: Masami Hiramatsu, Ananth N Mavinakayanahalli, Jim Keniston,
Roland McGrath, Arjan van de Ven, prasanna, anil.s.keshavamurthy,
davem, systemtap-ml, LKML
On Mon, 2007-12-17 at 17:33 +0100, Ingo Molnar wrote:
> * Masami Hiramatsu <mhiramat@redhat.com> wrote:
>
> > 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.
>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [-mm][PATCH 0/6] (yet another) kprobes x86 code unification and boosters
2007-12-17 16:33 ` Ingo Molnar
2007-12-17 19:17 ` Harvey Harrison
@ 2007-12-17 19:24 ` Masami Hiramatsu
1 sibling, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2007-12-17 19:24 UTC (permalink / raw)
To: Ingo Molnar, Harvey Harrison
Cc: Ananth N Mavinakayanahalli, Jim Keniston, Roland McGrath,
Arjan van de Ven, prasanna, anil.s.keshavamurthy, davem,
systemtap-ml, LKML, Andrew Morton
Hi,
Ingo Molnar wrote:
> * Masami Hiramatsu <mhiramat@redhat.com> wrote:
>
>> 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.
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.
> @@ -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;
> @@ -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,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 4+ messages in thread