On Tue, 14 Feb 2017 14:08:01 +0530
"Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote:
> commit 239aeba76409 ("perf powerpc: Fix kprobe and kretprobe handling
> with kallsyms on ppc64le") changed how we use the offset field in struct
> kprobe on ABIv2. perf now offsets from the GEP (Global entry point) if an
> offset is specified and otherwise chooses the LEP (Local entry point).
>
> Fix the same in kernel for kprobe API users. We do this by extending
> kprobe_lookup_name() to accept an additional parameter to indicate the
> offset specified with the kprobe registration. If offset is 0, we return
> the local function entry and return the global entry point otherwise.
>
> With:
> # cd /sys/kernel/debug/tracing/
> # echo "p _do_fork" >> kprobe_events
> # echo "p _do_fork+0x10" >> kprobe_events
>
> before this patch:
> # cat ../kprobes/list
> c0000000000d0748 k _do_fork+0x8 [DISABLED]
> c0000000000d0758 k _do_fork+0x18 [DISABLED]
> c0000000000412b0 k kretprobe_trampoline+0x0 [OPTIMIZED]
>
> and after:
> # cat ../kprobes/list
> c0000000000d04c8 k _do_fork+0x8 [DISABLED]
> c0000000000d04d0 k _do_fork+0x10 [DISABLED]
> c0000000000412b0 k kretprobe_trampoline+0x0 [OPTIMIZED]
>
> Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kprobes.h | 6 +++---
> arch/powerpc/kernel/optprobes.c | 4 ++--
> kernel/kprobes.c | 6 +++---
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kprobes.h
> b/arch/powerpc/include/asm/kprobes.h
> index d821835ade86..e7ada061aa12 100644
> --- a/arch/powerpc/include/asm/kprobes.h
> +++ b/arch/powerpc/include/asm/kprobes.h
> @@ -60,10 +60,10 @@ extern kprobe_opcode_t optprobe_template_end[];
>
> #ifdef PPC64_ELF_ABI_v2
> /* PPC64 ABIv2 needs local entry point */
> -#define kprobe_lookup_name(name, addr)
> \
> +#define kprobe_lookup_name(name, addr, offset)
> \
> { \
> addr = (kprobe_opcode_t *)kallsyms_lookup_name(name); \
> - if (addr) \
> + if (addr && !(offset)) \
> addr = (kprobe_opcode_t *)ppc_function_entry(addr); \
> }
> #elif defined(PPC64_ELF_ABI_v1)
> @@ -75,7 +75,7 @@ extern kprobe_opcode_t optprobe_template_end[];
> * This ensures we always get to the actual symbol and not the descriptor.
> * Also handle <module:symbol> format.
> */
> -#define kprobe_lookup_name(name, addr)
> \
> +#define kprobe_lookup_name(name, addr, offset)
> \
> { \
> char dot_name[MODULE_NAME_LEN + 1 + KSYM_NAME_LEN]; \
> const char *modsym;
> \
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 2282bf4e63cd..e51a045f3d3b 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -243,8 +243,8 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe
> *op, struct kprobe *p)
> /*
> * 2. branch to optimized_callback() and emulate_step()
> */
> - kprobe_lookup_name("optimized_callback", op_callback_addr);
> - kprobe_lookup_name("emulate_step", emulate_step_addr);
> + kprobe_lookup_name("optimized_callback", op_callback_addr, 0);
> + kprobe_lookup_name("emulate_step", emulate_step_addr, 0);
> if (!op_callback_addr || !emulate_step_addr) {
> WARN(1, "kprobe_lookup_name() failed\n");
> goto error;
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 83ad7e440417..9bc433575d98 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -63,7 +63,7 @@
> * so this must be overridable.
> */
> #ifndef kprobe_lookup_name
> -#define kprobe_lookup_name(name, addr) \
> +#define kprobe_lookup_name(name, addr, offset) \
> addr = ((kprobe_opcode_t *)(kallsyms_lookup_name(name)))
> #endif
Hmm, it smells no good coding... I would like to use __weak function
instead of this "#ifndef" trick.
Thank you,
>
> @@ -1365,7 +1365,7 @@ static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
> goto invalid;
>
> if (p->symbol_name) {
> - kprobe_lookup_name(p->symbol_name, addr);
> + kprobe_lookup_name(p->symbol_name, addr, p->offset);
> if (!addr)
> return ERR_PTR(-ENOENT);
> }
> @@ -2161,7 +2161,7 @@ static int __init init_kprobes(void)
> /* lookup the function address from its name */
> for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> kprobe_lookup_name(kretprobe_blacklist[i].name,
> - kretprobe_blacklist[i].addr);
> + kretprobe_blacklist[i].addr, 0);
> if (!kretprobe_blacklist[i].addr)
> printk("kretprobe: lookup failed: %s\n",
> kretprobe_blacklist[i].name);
> --
> 2.11.0
>
--
Masami Hiramatsu <mhira...@kernel.org>
