Hi,

On Thu, 22 Aug 2019 12:23:58 +0530
"Naveen N. Rao" [off-list ref] wrote:

Jisheng Zhang wrote:

quoted

KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
eliminates the need for a trap, as well as the need to emulate or
single-step instructions.

Tested on berlin arm64 platform.

~ # mount -t debugfs debugfs /sys/kernel/debug/
~ # cd /sys/kernel/debug/
/sys/kernel/debug # echo 'p _do_fork' > tracing/kprobe_events

before the patch:

/sys/kernel/debug # cat kprobes/list
ffffff801009fe28  k  _do_fork+0x0    [DISABLED]

after the patch:

/sys/kernel/debug # cat kprobes/list
ffffff801009ff54  k  _do_fork+0x4    [DISABLED][FTRACE]

Signed-off-by: Jisheng Zhang <redacted>  

This looks good to me. Except for a small confirmation below:
Reviewed-by: Naveen N. Rao <redacted>

<...>

quoted

+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+                        struct ftrace_ops *ops, struct pt_regs *regs)
+{
+     struct kprobe *p;
+     struct kprobe_ctlblk *kcb;
+
+     /* Preempt is disabled by ftrace */
+     p = get_kprobe((kprobe_opcode_t *)ip);
+     if (unlikely(!p) || kprobe_disabled(p))
+             return;
+
+     kcb = get_kprobe_ctlblk();
+     if (kprobe_running()) {
+             kprobes_inc_nmissed_count(p);
+     } else {
+             unsigned long orig_ip = instruction_pointer(regs);
+             /* Kprobe handler expects regs->pc = pc + 4 as breakpoint hit */
+             instruction_pointer_set(regs, ip + sizeof(kprobe_opcode_t));  

Just want to make sure that you've confirmed that this is what happens
with a regular trap/brk based kprobe on ARM64. The reason for setting
the instruction pointer here is to ensure that it is set to the same
value as would be set if there was a trap/brk instruction at the ftrace
location. This ensures that the kprobe pre handler sees the same value
regardless.

Due to the arm64's DYNAMIC_FTRACE_WITH_REGS implementation, the code itself
is correct. But this doesn't look like "there was a trap instruction at
the ftrace location".

W/O KPROBE_ON_FTRACE:

foo:
00	insA
04	insB
08	insC

kprobe's pre_handler() will see pc points to 00.

W/ KPROBE_ON_FTRACE:

foo:
00	lr saver
04	nop     // will be modified to ftrace call ins when KPROBE is armed
08	insA
0c	insB

later, kprobe_ftrace_handler() will see pc points to 04, so pc + 4 will
point to 08 the same as the one w/o KPROBE_ON_FTRACE.

It seems I need to fix the comment.

Further changes to the instruction pointer are to achieve the same
effect for kprobe post handlers.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel