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

Leo Yan
2019-05-21T01:55:19Z

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