RE: [PATCH] btrace: read entire aux buffer

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

I pushed this patch.

Regards,
Markus.

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Friday, October 28, 2016 12:46 PM
> To: gdb-patches@sourceware.org
> Subject: [PATCH] btrace: read entire aux buffer
> 
> The data_head of a perf event data buffer grows indefinitely.  Users are
> expected to compute data_head % data_size to find the location inside the perf
> event data buffer.
> 
> The aux_head of a perf event aux buffer wraps around and always stays within
> the
> perf event aux buffer.
> 
> Well, at least that's the behaviour for BTS and PT - where BTS uses the data
> buffer and PT the aux buffer.
> 
> GDB does not read beyond data_head or aux_head.  This is OK for BTS but wrong
> for PT.  It causes only a portion of the trace to be considered by GDB.  In the
> extreme case, the buffer may appear (almost) empty.
> 
> Thanks to Tim Wiederhake  <tim.wiederhake@intel.com> for reporting the
> anomaly.
> 
> Change it to read the entire aux buffer for PT.  The buffer is initially zero so
> any extra zeroes we read before aux_head wraps around the first time will be
> ignored when searching for the first PSB packet in order to synchronize onto the
> trace stream.
> 
> 2016-10-28  Markus Metzger  <markus.t.metzger@intel.com>
> 
> gdb/
> 	* nat/linux-btrace.c (perf_event_read): Allow data_head < size.
> 	* nat/linux-btrace.c (perf_event_read_all): Do not adjust size.
> 
> 
> ---
>  gdb/nat/linux-btrace.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
> index 5ac5db2..e4144c6 100644
> --- a/gdb/nat/linux-btrace.c
> +++ b/gdb/nat/linux-btrace.c
> @@ -119,10 +119,24 @@ perf_event_read (const struct perf_event_buffer
> *pev, __u64 data_head,
>    if (size == 0)
>      return NULL;
> 
> +  /* We should never ask for more data than the buffer can hold.  */
> +  buffer_size = pev->size;
> +  gdb_assert (size <= buffer_size);
> +
> +  /* If we ask for more data than we seem to have, we wrap around and read
> +     data from the end of the buffer.  This is already handled by the %
> +     BUFFER_SIZE operation, below.  Here, we just need to make sure that we
> +     don't underflow.
> +
> +     Note that this is perfectly OK for perf event buffers where data_head
> +     doesn'grow indefinitely and instead wraps around to remain within the
> +     buffer's boundaries.  */
> +  if (data_head < size)
> +    data_head += buffer_size;
> +
>    gdb_assert (size <= data_head);
>    data_tail = data_head - size;
> 
> -  buffer_size = pev->size;
>    begin = pev->mem;
>    start = begin + data_tail % buffer_size;
>    stop = begin + data_head % buffer_size;
> @@ -153,10 +167,7 @@ perf_event_read_all (struct perf_event_buffer *pev,
> gdb_byte **data,
>    __u64 data_head;
> 
>    data_head = *pev->data_head;
> -
>    size = pev->size;
> -  if (data_head < size)
> -    size = (size_t) data_head;
> 
>    *data = perf_event_read (pev, data_head, size);
>    *psize = size;
> --
> 1.8.3.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928