lkml.org 
[lkml]   [2009]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] add binary printf
On Thu, Feb 26, 2009 at 02:02:43PM +0100, Ingo Molnar wrote:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
> > From: Lai Jiangshan <laijs@cn.fujitsu.com>
> >
> > Impact: Add APIs for binary trace printk infrastructure
> >
> > vbin_printf(): write args to binary buffer, string is copied
> > when "%s" is occurred.
> >
> > bstr_printf(): read from binary buffer for args and format a string
> >
> > [fweisbec@gmail.com: ported to latest -tip]
> >
> > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > ---
> > include/linux/string.h | 7 +
> > lib/Kconfig | 3 +
> > lib/vsprintf.c | 442 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 452 insertions(+), 0 deletions(-)
>
> OK, it's a nice idea and speedup for printf based tracing -
> which is common and convenient. Would you mind to post the
> performance measurements you've done using the new bstr_printf()
> facility? (the nanoseconds latency figures you did in the timer
> irq in a system under load and on a system that is idle)


Ok.


> The new printf code itself should be done cleaner i think and is
> not acceptable in its current form.
>
> These two new functions:
>
> > +#ifdef CONFIG_BINARY_PRINTF
> > +/*
> > + * bprintf service:
> > + * vbin_printf() - VA arguments to binary data
> > + * bstr_printf() - Binary data to text string
> > + */
>
> Duplicate hundreds of lines of code into three large functions
> (vsnprintf, vbin_printf, bstr_printf). These functions only have
> a difference in the way the argument list is iterated and the
> way the parsed result is stored:
>
> vsnprintf: iterates va_list, stores into string
> bstr_printf: iterates bin_buf, stores into string
> vbin_printf: iterates va_list, stores into bin_buf
>
> We should try _much_ harder at unifying these functions before
> giving up and duplicating them...
>
> An opaque in_buf/out_buf handle plus two helper function
> pointers passed in would be an obvious implementation.
>
> That way we'd have a single generic (inline) function that knows
> about the printf format itself:
>
> __generic_printf(void *in_buf,
> void *out_buf,
> void * (*read_in_buf)(void **),
> void * (*store_out_buf)(void **));
>
> And we'd have various variants for read_in_buf and
> store_out_buf. The generic function iterates the following way:
>
> in_val = read_in_buf(&in_buf);
> ...
> store_out_buf(&out_buf, in_val);
>
> (where in_val is wide enough to store a single argument.) The
> iterators modify the in_buf / out_buf pointers. Argument
> skipping can be done by reading the in-buf and not using it. I
> think we can do it with just two iterator methods.
>
> Or something like that - you get the idea. It can all be inlined
> so that we'd end up with essentially the same vsnprint()
> instruction sequence we have today.
>
> Ingo


Ok, I just looked deeply inside vsnprintf, and I don't think such
a generic interface would allow that. We need to know the size of the argument,
it's precision, width and flags.... And we need to know if we want to skip the non
format char.


What do you think of the following:

__ generic_printf(void *in,
void *out,
void *(*read_in)(void **buf, int size),
void *(store_char)(char *dst, char *end, char val, int field_width, int flags),
void *(*store_string)(char *dst, char *end, char *val, int field_width, int precision, int flags),
void *(*store_pointer)(char type, char *dst, char *end, void *val,
int field_width, int precision, int flags),
void *(*store_number)(char *dst, char *size, int base,int field_width, int precision, int flags),
bool skip_non_format
)


Well, something like that...

read_in can advance the pointer to the buffer itself (buf can be a va_args or u32 *)
and it returns a value, void * is generic for the type.

The storage functions are more specialized because of the interpretation of flags, precision...
So we can easily pass the usual string(), pointer(), .... that are already present in vsnprintf.c
or use custom ones.
They return the advanced dst pointer.

And at last, skip_non_format will decide if we want to consider non-format characters from fmt
to be copied as common %c characters or if we want to ignore them (useful for vbin_printf()).

Hmm?



\
 
 \ /
  Last update: 2009-02-26 18:07    [W:0.075 / U:1.708 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site