Re: [PATCH] ring-buffer: fix uninitialized read_stamp

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 2012-06-26 at 17:35 -0700, David Sharp wrote:
> On Fri, Jun 22, 2012 at 6:27 PM, David Sharp <dhsharp@google.com> wrote:
> > On Fri, Jun 22, 2012 at 4:31 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >>
> >> Now I think you may understand my patch.
> >
> > Yeah, mostly. At least enough that I think it's worth testing. But Monday.
> 
> I got around to testing your patch today, and it fixes the issue. No
> bad-looking timestamps in 40 runs, whereas could reproduce within 3
> runs before. Do you want me to send you a fresh patch, or just use the
> one you have?

I rather use this one for two reasons.

1) it limits the places where read_stamp is updated. And I rather not
add an update because "it fixes an anomaly".

2) I think it is wrong to force the writer on the reader page when no
write has been made. There's some side effects that this causes. One is
that if you do a read with no write, and then do nothing, it forces the
writer on that page. Now if a lot of writes happen (function tracing),
the writes that were on the reader page are never overwritten when the
buffer is full. Then you get a page of very old data, followed by a
buffer full of new data.

-- Steve

> 
> >>> On Thu, Jun 21, 2012 at 8:56 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >>> > Does something like this work for you. Note, this is totally untested!
> >>> >
> >>> > -- Steve
> >>> >
> >>> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> >>> > index ad0239b..5943044 100644
> >>> > --- a/kernel/trace/ring_buffer.c
> >>> > +++ b/kernel/trace/ring_buffer.c
> >>> > @@ -3246,6 +3246,10 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
> >>> >        if (cpu_buffer->commit_page == cpu_buffer->reader_page)
> >>> >                goto out;
> >>> >
> >>> > +       /* Don't bother swapping if the ring buffer is empty */
> >>> > +       if (rb_num_of_entries(cpu_buffer) == 0)
> >>> > +               goto out;
> >>> > +
> >>> >        /*
> >>> >         * Reset the reader page to size zero.
> >>> >         */