Ingo,
The first patch is a critical fix that needs to get into 2.6.29.
The next patch is a rename of tracing_on to writing_enabled since
tracing_on is confusing, and the file enables or disables writes.
The last patch is a fix to tip that prevents a memory leak.
This version I added some clean ups that Andrew Morton suggested.
I know you did not pull the previous version, because I forgot
to push the changes to kernel.org ;-)
The following patches are in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: tip/devel
Frederic Weisbecker (1):
tracing/ftrace: fix a memory leak in stat tracing
Steven Rostedt (3):
ftrace: convert unsigned index to signed
ring-buffer: rename debugfs file tracing_on to writing_enabled
trace: clean up funny line breaks in stat_seq_show
----
kernel/trace/ftrace.c | 4 ++--
kernel/trace/ring_buffer.c | 5 +++--
kernel/trace/trace_stat.c | 40 ++++++++++++++++------------------------
3 files changed, 21 insertions(+), 28 deletions(-)
--
* Steven Rostedt <[email protected]> wrote:
> Ingo,
>
> The first patch is a critical fix that needs to get into 2.6.29.
>
> The next patch is a rename of tracing_on to writing_enabled since
> tracing_on is confusing, and the file enables or disables writes.
>
> The last patch is a fix to tip that prevents a memory leak.
>
> This version I added some clean ups that Andrew Morton suggested.
> I know you did not pull the previous version, because I forgot
> to push the changes to kernel.org ;-)
>
> The following patches are in:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
>
> branch: tip/devel
>
>
> Frederic Weisbecker (1):
> tracing/ftrace: fix a memory leak in stat tracing
>
> Steven Rostedt (3):
> ftrace: convert unsigned index to signed
> trace: clean up funny line breaks in stat_seq_show
pulled most of it, thanks Steve, except this bit:
> ring-buffer: rename debugfs file tracing_on to writing_enabled
writing_enabled is at least as confusing as tracing_on - if not more so.
The user really does not care about all the deeper machinery that happens
in ftrace - the difference between a 'light' disabling of a tracer and a
'heavy' disabling of a tracer (which means it unregisters itself
completely in essence).
To resolve this, we should probably hide this difference altogether (as i
have suggested to do many months ago, when this first came up), by
removing tracing_enabled.
A tracer can still be fully unregistered: by simply switching the current
tracer to the 'nop' tracer. tracing_on/off remains the lightweight version
that most users are interested in anyway.
Ingo
On Wed, 7 Jan 2009 10:49:05 +0100
Ingo Molnar <[email protected]> wrote:
> * Steven Rostedt <[email protected]> wrote:
>
> > ring-buffer: rename debugfs file tracing_on to writing_enabled
>
> writing_enabled is at least as confusing as tracing_on - if not more so.
>
> The user really does not care about all the deeper machinery that happens
> in ftrace - the difference between a 'light' disabling of a tracer and a
> 'heavy' disabling of a tracer (which means it unregisters itself
> completely in essence).
>
> To resolve this, we should probably hide this difference altogether (as i
> have suggested to do many months ago, when this first came up), by
> removing tracing_enabled.
>
> A tracer can still be fully unregistered: by simply switching the current
> tracer to the 'nop' tracer. tracing_on/off remains the lightweight version
> that most users are interested in anyway.
This sounds even better. We should not need tracing_enabled anymore, since
the buffer size can be changed regardless. tracing_on is more useful to
mmiotrace than tracing_enabled, too, since mmiotrace does not implement
the proper pausing behaviour on the tracing_enabled trigger.
But if Steven wants to keep the "pause" semantics, should there be a
callback dispatched to the tracer from tracing_on, just like there is
from tracing_enabled currently? Of course the callback semantics would
be different and the usual reaction would be to do nothing. It would
only be meaningful to tracers that update data outside the ring buffer,
so that they can properly pause.
--
Pekka Paalanen
http://www.iki.fi/pq/
On Wed, 7 Jan 2009, Pekka Paalanen wrote:
> On Wed, 7 Jan 2009 10:49:05 +0100
> Ingo Molnar <[email protected]> wrote:
>
> > * Steven Rostedt <[email protected]> wrote:
> >
> > > ring-buffer: rename debugfs file tracing_on to writing_enabled
> >
> > writing_enabled is at least as confusing as tracing_on - if not more so.
> >
> > The user really does not care about all the deeper machinery that happens
> > in ftrace - the difference between a 'light' disabling of a tracer and a
> > 'heavy' disabling of a tracer (which means it unregisters itself
> > completely in essence).
> >
> > To resolve this, we should probably hide this difference altogether (as i
> > have suggested to do many months ago, when this first came up), by
> > removing tracing_enabled.
> >
> > A tracer can still be fully unregistered: by simply switching the current
> > tracer to the 'nop' tracer. tracing_on/off remains the lightweight version
> > that most users are interested in anyway.
>
> This sounds even better. We should not need tracing_enabled anymore, since
> the buffer size can be changed regardless. tracing_on is more useful to
> mmiotrace than tracing_enabled, too, since mmiotrace does not implement
> the proper pausing behaviour on the tracing_enabled trigger.
>
> But if Steven wants to keep the "pause" semantics, should there be a
> callback dispatched to the tracer from tracing_on, just like there is
> from tracing_enabled currently? Of course the callback semantics would
> be different and the usual reaction would be to do nothing. It would
> only be meaningful to tracers that update data outside the ring buffer,
> so that they can properly pause.
The tracing_on is implemented by the ring buffer, and disables all ring
buffers, even those that are not part of ftrace. This file really has no
concept of a tracer. It simply stops writing to the ring buffer.
Things like the irq latency tracer is will still update its "max time"
when tracing is off, although the trace output will not be updated.
I could remove tracing_enabled, and move the tracing_on to trace.c,
that just calls the tracing_off of the ring buffer, and then the file will
be at the higher layer, and have a concept of the current tracer.
-- Steve
* Steven Rostedt <[email protected]> wrote:
> On Wed, 7 Jan 2009, Pekka Paalanen wrote:
>
> > On Wed, 7 Jan 2009 10:49:05 +0100
> > Ingo Molnar <[email protected]> wrote:
> >
> > > * Steven Rostedt <[email protected]> wrote:
> > >
> > > > ring-buffer: rename debugfs file tracing_on to writing_enabled
> > >
> > > writing_enabled is at least as confusing as tracing_on - if not more so.
> > >
> > > The user really does not care about all the deeper machinery that happens
> > > in ftrace - the difference between a 'light' disabling of a tracer and a
> > > 'heavy' disabling of a tracer (which means it unregisters itself
> > > completely in essence).
> > >
> > > To resolve this, we should probably hide this difference altogether (as i
> > > have suggested to do many months ago, when this first came up), by
> > > removing tracing_enabled.
> > >
> > > A tracer can still be fully unregistered: by simply switching the current
> > > tracer to the 'nop' tracer. tracing_on/off remains the lightweight version
> > > that most users are interested in anyway.
> >
> > This sounds even better. We should not need tracing_enabled anymore, since
> > the buffer size can be changed regardless. tracing_on is more useful to
> > mmiotrace than tracing_enabled, too, since mmiotrace does not implement
> > the proper pausing behaviour on the tracing_enabled trigger.
> >
> > But if Steven wants to keep the "pause" semantics, should there be a
> > callback dispatched to the tracer from tracing_on, just like there is
> > from tracing_enabled currently? Of course the callback semantics would
> > be different and the usual reaction would be to do nothing. It would
> > only be meaningful to tracers that update data outside the ring buffer,
> > so that they can properly pause.
>
> The tracing_on is implemented by the ring buffer, and disables all ring
> buffers, even those that are not part of ftrace. This file really has no
> concept of a tracer. It simply stops writing to the ring buffer.
>
> Things like the irq latency tracer is will still update its "max time"
> when tracing is off, although the trace output will not be updated.
>
> I could remove tracing_enabled, and move the tracing_on to trace.c, that
> just calls the tracing_off of the ring buffer, and then the file will be
> at the higher layer, and have a concept of the current tracer.
Sounds good to me. I think the highest utility is in the 'lightweight'
tracing_on toggle - even though it does not do as thorough of a job of
disabling a tracer as writing 0 to tracing_on.
I think we shouldnt even _try_ to provide a simple toggle for unregister:
it is clear from looking at /debug/tracing/current_tracer that the current
tracer is still active - so asking people to change that back to 'nop'
will be rather intuitive IMO.
Ingo