Re: [RFC] ftrace: Add support to keep some functions out of ftrace

[Jiri Olsa <olsajiri@gmail.com>]

On Sat, Jul 23, 2022 at 11:39:27PM +0200, Jiri Olsa wrote:
> On Fri, Jul 22, 2022 at 05:41:20PM -0400, Steven Rostedt wrote:
> > On Fri, 22 Jul 2022 23:05:19 +0200
> > Jiri Olsa <olsajiri@gmail.com> wrote:
> > 
> > > ok, I think we could use that, I'll check
> > > 
> > > > 
> > > > But other than that, we don't need infrastructure to hide any mcount/fentry
> > > > locations from ftrace. Those were add *for* ftrace.  
> > > 
> > > I think I understand the fentry/ftrace equivalence you see, I remember
> > > the perl mcount script ;-)
> > 
> > It's even more than that. We worked with the compiler folks to get fentry
> > for ftrace purposes (namely to speed it up, and not rely on frame
> > pointers, which mcount did). fentry never existed until then. Like I said.
> > fentry was created *for* ftrace. And currently it's x86 specific, as it
> > relies on the calling convention that a call does both, push the return
> > address onto the  stack, and jump to a function. The blr
> > (branch-link-register) method is more complex, which is where the
> > "patchable" work comes from.
> > 
> > > 
> > > still I think we should be able to define function that has fentry
> > > profile call and be able to manage it without ftrace
> > > 
> > > one other thought.. how about adding function that would allow to disable
> > > function in ftrace, with existing FTRACE_FL_DISABLED or some new flag
> > > 
> > > that way ftrace still keeps track of it, but won't allow to use it in
> > > ftrace infra
> > 
> > Another way is to remove it at compile time from the mcount_loc table, and
> > add it to your own table. I take it, this is for bpf infrastructure code
> 
> hm, perhaps we could move it to separate object and switch off
> -mrecord-mcount for it, I'll check

the patch below moves the bpf function into sepatate object and switches
off the -mrecord-mcount for it.. so the function gets profile call
generated but it's not visible to ftrace

this seems to work, but it depends on -mrecord-mcount support in gcc and
it's x86 specific... other archs seems to use -fpatchable-function-entry,
which does not seem to have option to omit symbol from being collected
to the section

disabling specific ftrace symbol with FTRACE_FL_DISABLED flag seems to
be easir and generic solution.. I'll send RFC for that

jirka


---
diff --git a/net/core/Makefile b/net/core/Makefile
index e8ce3bd283a6..7d7ba2038879 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -12,10 +12,14 @@ obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
 obj-y		     += dev.o dev_addr_lists.o dst.o netevent.o \
 			neighbour.o rtnetlink.o utils.o link_watch.o filter.o \
 			sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \
-			fib_notifier.o xdp.o flow_offload.o gro.o
+			fib_notifier.o xdp.o flow_offload.o gro.o \
+			dispatcher.o
 
 obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o
 
+# remove dispatcher function from ftrace sight
+CFLAGS_REMOVE_dispatcher.o = -mrecord-mcount
+
 obj-y += net-sysfs.o
 obj-$(CONFIG_PAGE_POOL) += page_pool.o
 obj-$(CONFIG_PROC_FS) += net-procfs.o
diff --git a/net/core/dispatcher.c b/net/core/dispatcher.c
new file mode 100644
index 000000000000..7d2730b15de0
--- /dev/null
+++ b/net/core/dispatcher.c
@@ -0,0 +1,3 @@
+#include <linux/bpf.h>
+
+DEFINE_BPF_DISPATCHER(xdp)
diff --git a/net/core/filter.c b/net/core/filter.c
index 5669248aff25..23fe2c5dfe9d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11333,8 +11333,6 @@ const struct bpf_verifier_ops sk_lookup_verifier_ops = {
 
 #endif /* CONFIG_INET */
 
-DEFINE_BPF_DISPATCHER(xdp)
-
 void bpf_prog_change_xdp(struct bpf_prog *prev_prog, struct bpf_prog *prog)
 {
 	bpf_dispatcher_change_prog(BPF_DISPATCHER_PTR(xdp), prev_prog, prog);