Re: [PATCH 2/4] perf intel-pt: Do not try to queue auxtrace data on pipe

[Adrian Hunter]

On 31/01/23 04:33, Namhyung Kim wrote:
> When it processes AUXTRACE_INFO, it calls to auxtrace_queue_data() to
> collect AUXTRACE data first.  That won't work with pipe since it needs
> lseek() to read the scattered aux data.
> 
>   $ perf record -o- -e intel_pt// true | perf report -i- --itrace=i100
>   # To display the perf.data header info, please use --header/--header-only options.
>   #
>   0x4118 [0xa0]: failed to process type: 70
>   Error:
>   failed to process sample
> 
> For the pipe mode, it can handle the aux data as it gets.  But there's
> no guarantee it can get the aux data in time.  So the following warning
> will be shown at the beginning:
> 
>   WARNING: Intel PT with pipe mode is not recommended.
>            The output cannot relied upon.  In particular,
>            time stamps and the order of events may be incorrect.
> 
> Fixes: dbd134322e74 ("perf intel-pt: Add support for decoding AUX area samples")
> Reviewed-by: James Clark <james.clark@xxxxxxx>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>

Minor changes to the documentation, see below, otherwise:

Reviewed-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>


> ---
>  tools/perf/Documentation/perf-intel-pt.txt | 30 ++++++++++++++++++++++
>  tools/perf/util/auxtrace.c                 |  3 +++
>  tools/perf/util/intel-pt.c                 |  6 +++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-intel-pt.txt b/tools/perf/Documentation/perf-intel-pt.txt
> index 7b6ccd2fa3bf..9d485a9cdb19 100644
> --- a/tools/perf/Documentation/perf-intel-pt.txt
> +++ b/tools/perf/Documentation/perf-intel-pt.txt
> @@ -1821,6 +1821,36 @@ a trace that encodes the payload data into TNT packets.  Here is an example
>   $
>  
>  
> +Pipe mode
> +---------
> +Pipe mode is a problem for Intel PT and possibly other auxtrace users.
> +It's not recommended to use a pipe as data output with Intel PT because
> +of the following reason.

'following reason' -> 'reason explained below'

> +
> +Essentially the auxtrace buffers do not behave like the regular perf
> +event buffers.  That is because the head and tail are updated by
> +software, but in the auxtrace case the data is written by hardware.
> +So the head and tail do not get updated as data is written.
> +
> +In the Intel PT case, the head and tail are updated only when the trace
> +is disabled by software, for example:

Needs a blank line here, otherwise all the text is merged into 1 paragraph.

> +    - full-trace, system wide : when buffer passes watermark
> +    - full-trace, not system-wide : when buffer passes watermark or
> +                                    context switches
> +    - snapshot mode : as above but also when a snapshot is made
> +    - sample mode : as above but also when a sample is made
> +
> +That means finished-round ordering doesn't work.  An auxtrace buffer
> +can turn up that has data that extends back in time, possibly to the
> +very beginning of tracing.
> +
> +For a perf.data file, that problem is solved by going through the trace
> +and queuing up the auxtrace buffers in advance.
> +
> +For pipe mode, the order of events and timestamps can presumably
> +be messed up.

'presumably be messed up' -> can be mixed up.

> +
> +
>  EXAMPLE
>  -------
>  
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index c2e323cd7d49..d4b04fa07a11 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -1133,6 +1133,9 @@ int auxtrace_queue_data(struct perf_session *session, bool samples, bool events)
>  	if (auxtrace__dont_decode(session))
>  		return 0;
>  
> +	if (perf_data__is_pipe(session->data))
> +		return 0;
> +
>  	if (!session->auxtrace || !session->auxtrace->queue_data)
>  		return -EINVAL;
>  
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index 6d3921627e33..b8b29756fbf1 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -4379,6 +4379,12 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
>  
>  	intel_pt_setup_pebs_events(pt);
>  
> +	if (perf_data__is_pipe(session->data)) {
> +		pr_warning("WARNING: Intel PT with pipe mode is not recommended.\n"
> +			   "         The output cannot relied upon.  In particular,\n"
> +			   "         timestamps and the order of events may be incorrect.\n");
> +	}
> +
>  	if (pt->sampling_mode || list_empty(&session->auxtrace_index))
>  		err = auxtrace_queue_data(session, true, true);
>  	else