From: Steven Rostedt <rostedt@goodmis.org>
To: Peter Zijlstra <peterz@infradead.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 07:39:33 -0400 [thread overview]
Message-ID: <1316777973.29966.163.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <1316777306.9084.11.camel@twins>
On Fri, 2011-09-23 at 13:28 +0200, Peter Zijlstra wrote:
> 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.
Not sure if the raw_spin_locks() had a try lock back when this was
written. The recursion detection was there from day one, and has nothing
to do with this patch. Which is why the changelog said nothing about it.
This patch only is about not having the trace_printk() cause extra
synchronization. This bit me once as trace_printk() cause a race to
disappear.
>
> 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.
Understood, but NMIs are always something to be worried about causing
strange issues.
>
> > > 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.
trace_printk() does disable interrupts, so it is only an NMI issue.
>
> Really loosing trace output is not an option, ever (aside from stuff
> falling of the end of the buffer). Reliability first, performance
> second.
>
Fine, then I'll add (on top of this patch) a CONFIG_TRACE_PRINTK_FULL or
some such name that will create the per cpu NMI and normal buffers,
which will not miss any interrupts. I just want to limit the unused
memory for 99.99999% of users.
I could make the TRACE_PRINTK_FULL not even disable interrupts and have
the normal/softirq/irq/nmi per cpu buffers. This will be something that
would *not* be enabled in production.
-- Steve
prev parent reply other threads:[~2011-09-23 11:39 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
2011-09-23 11:39 ` Steven Rostedt [this message]
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=1316777973.29966.163.camel@gandalf.stny.rr.com \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.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
all inboxes | Powered by JetHome®