[PATCH 0/3] tracing/ftrace: ftrace_bprintk

[PATCH 0/3] tracing/ftrace: ftrace_bprintk

[Frederic Weisbecker]

Hi,

These three patches are part of a patchset posted by Lai Jiangshan in december 2008.
They introduce a binary version of ftrace_printk() called ftrace_bprintk()

While having the same goal: print a generic message entry into the ring buffer,
their approaches are very different.

- ftrace_printk() does the formatting job on tracing time, insert the whole resulting string
  into the ring buffer, and then the string is printed on output time without a lot of modifications.

- ftrace_bprintk() does no formatting on tracing time. Instead, it looks at the format string
  to find the types and the numbers of the arguments and directly stores them as-is into the
  ring-buffer. Then the format string is stored into the ring-buffer too, but only by its address,
  it is not copied. Then on output time only, the final string is formatted and sent to the user.
  This gives a result about as fast as a traditional tracer with fixed fields types, except that
  we can print random types and numbers of fields here.


The first patch adds the generic support for binary formatting.
The second adds the support for binary print types on ftrace
and the last introduces ftrace_bprintk() which supports safely the modules
by listening on the module loading/unloading notifier to keep track of
unwanted freed format strings.

Lai Jiangshan (3):
      add binary printf
      ftrace: infrastructure for supporting binary record
      ftrace: add ftrace_bprintk()


 include/asm-generic/vmlinux.lds.h |    3 +
 include/linux/ftrace.h            |   25 ++
 include/linux/module.h            |    5 +
 include/linux/string.h            |    7 +
 kernel/module.c                   |    6 +
 kernel/trace/Kconfig              |    6 +
 kernel/trace/Makefile             |    1 +
 kernel/trace/trace.c              |   70 ++++++
 kernel/trace/trace.h              |   12 +
 kernel/trace/trace_bprintk.c      |  217 ++++++++++++++++++
 kernel/trace/trace_output.c       |   76 +++++++
 lib/Kconfig                       |    3 +
 lib/vsprintf.c                    |  442 +++++++++++++++++++++++++++++++++++++

Re: [PATCH 0/3] tracing/ftrace: ftrace_bprintk

[Jason Baron]

On Tue, Feb 24, 2009 at 06:16:18AM +0100, Frederic Weisbecker wrote:
> Hi,
> 
> These three patches are part of a patchset posted by Lai Jiangshan in december 2008.
> They introduce a binary version of ftrace_printk() called ftrace_bprintk()
> 
> While having the same goal: print a generic message entry into the ring buffer,
> their approaches are very different.
> 
> - ftrace_printk() does the formatting job on tracing time, insert the whole resulting string
>   into the ring buffer, and then the string is printed on output time without a lot of modifications.
> 
> - ftrace_bprintk() does no formatting on tracing time. Instead, it looks at the format string
>   to find the types and the numbers of the arguments and directly stores them as-is into the
>   ring-buffer. Then the format string is stored into the ring-buffer too, but only by its address,
>   it is not copied. Then on output time only, the final string is formatted and sent to the user.
>   This gives a result about as fast as a traditional tracer with fixed fields types, except that
>   we can print random types and numbers of fields here.
> 
> 
> The first patch adds the generic support for binary formatting.
> The second adds the support for binary print types on ftrace
> and the last introduces ftrace_bprintk() which supports safely the modules
> by listening on the module loading/unloading notifier to keep track of
> unwanted freed format strings.
> 
> Lai Jiangshan (3):
>       add binary printf
>       ftrace: infrastructure for supporting binary record
>       ftrace: add ftrace_bprintk()
> 

hi,

this seems like a really valuable feature....I'm just wondering about a
couple of things....

If the 'brpintk tracer' in trace/trace_bprintk.c is just being used to
set an enabled flag for printing out these binary records, then are we
better off with just an option flag in the 'trace_options' file? 

Second, can we somehow combine ftrace_printk() and ftrace_bprintk(), so
that a developer can just use one interface? Perhaps, ftrace_printk
calls ftrace_bprintk if binary option flag is set, otherwise, it just
outputs things normally.

