Re: [PATCH v7 00/16] tracing: probeevent: Improve fetcharg features

From: Naveen N. Rao
Date: Tue May 08 2018 - 06:11:29 EST

Masami Hiramatsu wrote:
On Mon, 07 May 2018 13:41:53 +0530
"Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> 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?

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.