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

[Alexei Starovoitov]

On Fri, Jun 26, 2020 at 06:14:55PM -0400, Steven Rostedt wrote:
> On Wed, 24 Jun 2020 20:59:13 -0700
> Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote:
> 
> > > >
> > > > Nack.
> 
> I nack your nack ;-)

ok. let's take it up to Linus to decide.

> 
> > > > The message is bogus. It's used in production kernels.
> > > > bpf_trace_printk() calls it.  
> > > 
> > > Interesting. BTW, the same information (trace_printk is for debugging
> > > only) is repeated all over the place, including where bpf_trace_printk
> > > is documented:
> > > https://elixir.bootlin.com/linux/latest/source/include/linux/kernel.h#L757
> > > https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/bpf.h#L706
> > > https://elixir.bootlin.com/linux/latest/source/kernel/trace/trace.c#L3157
> > > 
> > > Steven added that warning (2184db46e425c ("tracing: Print nasty banner
> > > when trace_printk() is in use")), so maybe he can confirm if it's
> > > still relevant.  
> > 
> > The banner is nasty and it's actively causing harm.
> 
> And it's doing exactly what it was intended on doing!

I disagree. The message is _lying_ about the state of the kernel.
It's not a debug kernel and it's absolutely fine for production.

> > Every few month I have to explain to users that it's absolulte ok to
> > ignore that banner. Nothing bad is happening with the kernel.
> > The kernel is still perfectly safe for production use.
> > It's not a debug kernel.
> > 
> > What bpf_trace_printk() doc is saying that it's not recommended to use
> > this helper for production bpf programs. There are better alternatives.
> > It is absolutely fine to use bpf_trace_printk() to debug production and
> > experimental bpf programs on production servers, android phones and
> > everywhere else.
> 
> Now I do have an answer for you that I believe is a great compromise.
> 
> There's something you can call (and even call it from a module). It's
> called "trace_array_vprintk()". But has one caveat, and that is, you
> can not write to the main top level trace buffer with it (I have
> patches for the next merge window to enforce that). And that's what
> I've been trying to avoid trace_printk() from doing, as that's what it
> does by default. It writes to /sys/kernel/tracing/trace.
> 
> Now what you can do, is have bpf create
> a /sys/kernel/tracing/instances/bpf_trace/ instance, and use
> trace_array_printk(), to print into that, and you will never have to
> see that warning again! It shows up in your own
> tracefs/instances/bpf_trace/trace file!
> 
> If you need more details, let me know, and I can give you all you need
> to know to create you very own trace instance (that can enable events,
> kprobe events, uprobe events, function tracing, and soon function graph
> tracing). And the bonus, you get trace_array_vprintk() and no more
> complaining. :-) :-) :-)

We added a bunch of code to libbcc in the past to support instances,
but eventually removed it all due to memory overhead per instance.
If I recall it was ~8Mbyte per instance. That was couple years ago.

By now everyone has learned to use bpf_trace_printk() and expects
to see the output in /sys/kernel/debug/tracing/trace.
It's documented in uapi/bpf.h and various docs.