All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Pekka Paalanen <pq@iki.fi>
Cc: "Frédéric Weisbecker" <fweisbec@gmail.com>,
	"Pekka J Enberg" <penberg@cs.helsinki.fi>,
	mingo@elte.hu, linux-kernel@vger.kernel.org,
	"Markus Metzger" <markus.t.metzger@gmail.com>
Subject: Re: ftrace behaviour (was: [PATCH] ftrace: introduce tracing_reset_online_cpus() helper)
Date: Fri, 19 Dec 2008 21:47:53 -0500 (EST)	[thread overview]
Message-ID: <alpine.DEB.1.10.0812192140350.29275@gandalf.stny.rr.com> (raw)
In-Reply-To: <20081220041759.5a026f5a@daedalus.pq.iki.fi>


On Sat, 20 Dec 2008, Pekka Paalanen wrote:
> > 
> > Note, this is the main reason why you need to check for NULL on return of 
> > a ring_buffer_lock_reserve. That will return NULL when the tracing_on is 0.
> > 
> > > The ring buffers are huge and eat a considerable chunk of precious RAM.
> > > How could distributors ever enable mmiotrace in their kernel
> > > configurations by default, if it eats lots of memory for nothing?
> > 
> > Hmm, good point. We could change the allocation to when it is first 
> > enabled. Something other than 'nop' being put into the current_tracer 
> > file.
> 
> I'm very much looking forward to this.

Unfortunately, this is not a trivial change. I'll have to look into it.

> 
> > > 
> > > If distributors do not enable mmiotrace by default, we are in a worse
> > > situation than with out-of-tree mmiotrace module (if it could work).
> > > Users need to reconfigure and recompile their kernels, which is
> > > something we wanted to avoid. This is the reality right now.
> > > 
> > > Unless you have an answer to this, I'd like to suggest we resurrect the
> > > "none" tracer. When "none" is the current tracer, there would be no
> > > buffers allocated at all, and you could request a new buffer size.
> > > "none" would be the default. Do you see any problems here?
> > > 
> > > AFAIK the "nop" tracer will not do, because it still allows text
> > > messages (markers) to be written, and hence the ring buffer must
> > > exist. Or am I wrong?
> > 
> > No, you are quite right. We could recreate the 'none' tracer again that
> > has no buffer. At boot up it would be the default tracer, unless something 
> > else changes that.
> 
> The "nop" having no buffers at boot would be enough, but this would be
> even better.

We would need something separate from nop, since nop can still record. If 
there is no buffers, its best not to record. Although, the ring buffers 
should just return NULL if the buffer is not allocated yet (I better make 
sure that is the case).

But just for accounting reasons, its best to have a "none" that does no 
tracing what-so-ever. This way, you can echo 'none' back into the current 
tracer, and have the buffers freed (?).


> 
> > > 
> > > > Now we have recently added /debug/tracing/tracing_on which can quickly 
> > > > stop tracing. I may be able to use that, and we can let the 
> > > > tracing_enable" reset it too.
> > > 
> > > Does this mean I have to implement yet another on/off hook?
> > 
> > Nope, the on/off hook is extremely fast, and the plugins do not even know 
> > when they happen. The on/off simply turns off writing to the ring buffer. 
> > The plugin functions will still be called, it is just that they will fail 
> > to write to the ring buffers. As stated above, the 
> > ring_buffer_reserve_lock will return a NULL.
> 
> Does this also increment the lost events counters?

Unfortunately no. The lost events are about overruns. But the calling 
tracer should easily be able to add these as well. They see the "NULL" 
returned from the function. This means it obviously did not record the 
event.  The overruns, on the otherhand, is not visible to the tracer, so 
the ring buffer keeps count.

> 
> > > IMHO it is starting to be confusing having all these
> > > current_tracer, tracing_enabled, tracing_on etc.
> 
> > 
> > The tracing_enabled is the way to start and stop a trace. I'm considering 
> > to implement Frederic's request to reset the buffer on enabled. This is 
> > quick but requires locks and mutexes to be taken. It calls a hook to the 
> > plugin because different plugins actually want to reset (the irq latency 
> > tracers reset with this).
> > 
> > The tracing_on is something that has been asked by developers to give a 
> > way to start and stop tracing fast as possible, with no mutexes or added 
> > locks. In fact, this option is local to the ring buffer code. Ftrace does 
> > not even use it directly.  It just a global flag to stop tracing. There's 
> > also an in-kernel equivalent tracing_on() and tracing_off(). This just 
> > sets or clears a global flag that will stop any more writes to the trace 
> > buffer.
> 
> Why not call tracing_on, say, logging_enabled?
> IMHO tracing_enabled vs. tracing_on is incomprehesible, but
> tracing_enabled vs. logging_enabled is a little more understandable.
> current_tracer is self-explanatory, and tracing_enabled used to be.

One of the hardest things about ftrace is coming up with good naming. This 
is not always so easy, as you can tell. I'm not sure "logging_enabled" is 
any better. That would confuse myself ;-)   The reason I chose "on" is 
because it is like an on/off switch, where as enabled/disabled is usually 
(in the kernel anyway) associated to nesting.

How about tracing_enabled_light? ;-)

-- Steve


  reply	other threads:[~2008-12-20  2:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-19 10:08 [PATCH] ftrace: introduce tracing_reset_online_cpus() helper Pekka J Enberg
2008-12-19 10:47 ` Frédéric Weisbecker
2008-12-19 17:20   ` Steven Rostedt
2008-12-19 22:05     ` Ingo Molnar
2008-12-19 22:44     ` ftrace behaviour (was: [PATCH] ftrace: introduce tracing_reset_online_cpus() helper) Pekka Paalanen
2008-12-19 22:57       ` Ingo Molnar
2008-12-19 23:27         ` Steven Rostedt
2008-12-19 23:34           ` Ingo Molnar
2008-12-20  0:29             ` Steven Rostedt
2008-12-20  1:38               ` Pekka Paalanen
2008-12-20  1:46                 ` Steven Rostedt
2008-12-20  2:32                   ` Pekka Paalanen
2008-12-20  2:56                     ` Steven Rostedt
2008-12-23 17:29                 ` Steven Rostedt
2008-12-20  0:03       ` Steven Rostedt
2008-12-20  2:17         ` Pekka Paalanen
2008-12-20  2:47           ` Steven Rostedt [this message]
2008-12-31 13:53             ` Pekka Paalanen
2008-12-31 18:33               ` Steven Rostedt
2008-12-31 18:57                 ` Pekka Paalanen
2008-12-31 19:06                   ` Steven Rostedt
2008-12-31 22:08                     ` Pekka Paalanen
2008-12-20 14:15         ` Frédéric Weisbecker
2008-12-20 13:15     ` [PATCH] ftrace: introduce tracing_reset_online_cpus() helper Frédéric Weisbecker
2008-12-19 15:30 ` Ingo Molnar

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=alpine.DEB.1.10.0812192140350.29275@gandalf.stny.rr.com \
    --to=rostedt@goodmis.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus.t.metzger@gmail.com \
    --cc=mingo@elte.hu \
    --cc=penberg@cs.helsinki.fi \
    --cc=pq@iki.fi \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.