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

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