Re: [PATCH] Kprobes for ppc64

[Paul Mackerras]

Ananth N Mavinakayanahalli writes:

> Here is kprobes for ppc64. The patch applies on 2.6.9-rc4/2.6.9-final
> and provides the kprobes + jprobes functionality.

> 1. The current implementation uses xmon's emulate_step() and hence 
>    requires xmon to be compiled in. 

We can move emulate_step out to arch/ppc64/lib/step.c (and take out
the printfs).

> 2. arch_prepare_kprobe() now returns an int. I have made the necessary
>    changes to i386 and sparc64 kprobes files, but is untested.

Are you going to send this upstream?

> + * Interrupts are disabled on entry as trap3 is an interrupt gate and they
> + * remain disabled thorough out this function.
> + */
> +static inline int kprobe_handler(struct pt_regs *regs)

Comments about "trap3" and "interrupt gate" don't help me understand
this function on ppc64. :)  At present interrupts are enabled in a
program check exception handler but disabled in a single-step handler.
When does this function get called?

> @@ -96,6 +97,9 @@ int do_page_fault(struct pt_regs *regs, 
>  	BUG_ON((trap == 0x380) || (trap == 0x480));
>  
>  	if (trap == 0x300) {
> +		if (notify_die(DIE_PAGE_FAULT, "page_fault", regs, error_code,
> +					11, SIGSEGV) == NOTIFY_STOP)
> +			return 0;

Hmmm, this seems a bit heavyweight for adding to the page fault path.
Have you done any benchmarks with vs. without kprobes?

On the whole the patch looks OK.  I haven't checked the kprobe_handler
code to see if I think it's all SMP- and preempt-safe, but I assume
you have done it similarly on x86 and checked it there.

Paul.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/