On Fri, 22 Jul 2022 09:53:32 -0700 Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > > > Sounds like a BPF bug to me. Ftrace did nothing to cause this breakage. It > > was something BPF must have done. What exactly is BPF doing to ftrace > > locations anyway? > > ftrace location? > fentry != ftrace. It was the entire reason to add that. Basically, Yes, fentry == ftrace! BPF can not steal it. Look at the history of it. Everything to do with fentry being added to Linux was for ftrace, and ftrace was designed specifically for fentry. The ftrace infrastructure was designed around fentry/mcount. The rest of the tracing infrastructure was called "ftrace" for the same reason people call distros Linux (and why FSF hates that). Just because it was built around the ftrace infrastructure, not the other way around. This is why I renamed all the ftrace.h files in include/trace/ to trace_event.h. Because calling it ftrace.h was a misnomer. > nop5 in the beginning of the function or in the middle of it > doesn't mean that it's safe for ftrace to attach there. How did that nop5 get put there? Before compilers added support for doing that at compile time (which they added *FOR* ftrace), it was ftrace that converted the calls to mcount/fentry to nops. In fact, the fentry trampoline exists in ftrace_64.S. > In some cases bpf has custom calling convention > like it preserves %rax. > In other cases there will be multiple nop5 locations > through the function where special care needs to be taken > to attach. > > > > > > > > > If you don't like Jiri's approach please propose something else. > > > > > > So, why not mark it as notrace? That will prevent ftrace from looking at it. > > > > > > > And if for some strange reason you need the mcount/fentry on some internal > > BPF infrastructure, the work around is to register two ftrace_ops() that > > have filters to that function. In which case ftrace will force the call to > > the ftrace iterator loop, and any more ops attached will simply be added to > > that loop, and ftrace will no longer touch that location. > > > > Then you can do whatever you want to it without fear of racing with ftrace. > > Jiri, > that sounds like a workable solution. > wdyt? > > > But other than that, we don't need infrastructure to hide any mcount/fentry > > locations from ftrace. Those were add *for* ftrace. > > fentry != ftrace. I 100% disagree with the above statement. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/ftrace_64.S#n134 -- Steve
