Re: [PATCH] uprobes/x86: emulate push insns for uprobe on x86
From: Oleg Nesterov <oleg@redhat.com>
Date: 2017-11-09 13:44:28
Also in:
lkml
On 11/09, Yonghong Song wrote:
This patch extends the emulation to "push <reg>" insns. These insns are typical in the beginning of the function. For example, bcc in https://github.com/iovisor/bcc repo provides tools to measure funclantency, detect memleak, etc. The tools will place uprobes in the beginning of function and possibly uretprobes at the end of function. This patch is able to reduce the trap overhead for uprobe from 2 to 1.
OK. but to be honest I do not like the implementation, please see below.
quoted hunk ↗ jump to hunk
+enum uprobe_insn_t { + UPROBE_BRANCH_INSN = 0, + UPROBE_PUSH_INSN = 1, +}; + struct uprobe_xol_ops; struct arch_uprobe {@@ -42,6 +47,7 @@ struct arch_uprobe { }; const struct uprobe_xol_ops *ops; + enum uprobe_insn_t insn_class;
Why?
I'd suggest to leave branch_xol_ops alone and add the new push_xol_ops{},
the code will look much simpler.
The only thing they can share is branch_post_xol_op() which is just
regs->sp += sizeof_long();
return -ERESTART;
I think a bit of code duplication would be fine in this case.
And. Do you really need ->post_xol() method to emulate "push"? Why we can't
simply execute it out-of-line if copy_to_user() fails?
branch_post_xol_op() is needed because we can't execute "call" out-of-line,
we need to restart and try again if copy_to_user() fails, but I don not
understand why it is needed to emulate "push".
Oleg.