From: Steven Rostedt <rostedt@goodmis.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Nicolas Boichat <drinkcat@chromium.org>,
LKML <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
Kees Cook <keescook@chromium.org>, Jason Gunthorpe <jgg@ziepe.ca>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Peter Zijlstra <peterz@infradead.org>,
Vinod Koul <vkoul@kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Alexey Dobriyan <adobriyan@gmail.com>,
Tiezhu Yang <yangtiezhu@loongson.cn>,
Thomas Gleixner <tglx@linutronix.de>,
"Guilherme G . Piccoli" <gpiccoli@canonical.com>,
Will Deacon <will@kernel.org>,
Douglas Anderson <dianders@chromium.org>,
Guenter Roeck <groeck@chromium.org>,
bpf@vger.kernel.org
Subject: Re: [PATCH] kernel/trace: Add TRACING_ALLOW_PRINTK config option
Date: Sun, 28 Jun 2020 18:28:42 -0400 [thread overview]
Message-ID: <20200628182842.2abb0de2@oasis.local.home> (raw)
In-Reply-To: <20200628220209.3oztcjnzsotlfria@ast-mbp.dhcp.thefacebook.com>
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
next prev parent reply other threads:[~2020-06-28 22:28 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-24 8:45 [PATCH] kernel/trace: Add TRACING_ALLOW_PRINTK config option Nicolas Boichat
2020-06-24 13:57 ` Jason Gunthorpe
2020-06-25 1:13 ` Nicolas Boichat
2020-06-24 16:04 ` Steven Rostedt
2020-06-24 17:25 ` Alexei Starovoitov
2020-06-25 2:00 ` Nicolas Boichat
2020-06-25 3:59 ` Alexei Starovoitov
2020-06-26 22:14 ` Steven Rostedt
2020-06-28 17:27 ` Alexei Starovoitov
2020-06-28 18:46 ` Steven Rostedt
2020-06-28 19:00 ` Steven Rostedt
2020-06-28 19:21 ` Alexei Starovoitov
2020-06-28 19:43 ` Steven Rostedt
2020-06-28 22:02 ` Alexei Starovoitov
2020-06-28 22:28 ` Steven Rostedt [this message]
2020-06-28 23:43 ` Steven Rostedt
2020-06-30 5:16 ` Alexei Starovoitov
2020-06-30 12:39 ` Steven Rostedt
2020-06-25 1:32 ` Nicolas Boichat
2020-06-24 17:37 ` kernel test robot
2020-06-24 19:07 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200628182842.2abb0de2@oasis.local.home \
--to=rostedt@goodmis.org \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=alexei.starovoitov@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bpf@vger.kernel.org \
--cc=daniel.vetter@ffwll.ch \
--cc=dianders@chromium.org \
--cc=drinkcat@chromium.org \
--cc=gpiccoli@canonical.com \
--cc=groeck@chromium.org \
--cc=jgg@ziepe.ca \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=vkoul@kernel.org \
--cc=will@kernel.org \
--cc=yangtiezhu@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox