lkml.org 
[lkml]   [2020]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] kernel/trace: Add TRACING_ALLOW_PRINTK config option
On Sun, 28 Jun 2020 15:02:09 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> >
> > Then do a bpf trace event and enable it when a bpf_trace_printk() is
> > loaded. It will work the same for your users.
>
> I'm not sure I follow. How that would preserve the expectation
> to see the output in /sys/kernel/debug/tracing/trace ?

You create a bpf event just like you create any other event. When a bpf
program that uses a bpf_trace_printk() is loaded, you can enable that
event from within the kernel. Yes, there's internal interfaces to
enabled and disable events just like echoing 1 into
tracefs/events/system/event/enable. See trace_set_clr_event().

Then the data of that event will appear in
the /sys/kernel/tracing/trace file just like the trace_printk does.

The difference is, if something in the kernel decides to use that
event, I can easily disable it from user space, where trace_printk() I
can't.


> >
> > Hmm, so you are happier to bully and burn bridges with me to deprecate
> > the trace_printk() interface, than to work with me and add an update to
> > look into an instance for the print instead of the top level? That's
> > not very collaborative.
>
> I'm seeing it differently.
> I'm saying bpf users are complaining about misleading dmesg warning.
> You're saying 'screw your users I want to keep that warning'.
> Though the warning is lying with a straight face. The only thing happened
> is few pages were allocated that will never be freed. The kernel didn't
> suddenly become non-production. It didn't become slower. No debug features
> were turned on.

Come now Alexei. That banner was there from day one trace_printk() was
added into the kernel. YOU used this knowing damn well that banner
existed. If the bpf users should be upset with someone, it is you for
not asking me for how to do this properly from the beginning.

This is not a regression. trace_printk() always has shown this, and
when I added trace_printk() I stated this is only for debugging a
kernel, and not to be kept in mainline. That banner helped enforce
that. If I didn't do that, there would be trace_printk()s all over the
place, and there's no way to disable one without disabling all the
others. This would have made trace_printk() become useless for
debugging a kernel, as then you will have to deal with everyone's
trace_printks() adding noise to what you want to debug.

-- Steve

\
 
 \ /
  Last update: 2020-06-29 00:29    [W:0.065 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site