[PATCH] tracing: bpf: Hide bpf trace events when they are not used

[PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Steven Rostedt]

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>
---
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

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Alexei Starovoitov]

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?

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Steven Rostedt]

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

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Alexei Starovoitov]

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.

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Steven Rostedt]

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

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[David Miller]

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?

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Steven Rostedt]

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

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[David Miller]

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>

Re: [PATCH] tracing: bpf: Hide bpf trace events when they are not used

[Steven Rostedt]

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