Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Alexei Starovoitov <ast@fb.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Andy Lutomirski <luto@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	"LSM List" <linux-security-module@vger.kernel.org>,
	James Morris <jmorris@namei.org>, Jann Horn <jannh@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Network Development <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>, Kernel Team <Kernel-team@fb.com>,
	Linux API <linux-api@vger.kernel.org>
Subject: Re: trace_printk issue. Was: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
Date: Fri, 4 Oct 2019 19:56:04 +0000
Message-ID: <36f0efac-d6b6-9439-d4c6-361d84cb3429@fb.com> (raw)
In-Reply-To: <20191003124148.4b94a720@gandalf.local.home>

On 10/3/19 9:41 AM, Steven Rostedt wrote:
> On Thu, 3 Oct 2019 09:18:40 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> I think dropping last events is just as bad. Is there a mode to overwrite old
>> and keep the last N (like perf does) ?
> 
> Well, it drops it by pages. Thus you should always have the last page
> of events.
> 
>> Peter Wu brought this issue to my attention in
>> commit 55c33dfbeb83 ("bpf: clarify when bpf_trace_printk discards lines").
>> And later sent similar doc fix to ftrace.rst.
> 
> It was documented there, he just elaborated on it more:
> 
>          This file holds the output of the trace in a human
>          readable format (described below). Note, tracing is temporarily
> -       disabled while this file is being read (opened).
> +       disabled when the file is open for reading. Once all readers
> +       are closed, tracing is re-enabled.
> 
> 
>> To be honest if I knew of this trace_printk quirk I would not have picked it
>> as a debugging mechanism for bpf.
>> I urge you to fix it.
> 
> It's not a trivial fix by far.
> 
> Note, trying to read the trace file without disabling the writes to it,
> will most likely make reading it when function tracing enabled totally
> garbage, as the buffer will most likely be filled for every read event.
> That is, each read event will not be related to the next event that is
> read, making it very confusing.
> 
> Although, I may be able to make it work per page. That way you get at
> least a page worth of events.

That sounds much better. As long as trace_printk() doesn't disappear
into the void, it's good.

But the part I'm not getting is why trace_printk() has
if (tracing_disabled) goto out;

It's a concurrent ring buffer. One cpu can write into it while
another reading. What is the point disabling trace_printk in particular?
Each __buffer_unlock_commit is an atomic ring buffer update,
so read from trace will either see it as a whole or won't see it.
'trace_pipe' clearly works fine. Why 'trace' is any different?
Just keep tracing enabled and keep reading it until the end of current
ring buffer. Whether open() determines current or it reads until next=0
is an implementation detail.

  reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190827205213.456318-1-ast@kernel.org>
2019-08-27 23:01 ` Andy Lutomirski
2019-08-27 23:21   ` Steven Rostedt
2019-08-27 23:34     ` Andy Lutomirski
2019-08-28  0:44       ` Steven Rostedt
2019-08-28  1:12         ` Andy Lutomirski
2019-08-28  2:22           ` Steven Rostedt
2019-08-28  0:38     ` Alexei Starovoitov
2019-08-28  3:30     ` Masami Hiramatsu
2019-08-28  4:47       ` Alexei Starovoitov
2019-08-28  0:34   ` Alexei Starovoitov
2019-08-28  0:55     ` Andy Lutomirski
2019-08-28  2:00       ` Andy Lutomirski
2019-08-28  4:49         ` Alexei Starovoitov
2019-08-28  6:20           ` Andy Lutomirski
2019-08-28 23:38             ` Alexei Starovoitov
2019-08-29  0:58               ` Andy Lutomirski
2019-08-28  4:43       ` Alexei Starovoitov
2019-08-28  6:12         ` Andy Lutomirski
2019-08-28 22:55           ` Alexei Starovoitov
2019-08-29  0:45             ` Andy Lutomirski
2019-08-29  0:53               ` Andy Lutomirski
2019-08-29  4:07               ` Alexei Starovoitov
2019-09-28 23:37                 ` Steven Rostedt
2019-09-30 18:31                   ` Kees Cook
2019-10-01  1:22                     ` Alexei Starovoitov
2019-10-01 22:10                       ` Steven Rostedt
2019-10-01 22:18                         ` Alexei Starovoitov
2019-10-01 22:47                           ` Steven Rostedt
2019-10-02 17:18                             ` Alexei Starovoitov
2019-10-02 23:00                               ` Steven Rostedt
2019-10-03 16:18                                 ` trace_printk issue. Was: " Alexei Starovoitov
2019-10-03 16:41                                   ` Steven Rostedt
2019-10-04 19:56                                     ` Alexei Starovoitov [this message]
2019-10-03  6:12                     ` Masami Hiramatsu
2019-10-03 16:20                       ` Alexei Starovoitov
2019-08-28  7:14   ` Peter Zijlstra
2019-08-28 22:08     ` Alexei Starovoitov
2019-08-29 13:34       ` Steven Rostedt
2019-08-29 15:43         ` Andy Lutomirski
2019-08-29 17:23           ` Alexei Starovoitov
2019-08-29 17:36             ` Andy Lutomirski
2019-08-29 17:49             ` Steven Rostedt
2019-08-29 17:19         ` Alexei Starovoitov
2019-08-29 17:47           ` Steven Rostedt

Reply instructions:

You may reply publically 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=36f0efac-d6b6-9439-d4c6-361d84cb3429@fb.com \
    --to=ast@fb.com \
    --cc=Kernel-team@fb.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    /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

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git