On Tue, 08 May 2018 15:41:11 +0530 "Naveen N. Rao" naveen.n.rao@linux.vnet.ibm.com wrote:
Masami Hiramatsu wrote:
On Mon, 07 May 2018 13:41:53 +0530 "Naveen N. Rao" naveen.n.rao@linux.vnet.ibm.com wrote:
I didn't understand that. Which code are you planning to remove? Can you please elaborate? I thought we still need to disable preemption in the ftrace handler.
Yes, kprobe_ftrace_handler itself must be run under preempt disabled because it depends on a per-cpu variable. What I will remove is the redundant preempt disable/enable_noresched (unbalanced) pair in the kprobe_ftrace_handler, and jprobe x86 ports which is no more used.
Won't that break out-of-tree users depending on returning a non-zero value to handle preemption differently? You seem to have alluded to it earlier in the mail chain above where you said that this is not just for jprobes (though it was added for jprobes as the main use case).
No, all users are in tree already (function override for bpf and error-injection).
Ok, so BPF error injection is a new user that can return a non-zero value from the pre handler. It looks like it can use KPROBES_ON_FTRACE too.
In that case, on function entry, we call into kprobe_ftrace_handler() which will call fei_kprobe_handler(), which can re-enable premption before returning 1. So, if you remove the additional prempt_disable()/enable_no_resched() in kprobe_ftrace_handler(), then it will become imbalanced, right?
Right. So we have to fix both at once. Please check the patch below.
https://patchwork.kernel.org/patch/10386171/
(Sorry, I missed to cc you...)
And also, for changing execution path by using kprobes, user handler must call not only preempt_enable(), but also clear current_kprobe per-cpu variable which is not exported to kmodules.
Ok, good point. And that means we don't have any external users any more.
Yes :)
Thank you,
Thanks, Naveen