Steven Rostedt wrote:
On Thu, 27 Jun 2019 20:58:20 +0530
"Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote:
> But interesting, I don't see a synchronize_rcu_tasks() call
> there.
We felt we don't need it in this case. We patch the branch to ftrace
with a nop first. Other cpus should see that first. But, now that I
think about it, should we add a memory barrier to ensure the writes get
ordered properly?
Do you send an ipi to the other CPUs. I would just to be safe.
<snip>
We are handling this through ftrace_replace_code() and
__ftrace_make_call_prep() below. For FTRACE_UPDATE_MAKE_CALL, we patch
in the mflr, followed by smp_call_function(isync) and
synchronize_rcu_tasks() before we proceed to patch the branch to ftrace.
I don't see any other scenario where we end up in
__ftrace_make_nop_kernel() without going through ftrace_replace_code().
For kernel modules, this can happen during module load/init and so, I
patch out both instructions in __ftrace_make_call() above without any
synchronization.
Am I missing anything?
No, I think I got confused ;-), it's the patch out that I was worried
about, but when I was going through the scenario, I somehow turned it
into the patching in (which I already audited :-p). I was going to
reply with just the top part of this email, but then the confusion
started :-/
OK, yes, patching out should be fine, and you already covered the
patching in. Sorry for the noise.
Just to confirm and totally remove the confusion, the patch does:
<func>:
mflr r0 <-- preempt here
bl _mcount
<func>:
mflr r0
nop
And this is fine regardless.
OK, Reviewed-by: Steven Rostedt (VMware) <rost...@goodmis.org>
Thanks for confirming! We do need an IPI to be sure, as you pointed out
above. I will have the patching out take the same path to simplify
things.
- Naveen