public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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 15:43:31 -0400	[thread overview]
Message-ID: <20200628154331.2c69d43e@oasis.local.home> (raw)
In-Reply-To: <20200628192107.sa3ppfmxtgxh7sfs@ast-mbp.dhcp.thefacebook.com>

 On Sun, 28 Jun 2020 12:21:07 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> > Re-teach them, or are you finally admitting that the tracing system is
> > a permanent API?  This is the reason people are refusing to add trace
> > points into their subsystems. Because user space may make it required.
> > 
> > I see no reason why you can't create a dedicated BPF tracing instance
> > (you only need one) to add all your trace_array_printk()s to.  
> 
> All bpf helpers are stable api. We cannot remove bpf_trace_printk() and
> cannot change the fact that it has to print into /sys/kernel/debug/tracing/trace.

Then do a bpf trace event and enable it when a bpf_trace_printk() is
loaded. It will work the same for your users.

> If we do so a lot of users will complain. Loudly.
> If you really want to see the flames, go ahead and rename 'trace_pipe'
> into something else.

The layout of the tracefs system *is* a stable API. No argument there.

> This has nothing to do with tracing in general and tracepoints.
> Those come and go.

And in this case, trace_printk() is no different than any other trace
event. Obviously, your use case doesn't let it go. If some tool starts
relying on another trace event (say someone adds another bpf handler that
enables a trace event, and is documented) then under your scenario,
it's a stable API.

Hence, your "tracepoints come and go" is not universal, and there's no
telling which ones will end up being a stable API.


> If you really want to nuke trace_printk from the kernel we need time
> to work on replacement and give users at least few releases of helper
> deprecation time.

I never said I would nuke it. This patch in question makes it so those
that don't want that banner to ever show up can do so. A trace-printk()
is something to add via compiling. And since I and others use it
heavily for debugging, I would have this option not be a default, but
something that others can enable.

> We've never done in the past though.
> There could be flames even if we deprecate it gradually.
> Looking how unyielding you're about this banner I guess we have to start
> working on replacement sooner than later. Oh well.

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.

-- Steve

  reply	other threads:[~2020-06-28 19:43 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 [this message]
2020-06-28 22:02                   ` Alexei Starovoitov
2020-06-28 22:28                     ` Steven Rostedt
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=20200628154331.2c69d43e@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