On Sun, 9 Apr 2023 22:44:37 +0800
Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> > You can use ftrace_test_recursion_trylock() before using migrate_disable()
> > if you ensure the callback is only for fentry. Can you prepare a fentry
> > specific callback?
> >
>
> fentry uses both ftrace-based probe and text-poke-based probe.
> ftrace_test_recursion_trylock() can avoid the recursion in the
> ftrace-based probe. Not sure if we can split bpf_trampoline_enter into
> two callbacks, one for ftrace and another for text poke. I will
> analyze it. Thanks for your suggestion.
ftrace_test_recusion_trylock() works with anything. What it basically
is doing is to make sure that you do not pass that same location in the
same context.
That is, it sets a specific bit to which context it is in (normal,
softirq, irq or NMI). If you call it again in the same context, it will
return false (recursion detected).
That is,
normal:
ftrace_test_recursion_trylock(); <- OK
softirq:
ftrace_test_recursion_trylock() <- OK
hard-irq:
ftrace_test_recusion_trylock() <- OK
[ recusion happens ]
ftrace_test_recursion_trylock() <- FAIL!
That's because it is perfectly fine for the same code path to enter
ftrace code in different contexts, but if it happens in the same
context, that has to be due something calling back into itself.
Thus if you had:
bit = ftrace_test_recursion_trylock();
if (bit < 0)
return;
migrate_disable();
ftrace_test_test_recursion_unlock(bit);
It would prevent migrate_disable from recursing.
bpf_func()
bit = ftrace_test_recursion_trylock() <<- returns 1
migrate_disable()
preempt_count_add()
bpf_func()
bit = ftrace_test_recusion_trylock() <<- returns -1
if (bit < 0)
return;
It would prevent the crash.
-- Steve
