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

Steven Rostedt
2020-06-30T12:39:20Z

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