Re: [PATCH v2] printk: add console output tracing

[Frederic Weisbecker]

On Thu, Nov 24, 2011 at 02:21:59PM +0100, Johannes Berg wrote:
> On Wed, 2011-11-23 at 14:16 +0100, Frederic Weisbecker wrote:
> 
> > > Also, call_console_drivers() doesn't pass this start/end through to
> > > _call_console_drivers(), it has loops and stuff there...
> > 
> > Yeah but looking at the loop there, start and end passed to
> > __call_console_drivers() are bounded between start and end passed to
> > call_console_drivers().
> 
> Looks like. Not that I have any idea why other code handles the wrap
> then?

It's not the same wrap.
More on that below.

> 
> > > In any case -- feel free to clean it all up, I basically copied the
> > > logic from _call_console_drivers assuming it was needed.
> > 
> > I'm just reviewing and suggesting a way to keep the patch more simple.
> > Outside that, I don't really need that patch myself ;)
> 
> :-)
> 
> > > I don't really want to know about the printk details :-)
> > 
> > Well, it's like working out outside in winter. It may not sound
> > very entertaining in the beginning but then it quickly becomes
> > invigorating, toning and good for the mood...I think...
> 
> Heh....
> 
> I don't really feel comfortable modifying the _call_console_drivers()
> function to not handle start > end (modulo log buf size of course), but
> at the same time I don't feel comfortable putting code into it that
> doesn't handle it.

So, the:

	BUG_ON(((int)(start - end)) > 0);

check is there to ensure we haven't wrapped INT_MAX. If we have reached
that point it definetly means we have a bug because log_buf_len is itself
an int and we shouldn't overlap INT_MAX.

The care on the wrapping that is done in _call_console_drivers() is
different and concerns log_buf_len itself. If log_buf_len = 8, start = 7
and end = 9, then you will enter the "((start & LOG_BUF_MASK) > (end & LOG_BUF_MASK))"
condition that handle the wrap on LOG_BUF_MASK to print the two chars.
But this is totally different from "start > end" which would mean we have
a bug.

So, in your tracepoint you can safely use "end - start" as a length for your
dynamic array. But the rest of your tracepoint (all the fast assign part)
still needs the masks as you did.

But well, I'm being anal about two lines in TP_struct__entry(), that's really
not important...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/