thanks,

-Jason 

Re: [PATCH 0/3] tracing/ftrace: ftrace_bprintk

[Ingo Molnar]

* Jason Baron <jbaron@redhat.com> wrote:

> On Tue, Feb 24, 2009 at 06:16:18AM +0100, Frederic Weisbecker wrote:
> > Hi,
> > 
> > These three patches are part of a patchset posted by Lai Jiangshan in december 2008.
> > They introduce a binary version of ftrace_printk() called ftrace_bprintk()
> > 
> > While having the same goal: print a generic message entry into the ring buffer,
> > their approaches are very different.
> > 
> > - ftrace_printk() does the formatting job on tracing time, insert the whole resulting string
> >   into the ring buffer, and then the string is printed on output time without a lot of modifications.
> > 
> > - ftrace_bprintk() does no formatting on tracing time. Instead, it looks at the format string
> >   to find the types and the numbers of the arguments and directly stores them as-is into the
> >   ring-buffer. Then the format string is stored into the ring-buffer too, but only by its address,
> >   it is not copied. Then on output time only, the final string is formatted and sent to the user.
> >   This gives a result about as fast as a traditional tracer with fixed fields types, except that
> >   we can print random types and numbers of fields here.
> > 
> > 
> > The first patch adds the generic support for binary formatting.
> > The second adds the support for binary print types on ftrace
> > and the last introduces ftrace_bprintk() which supports safely the modules
> > by listening on the module loading/unloading notifier to keep track of
> > unwanted freed format strings.
> > 
> > Lai Jiangshan (3):
> >       add binary printf
> >       ftrace: infrastructure for supporting binary record
> >       ftrace: add ftrace_bprintk()
> > 
> 
> hi,
> 
> this seems like a really valuable feature....I'm just 
> wondering about a couple of things....
> 
> If the 'brpintk tracer' in trace/trace_bprintk.c is just being 
> used to set an enabled flag for printing out these binary 
> records, then are we better off with just an option flag in 
> the 'trace_options' file?
> 
> Second, can we somehow combine ftrace_printk() and 
> ftrace_bprintk(), so that a developer can just use one 
> interface? Perhaps, ftrace_printk calls ftrace_bprintk if 
> binary option flag is set, otherwise, it just outputs things 
> normally.

Well, ftrace_bprintk() seems to be a worthy and transparent 
replacement for ftrace_printk() to me. I.e. lets just use this 
as the new implementation for ftrace_printk().

Would there be any downsides of doing so? I dont see any.

	Ingo

Re: [PATCH 0/3] tracing/ftrace: ftrace_bprintk

[Frederic Weisbecker]

On Tue, Feb 24, 2009 at 06:25:30PM +0100, Ingo Molnar wrote:
> 
> * Jason Baron <jbaron@redhat.com> wrote:
> 
> > On Tue, Feb 24, 2009 at 06:16:18AM +0100, Frederic Weisbecker wrote:
> > > Hi,
> > > 
> > > These three patches are part of a patchset posted by Lai Jiangshan in december 2008.
> > > They introduce a binary version of ftrace_printk() called ftrace_bprintk()
> > > 
> > > While having the same goal: print a generic message entry into the ring buffer,
> > > their approaches are very different.
> > > 
> > > - ftrace_printk() does the formatting job on tracing time, insert the whole resulting string
> > >   into the ring buffer, and then the string is printed on output time without a lot of modifications.
> > > 
> > > - ftrace_bprintk() does no formatting on tracing time. Instead, it looks at the format string
> > >   to find the types and the numbers of the arguments and directly stores them as-is into the
> > >   ring-buffer. Then the format string is stored into the ring-buffer too, but only by its address,
> > >   it is not copied. Then on output time only, the final string is formatted and sent to the user.
> > >   This gives a result about as fast as a traditional tracer with fixed fields types, except that
> > >   we can print random types and numbers of fields here.
> > > 
> > > 
> > > The first patch adds the generic support for binary formatting.
> > > The second adds the support for binary print types on ftrace
> > > and the last introduces ftrace_bprintk() which supports safely the modules
> > > by listening on the module loading/unloading notifier to keep track of
> > > unwanted freed format strings.
> > > 
> > > Lai Jiangshan (3):
> > >       add binary printf
> > >       ftrace: infrastructure for supporting binary record
> > >       ftrace: add ftrace_bprintk()
> > > 
> > 
> > hi,
> > 
> > this seems like a really valuable feature....I'm just 
> > wondering about a couple of things....
> > 
> > If the 'brpintk tracer' in trace/trace_bprintk.c is just being 
> > used to set an enabled flag for printing out these binary 
> > records, then are we better off with just an option flag in 
> > the 'trace_options' file?
> > 
> > Second, can we somehow combine ftrace_printk() and 
> > ftrace_bprintk(), so that a developer can just use one 
> > interface? Perhaps, ftrace_printk calls ftrace_bprintk if 
> > binary option flag is set, otherwise, it just outputs things 
> > normally.
> 
> Well, ftrace_bprintk() seems to be a worthy and transparent 
> replacement for ftrace_printk() to me. I.e. lets just use this 
> as the new implementation for ftrace_printk().
> 
> Would there be any downsides of doing so? I dont see any.
> 
> 	Ingo


