From: Peter Zijlstra <peterz@infradead.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
Andrew Morton <akpm@linux-foundation.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 21/21] tracing: Add optional percpu buffers for trace_printk()
Date: Fri, 23 Sep 2011 13:28:26 +0200 [thread overview]
Message-ID: <1316777306.9084.11.camel@twins> (raw)
In-Reply-To: <1316776605.29966.153.camel@gandalf.stny.rr.com>
On Fri, 2011-09-23 at 07:16 -0400, Steven Rostedt wrote:
> On Fri, 2011-09-23 at 13:07 +0200, Peter Zijlstra wrote:
> > On Fri, 2011-09-23 at 13:02 +0200, Peter Zijlstra wrote:
> > > On Thu, 2011-09-22 at 18:09 -0400, Steven Rostedt wrote:
> > > >
> > > > Currently, trace_printk() uses a single buffer to write into
> > > > to calculate the size and format needed to save the trace. To
> > > > do this safely in an SMP environment, a spin_lock() is taken
> > > > to only allow one writer at a time to the buffer. But this could
> > > > also affect what is being traced, and add synchronization that
> > > > would not be there otherwise.
> > >
> > > so trace_printk() isn't NMI safe? #$%@^%@@$%@
>
> It is NMI safe, always was (I use it there too). It has a percpu
> recursion detection (always has), thus if an NMI interrupts a current
> trace_printk(), the NMI trace_printk() will not print. I could add an
> NMI buffer to allow NMIs to print, but so far, we don't usually have
> issues with trace_printk(). Heck, I'm not sure printk() wont cause
> issues in NMIs. I think trace_printk() is still safer than printk.
Of course, printk() is most useless, its too slow, and its definitely
not NMI-safe.
If you've only got a single buffer, I don't see why you would need
per-cpu recursion detection, just spin_try_lock() the thing and if you
fail, bail. But that's not what the changelog said, or at least implied.
The possibility of dropping trace output like that is most worrying
though. I can imagine myself tearing my hair out trying to make sense of
a trace, and then wanting to kick you after finding out it lost the
crucial bit.
> > better to make all of trace_printk() depend on that extra config, there
> > is absolutely 0 point in having a broken and fully serialized trace
> > 'fail^wfeature'.
>
> Not, having per cpu buffers still doesn't allow NMIs to interrupt
> trace_printk(). Otherwise the NMI would just corrupt the current percpu
> buffer.
Multiple buffers sounds about right, not sure if you disable interrupts
over the normal trace path, but ISTR you don't, so you need
task/softirq/irq/nmi buffers per cpu.
Really loosing trace output is not an option, ever (aside from stuff
falling of the end of the buffer). Reliability first, performance
second.
next prev parent reply other threads:[~2011-09-23 11:28 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-22 22:09 [PATCH 00/21] [GIT PULL][v3.2] tracing: queued up stuff waiting for k.org to come back on line Steven Rostedt
2011-09-22 22:09 ` [PATCH 01/21] tracing: Clean up tb_fmt to not give faulty compile warning Steven Rostedt
2011-09-22 22:09 ` [PATCH 02/21] Tracepoint: Dissociate from module mutex Steven Rostedt
2011-09-22 22:09 ` [PATCH 03/21] x86: jump_label: arch_jump_label_text_poke_early: add missing __init Steven Rostedt
2011-09-22 22:09 ` [PATCH 04/21] tracing/filter: Use static allocation for filter predicates Steven Rostedt
2011-09-22 22:09 ` [PATCH 05/21] tracing/filter: Separate predicate init and filter addition Steven Rostedt
2011-09-22 22:09 ` [PATCH 06/21] tracing/filter: Remove field_name from filter_pred struct Steven Rostedt
2011-09-22 22:09 ` [PATCH 07/21] tracing/filter: Simplify tracepoint event lookup Steven Rostedt
2011-09-22 22:09 ` [PATCH 08/21] tracing/filter: Unify predicate tree walking, change check_pred_tree Steven Rostedt
2011-09-22 22:09 ` [PATCH 09/21] tracing/filter: Change count_leafs function to use walk_pred_tree Steven Rostedt
2011-09-22 22:09 ` [PATCH 10/21] tracing/filter: Change fold_pred_tree " Steven Rostedt
2011-09-22 22:09 ` [PATCH 11/21] tracing/filter: Change fold_pred " Steven Rostedt
2011-09-22 22:09 ` [PATCH 12/21] tracing/filter: Change filter_match_preds function to use Steven Rostedt
2011-09-22 22:09 ` [PATCH 13/21] tracing/filter: Add startup tests for events filter Steven Rostedt
2011-09-22 22:09 ` [PATCH 14/21] tracing: Add preempt disable for filter self test Steven Rostedt
2011-09-22 22:09 ` [PATCH 15/21] trace: Add a new readonly entry to report total buffer size Steven Rostedt
2011-09-22 22:09 ` [PATCH 16/21] trace: Add ring buffer stats to measure rate of events Steven Rostedt
2011-09-22 22:09 ` [PATCH 17/21] tracing: Add a counter clock for those that do not trust clocks Steven Rostedt
2011-09-22 22:09 ` [PATCH 18/21] tracing: Fix preemptirqsoff tracer to not stop at preempt off Steven Rostedt
2011-09-22 22:09 ` [PATCH 19/21] tracing: Account for preempt off in preempt_schedule() Steven Rostedt
2011-09-23 11:00 ` Peter Zijlstra
2011-09-23 11:19 ` Steven Rostedt
2011-09-23 11:22 ` Peter Zijlstra
2011-09-23 12:24 ` Steven Rostedt
2011-09-23 14:00 ` Peter Zijlstra
2011-09-23 14:07 ` Peter Zijlstra
2011-09-23 13:24 ` Peter Zijlstra
2011-09-22 22:09 ` [PATCH 20/21] tracing: Do not allocate buffer for trace_marker Steven Rostedt
2011-09-22 22:09 ` [PATCH 21/21] tracing: Add optional percpu buffers for trace_printk() Steven Rostedt
2011-09-23 11:02 ` Peter Zijlstra
2011-09-23 11:07 ` Peter Zijlstra
2011-09-23 11:16 ` Steven Rostedt
2011-09-23 11:28 ` Peter Zijlstra [this message]
2011-09-23 11:39 ` Steven Rostedt
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=1316777306.9084.11.camel@twins \
--to=peterz@infradead.org \
--cc=akpm@linux-foundation.org \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/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