Re: [PATCH] kernel/trace: Add TRACING_ALLOW_PRINTK config option

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 29 Jun 2020 22:16:59 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> > 
> > Warning, not even compiled tested.  
> 
> Thanks! I see what you mean now.

Great! :-)

> 
> > 
> > -- Steve
> > 
> > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > index 6575bb0a0434..aeba5ee7325a 100644
> > --- a/kernel/trace/Makefile
> > +++ b/kernel/trace/Makefile
> > @@ -31,6 +31,8 @@ ifdef CONFIG_GCOV_PROFILE_FTRACE
> >  GCOV_PROFILE := y
> >  endif
> >  
> > +CFLAGS_bpf_trace.o := -I$(src)  
> 
> not following. why this is needed?

It's required in order to have the TRACE_EVENT macro magic work. More
info about it here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/trace_events/Makefile


> 
> > +
> >  CFLAGS_trace_benchmark.o := -I$(src)
> >  CFLAGS_trace_events_filter.o := -I$(src)
> >  
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index dc05626979b8..01bedf335b2e 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -19,6 +19,9 @@
> >  #include "trace_probe.h"
> >  #include "trace.h"
> >  
> > +#define CREATE_TRACE_EVENTS  
> 
> CREATE_TRACE_POINTS ?


Doh, yeah. I did say it wasn't even compiled tested ;-)

> 
> > +#include "bpf_trace.h"
> > +
> >  #define bpf_event_rcu_dereference(p)					\
> >  	rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))
> >  
> > @@ -473,13 +476,29 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> >  		fmt_cnt++;
> >  	}
> >  
> > +static DEFINE_SPINLOCK(trace_printk_lock);
> > +#define BPF_TRACE_PRINTK_SIZE	1024
> > +
> > +static inline void do_trace_printk(const char *fmt, ...)
> > +{
> > +	static char buf[BPF_TRACE_PRINT_SIZE];
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&trace_printk_lock, flags);
> > +	va_start(ap, fmt);
> > +	vsnprintf(buf, BPF_TRACE_PRINT_SIZE, fmt, ap);
> > +	va_end(ap);
> > +
> > +	trace_bpf_trace_printk(buf);
> > +	spin_unlock_irqrestore(&trace_printk_lock, flags);  
> 
> interesting. I don't think anyone would care about spin_lock overhead.
> It's better because 'trace_bpf_trace_printk' would be a separate event
> that can be individually enabled/disabled?
> I guess it can work.
> Thanks!

I hope this does everything you need for bpf_trace_printk. If there's
something  that's not working for you, PLEASE reach out to me and ask
what you need.

Cheers!

-- Steve