That's what I think too.
For now I posted Lai's patchset, but my purpose is to replace the old 
ftrace_printk by this new one. Once it is merged on -tip I'll start
iterating around to improve it and replace ftrace_printk.


> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] tracing/ftrace: ftrace_bprintk

[Frederic Weisbecker]

On Tue, Feb 24, 2009 at 12:17:18PM -0500, Jason Baron wrote:
> On Tue, Feb 24, 2009 at 06:16:18AM +0100, Frederic Weisbecker wrote:
> > Hi,
> > 
> > These three patches are part of a patchset posted by Lai Jiangshan in december 2008.
> > They introduce a binary version of ftrace_printk() called ftrace_bprintk()
> > 
> > While having the same goal: print a generic message entry into the ring buffer,
> > their approaches are very different.
> > 
> > - ftrace_printk() does the formatting job on tracing time, insert the whole resulting string
> >   into the ring buffer, and then the string is printed on output time without a lot of modifications.
> > 
> > - ftrace_bprintk() does no formatting on tracing time. Instead, it looks at the format string
> >   to find the types and the numbers of the arguments and directly stores them as-is into the
> >   ring-buffer. Then the format string is stored into the ring-buffer too, but only by its address,
> >   it is not copied. Then on output time only, the final string is formatted and sent to the user.
> >   This gives a result about as fast as a traditional tracer with fixed fields types, except that
> >   we can print random types and numbers of fields here.
> > 
> > 
> > The first patch adds the generic support for binary formatting.
> > The second adds the support for binary print types on ftrace
> > and the last introduces ftrace_bprintk() which supports safely the modules
> > by listening on the module loading/unloading notifier to keep track of
> > unwanted freed format strings.
> > 
> > Lai Jiangshan (3):
> >       add binary printf
> >       ftrace: infrastructure for supporting binary record
> >       ftrace: add ftrace_bprintk()
> > 
> 
> hi,
> 
> this seems like a really valuable feature....I'm just wondering about a
> couple of things....
> 
> If the 'brpintk tracer' in trace/trace_bprintk.c is just being used to
> set an enabled flag for printing out these binary records, then are we
> better off with just an option flag in the 'trace_options' file? 


This tracer does a bit more, it keeps track of the string formats used
by a module which uses ftrace_bprintk().
So for now, the use of this facility can't be decoupled from its tracer.
In fact, I will try to move this module safety into tracing core so that
any tracer will be able to use it.

 
> Second, can we somehow combine ftrace_printk() and ftrace_bprintk(), so
> that a developer can just use one interface? Perhaps, ftrace_printk
> calls ftrace_bprintk if binary option flag is set, otherwise, it just
> outputs things normally.
> 
> thanks,
> 
> -Jason 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>