Re: [PATCH V2 6/6] perf tools: Properly set the value of 'old' and 'head' in snapshot mode

[Leo Yan <leo.yan@linaro.org>]

On Mon, May 20, 2019 at 01:53:29PM -0600, Mathieu Poirier wrote:

[...]

> > > +static bool cs_etm_buffer_has_wrapped(unsigned char *buffer,
> > > +                                   size_t buffer_size, u64 head)
> > > +{
> > > +     u64 i, watermark;
> > > +     u64 *buf = (u64 *)buffer;
> > > +     size_t buf_size = buffer_size;
> > > +
> > > +     /*
> > > +      * We want to look the very last 512 byte (chosen arbitrarily) in
> > > +      * the ring buffer.
> > > +      */
> > > +     watermark = buf_size - 512;
> > > +
> > > +     /*
> > > +      * @head is continuously increasing - if its value is equal or greater
> > > +      * than the size of the ring buffer, it has wrapped around.
> > > +      */
> > > +     if (head >= buffer_size)
> > > +             return true;
> > > +
> > > +     /*
> > > +      * The value of @head is somewhere within the size of the ring buffer.
> > > +      * This can be that there hasn't been enough data to fill the ring
> > > +      * buffer yet or the trace time was so long that @head has numerically
> > > +      * wrapped around.  To find we need to check if we have data at the very
> > > +      * end of the ring buffer.  We can reliably do this because mmap'ed
> > > +      * pages are zeroed out and there is a fresh mapping with every new
> > > +      * session.
> > > +      */
> > > +
> > > +     /* @head is less than 512 byte from the end of the ring buffer */
> > > +     if (head > watermark)
> > > +             watermark = head;
> > > +
> > > +     /*
> > > +      * Speed things up by using 64 bit transactions (see "u64 *buf" above)
> > > +      */
> > > +     watermark >>= 3;
> > > +     buf_size >>= 3;
> > > +
> > > +     /*
> > > +      * If we find trace data at the end of the ring buffer, @head has
> > > +      * been there and has numerically wrapped around at least once.
> > > +      */
> > > +     for (i = watermark; i < buf_size; i++)
> > > +             if (buf[i])
> >
> > I think here should be:
> >                 if (buf[i << 3])
> >
> 
> It would be if buf[] was a char *, but it is a u64 *.

You are right, I missed that.  Sorry for noise.

Thanks,
Leo Yan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel