| Message ID | 20171012184002.0661a867@gandalf.local.home |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | tracing: bpf: Hide bpf trace events when they are not used | expand |
On Thu, Oct 12, 2017 at 06:40:02PM -0400, Steven Rostedt wrote: > From: Steven Rostedt (VMware) <rostedt@goodmis.org> > > All the trace events defined in include/trace/events/bpf.h are only > used when CONFIG_BPF_SYSCALL is defined. But this file gets included by > include/linux/bpf_trace.h which is included by the networking code with > CREATE_TRACE_POINTS defined. > > If a trace event is created but not used it still has data structures > and functions created for its use, even though nothing is using them. > To not waste space, do not define the BPF trace events in bpf.h unless > CONFIG_BPF_SYSCALL is defined. > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Looks fine. Acked-by: Alexei Starovoitov <ast@kernel.org> I'm assuming you want to take it through tracing tree along with all other cleanups?
On Thu, 12 Oct 2017 18:14:52 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > On Thu, Oct 12, 2017 at 06:40:02PM -0400, Steven Rostedt wrote: > > From: Steven Rostedt (VMware) <rostedt@goodmis.org> > > > > All the trace events defined in include/trace/events/bpf.h are only > > used when CONFIG_BPF_SYSCALL is defined. But this file gets included by > > include/linux/bpf_trace.h which is included by the networking code with > > CREATE_TRACE_POINTS defined. > > > > If a trace event is created but not used it still has data structures > > and functions created for its use, even though nothing is using them. > > To not waste space, do not define the BPF trace events in bpf.h unless > > CONFIG_BPF_SYSCALL is defined. > > > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > Looks fine. > Acked-by: Alexei Starovoitov <ast@kernel.org> > > I'm assuming you want to take it through tracing tree along > with all other cleanups? Either way is fine. I have a few other ones. I believe Paul is taking the RCU patch. There's no dependency. I'll take it if it is easier for you. I just need the ack. -- Steve
On Thu, Oct 12, 2017 at 09:35:01PM -0400, Steven Rostedt wrote: > On Thu, 12 Oct 2017 18:14:52 -0700 > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > > On Thu, Oct 12, 2017 at 06:40:02PM -0400, Steven Rostedt wrote: > > > From: Steven Rostedt (VMware) <rostedt@goodmis.org> > > > > > > All the trace events defined in include/trace/events/bpf.h are only > > > used when CONFIG_BPF_SYSCALL is defined. But this file gets included by > > > include/linux/bpf_trace.h which is included by the networking code with > > > CREATE_TRACE_POINTS defined. > > > > > > If a trace event is created but not used it still has data structures > > > and functions created for its use, even though nothing is using them. > > > To not waste space, do not define the BPF trace events in bpf.h unless > > > CONFIG_BPF_SYSCALL is defined. > > > > > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > > > Looks fine. > > Acked-by: Alexei Starovoitov <ast@kernel.org> > > > > I'm assuming you want to take it through tracing tree along > > with all other cleanups? > > Either way is fine. I have a few other ones. I believe Paul is taking > the RCU patch. There's no dependency. > > I'll take it if it is easier for you. I just need the ack. actually just noticed that xdp tracepoints are not covered by ifdef. They depend on bpf_syscall too. So probably makes sense to wrap them together. bpf tracepoints are not being actively worked on whereas xdp tracepoints keep evolving quickly, so the best is probalby to go via net-next if you don't mind.
On Thu, 12 Oct 2017 18:38:36 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > actually just noticed that xdp tracepoints are not covered by ifdef. > They depend on bpf_syscall too. So probably makes sense to wrap > them together. > bpf tracepoints are not being actively worked on whereas xdp tracepoints > keep evolving quickly, so the best is probalby to go via net-next > if you don't mind. Hmm, they didn't trigger a warning, with the exception of trace_xdp_redirect_map. I have code to check if tracepoints are used or not, and it appears that the xdp can be used without BPF_SYSCALL. I don't think they should be wrapped together until we know why they are used. I can still take this patch and just not touch the xdp ones. Note, my kernel was using trace_xdp_redirect_map_err, trace_xdp_redirect_err, trace_xdp_redirect and trace_xdp_exception. As they did appear. -- Steve
From: Steven Rostedt <rostedt@goodmis.org> Date: Thu, 12 Oct 2017 18:40:02 -0400 > From: Steven Rostedt (VMware) <rostedt@goodmis.org> > > All the trace events defined in include/trace/events/bpf.h are only > used when CONFIG_BPF_SYSCALL is defined. But this file gets included by > include/linux/bpf_trace.h which is included by the networking code with > CREATE_TRACE_POINTS defined. > > If a trace event is created but not used it still has data structures > and functions created for its use, even though nothing is using them. > To not waste space, do not define the BPF trace events in bpf.h unless > CONFIG_BPF_SYSCALL is defined. > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Steven, I lost track of how this patch is being handled. Do you want me to merge it via my net-next tree?
On Mon, 16 Oct 2017 20:54:34 +0100 (WEST) David Miller <davem@davemloft.net> wrote: > Steven, I lost track of how this patch is being handled. > > Do you want me to merge it via my net-next tree? If you want. I could take it too if you give me an ack. It's not dependent on any other changes so it doesn't matter which way it goes. I know Alexei was thinking about doing the same for xdp but those appear to be used even without BPF_SYSCALLS. -- Steve
From: Steven Rostedt <rostedt@goodmis.org> Date: Mon, 16 Oct 2017 16:01:25 -0400 > On Mon, 16 Oct 2017 20:54:34 +0100 (WEST) > David Miller <davem@davemloft.net> wrote: > >> Steven, I lost track of how this patch is being handled. >> >> Do you want me to merge it via my net-next tree? > > If you want. I could take it too if you give me an ack. It's not > dependent on any other changes so it doesn't matter which way it goes. I > know Alexei was thinking about doing the same for xdp but those appear > to be used even without BPF_SYSCALLS. Ok, applied to my net-next tree and if you want to apply it to your's too, here is the ACK: Acked-by: David S. Miller <davem@davemloft.net>
On Mon, 16 Oct 2017 21:11:06 +0100 (WEST) David Miller <davem@davemloft.net> wrote: > From: Steven Rostedt <rostedt@goodmis.org> > Date: Mon, 16 Oct 2017 16:01:25 -0400 > > > On Mon, 16 Oct 2017 20:54:34 +0100 (WEST) > > David Miller <davem@davemloft.net> wrote: > > > >> Steven, I lost track of how this patch is being handled. > >> > >> Do you want me to merge it via my net-next tree? > > > > If you want. I could take it too if you give me an ack. It's not > > dependent on any other changes so it doesn't matter which way it goes. I > > know Alexei was thinking about doing the same for xdp but those appear > > to be used even without BPF_SYSCALLS. > > Ok, applied to my net-next tree and if you want to apply it to your's > too, here is the ACK: > > Acked-by: David S. Miller <davem@davemloft.net> Thanks! But if you are taking it, I'm fine with that. It may be a while before I get my tool applied that detects unused tracepoints at compile time. I have it working at runtime, but rather have a compiler warning. -- Steve
Index: linux-trace.git/include/trace/events/bpf.h =================================================================== --- linux-trace.git.orig/include/trace/events/bpf.h +++ linux-trace.git/include/trace/events/bpf.h @@ -4,6 +4,9 @@ #if !defined(_TRACE_BPF_H) || defined(TRACE_HEADER_MULTI_READ) #define _TRACE_BPF_H +/* These are only used within the BPF_SYSCALL code */ +#ifdef CONFIG_BPF_SYSCALL + #include <linux/filter.h> #include <linux/bpf.h> #include <linux/fs.h> @@ -345,7 +348,7 @@ TRACE_EVENT(bpf_map_next_key, __print_hex(__get_dynamic_array(nxt), __entry->key_len), __entry->key_trunc ? " ..." : "") ); - +#endif /* CONFIG_BPF_SYSCALL */ #endif /* _TRACE_BPF_H */ #include <trace/define_trace.h> Index: linux-trace.git/kernel/bpf/core.c =================================================================== --- linux-trace.git.orig/kernel/bpf/core.c +++ linux-trace.git/kernel/bpf/core.c @@ -1498,5 +1498,8 @@ int __weak skb_copy_bits(const struct sk EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_exception); +/* These are only used within the BPF_SYSCALL code */ +#ifdef CONFIG_BPF_SYSCALL EXPORT_TRACEPOINT_SYMBOL_GPL(bpf_prog_get_type); EXPORT_TRACEPOINT_SYMBOL_GPL(bpf_prog_put_rcu); +#endif