[0/4] perf intel-pt: Fix the pipe mode (v1)

  Hello,

I found some problems in Intel-PT and auxtrace in general with pipe.
In the past it used to work with pipe, but recent code fails.  As it
also touches the generic code, other auxtrace users like ARM SPE will
be affected too.  I added a test case to verify it works with pipes.

At last, I can run this command without a problem.

  $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000

The code is available at 'perf/auxtrace-pipe-v1' branch in

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung

Namhyung Kim (4):
  perf inject: Use perf_data__read() for auxtrace
  perf intel-pt: Do not try to queue auxtrace data on pipe
  perf session: Avoid calling lseek(2) for pipe
  perf test: Add pipe mode test to the Intel PT test suite

 tools/perf/builtin-inject.c             |  6 +++---
 tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
 tools/perf/util/auxtrace.c              |  3 +++
 tools/perf/util/session.c               |  9 +++++++--
 4 files changed, 30 insertions(+), 5 deletions(-)


base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
  

On 27/01/23 02:19, Namhyung Kim wrote:
> Hello,
> 
> I found some problems in Intel-PT and auxtrace in general with pipe.
> In the past it used to work with pipe, but recent code fails.

Pipe mode is a problem for Intel PT and possibly other auxtrace users.
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:
    - 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.

For Intel PT, it is a bit of a surprise that there is not 
validation to error out in pipe mode.

At the least, a warning is needed, and the above explanation needs
to be added to the documentation.

>                                                                As it
> also touches the generic code, other auxtrace users like ARM SPE will
> be affected too.  I added a test case to verify it works with pipes.
> 
> At last, I can run this command without a problem.
> 
>   $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000
> 
> The code is available at 'perf/auxtrace-pipe-v1' branch in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks,
> Namhyung
> 
> Namhyung Kim (4):
>   perf inject: Use perf_data__read() for auxtrace
>   perf intel-pt: Do not try to queue auxtrace data on pipe
>   perf session: Avoid calling lseek(2) for pipe
>   perf test: Add pipe mode test to the Intel PT test suite
> 
>  tools/perf/builtin-inject.c             |  6 +++---
>  tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
>  tools/perf/util/auxtrace.c              |  3 +++
>  tools/perf/util/session.c               |  9 +++++++--
>  4 files changed, 30 insertions(+), 5 deletions(-)
> 
> 
> base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
> prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
  

On 27/01/2023 07:22, Adrian Hunter wrote:
> On 27/01/23 02:19, Namhyung Kim wrote:
>> Hello,
>>
>> I found some problems in Intel-PT and auxtrace in general with pipe.
>> In the past it used to work with pipe, but recent code fails.
> 
> Pipe mode is a problem for Intel PT and possibly other auxtrace users.

Just some info from my side: For Arm Coresight we ended up deprecating
pipe mode, then not supporting it altogether. First was when we added an
optional step to peek through all of the data to help with an edge case.
Then we added a requirement to receive a HW_ID packet before decoding
which necessitated the peek. You can't peek in pipe mode because you
need to be able to seek, so it's not supported at all anymore.

For Arm SPE I never tested it with piped data. I suppose I could add a
test at some point, but I don't really see the usecase.

James

> 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:
>     - 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.
> 
> For Intel PT, it is a bit of a surprise that there is not 
> validation to error out in pipe mode.
> 
> At the least, a warning is needed, and the above explanation needs
> to be added to the documentation.
> 
>>                                                                As it
>> also touches the generic code, other auxtrace users like ARM SPE will
>> be affected too.  I added a test case to verify it works with pipes.
>>
>> At last, I can run this command without a problem.
>>
>>   $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000
>>
>> The code is available at 'perf/auxtrace-pipe-v1' branch in
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>
>> Thanks,
>> Namhyung
>>
>> Namhyung Kim (4):
>>   perf inject: Use perf_data__read() for auxtrace
>>   perf intel-pt: Do not try to queue auxtrace data on pipe
>>   perf session: Avoid calling lseek(2) for pipe
>>   perf test: Add pipe mode test to the Intel PT test suite
>>
>>  tools/perf/builtin-inject.c             |  6 +++---
>>  tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
>>  tools/perf/util/auxtrace.c              |  3 +++
>>  tools/perf/util/session.c               |  9 +++++++--
>>  4 files changed, 30 insertions(+), 5 deletions(-)
>>
>>
>> base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
>> prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
>
  

On 27/01/2023 00:19, Namhyung Kim wrote:
> Hello,
> 
> I found some problems in Intel-PT and auxtrace in general with pipe.
> In the past it used to work with pipe, but recent code fails.  As it
> also touches the generic code, other auxtrace users like ARM SPE will
> be affected too.  I added a test case to verify it works with pipes.
> 
> At last, I can run this command without a problem.
> 
>   $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000
> 
> The code is available at 'perf/auxtrace-pipe-v1' branch in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks,
> Namhyung
> 
> Namhyung Kim (4):
>   perf inject: Use perf_data__read() for auxtrace
>   perf intel-pt: Do not try to queue auxtrace data on pipe
>   perf session: Avoid calling lseek(2) for pipe
>   perf test: Add pipe mode test to the Intel PT test suite
> 
>  tools/perf/builtin-inject.c             |  6 +++---
>  tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
>  tools/perf/util/auxtrace.c              |  3 +++
>  tools/perf/util/session.c               |  9 +++++++--
>  4 files changed, 30 insertions(+), 5 deletions(-)
> 
> 
For the whole set:

Reviewed-by: James Clark <james.clark@arm.com>

> base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
> prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
  

Hi Adrian,

On Thu, Jan 26, 2023 at 11:22 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 27/01/23 02:19, Namhyung Kim wrote:
> > Hello,
> >
> > I found some problems in Intel-PT and auxtrace in general with pipe.
> > In the past it used to work with pipe, but recent code fails.
>
> Pipe mode is a problem for Intel PT and possibly other auxtrace users.
> 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:
>     - 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.

Ok, IIUC we want to process the main buffer and auxtrace buffer
together in time order but there's no guarantee to get the auxtrace
data in time, right?

I wonder if it's possible to use 2 pass processing for pipe mode.
We may keep the events in the ordered queue and auxtrace queue
in the first pass, and process together from the beginning in the
second pass. But I guess the data size would be a problem.

Or, assuming that the auxtrace buffer comes later than (or equal to)
the main buffer, we may start processing the main buffer as soon as
every auxtrace queue gets some data.  Thoughts?

>
> 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.
>
> For Intel PT, it is a bit of a surprise that there is not
> validation to error out in pipe mode.

What kind of validation do you have in mind?  Checking pid/tid?

>
> At the least, a warning is needed, and the above explanation needs
> to be added to the documentation.

Thanks, I'll add it to the documentation.

How about showing something like this for pipe mode?

  WARNING: Intel-PT with pipe mode may not work correctly.

Thanks,
Namhyung


>
> >                                                                As it
> > also touches the generic code, other auxtrace users like ARM SPE will
> > be affected too.  I added a test case to verify it works with pipes.
> >
> > At last, I can run this command without a problem.
> >
> >   $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000
> >
> > The code is available at 'perf/auxtrace-pipe-v1' branch in
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >
> > Thanks,
> > Namhyung
> >
> > Namhyung Kim (4):
> >   perf inject: Use perf_data__read() for auxtrace
> >   perf intel-pt: Do not try to queue auxtrace data on pipe
> >   perf session: Avoid calling lseek(2) for pipe
> >   perf test: Add pipe mode test to the Intel PT test suite
> >
> >  tools/perf/builtin-inject.c             |  6 +++---
> >  tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
> >  tools/perf/util/auxtrace.c              |  3 +++
> >  tools/perf/util/session.c               |  9 +++++++--
> >  4 files changed, 30 insertions(+), 5 deletions(-)
> >
> >
> > base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
> > prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
>
  

Hi James,

On Fri, Jan 27, 2023 at 6:42 AM James Clark <james.clark@arm.com> wrote:
>
>
>
> On 27/01/2023 07:22, Adrian Hunter wrote:
> > On 27/01/23 02:19, Namhyung Kim wrote:
> >> Hello,
> >>
> >> I found some problems in Intel-PT and auxtrace in general with pipe.
> >> In the past it used to work with pipe, but recent code fails.
> >
> > Pipe mode is a problem for Intel PT and possibly other auxtrace users.
>
> Just some info from my side: For Arm Coresight we ended up deprecating
> pipe mode, then not supporting it altogether. First was when we added an
> optional step to peek through all of the data to help with an edge case.
> Then we added a requirement to receive a HW_ID packet before decoding
> which necessitated the peek. You can't peek in pipe mode because you
> need to be able to seek, so it's not supported at all anymore.
>
> For Arm SPE I never tested it with piped data. I suppose I could add a
> test at some point, but I don't really see the usecase.

Yeah, it'd be great if we can have a test for Arm SPE.

Anyway, my work env (Google) requires the pipe mode due to the
restriction in disk usage.  Without the pipe support, it's not possible
to run `perf record` in production.

Thanks,
Namhyung
  

On 27/01/2023 23:08, Namhyung Kim wrote:
> Hi James,
> 
> On Fri, Jan 27, 2023 at 6:42 AM James Clark <james.clark@arm.com> wrote:
>>
>>
>>
>> On 27/01/2023 07:22, Adrian Hunter wrote:
>>> On 27/01/23 02:19, Namhyung Kim wrote:
>>>> Hello,
>>>>
>>>> I found some problems in Intel-PT and auxtrace in general with pipe.
>>>> In the past it used to work with pipe, but recent code fails.
>>>
>>> Pipe mode is a problem for Intel PT and possibly other auxtrace users.
>>
>> Just some info from my side: For Arm Coresight we ended up deprecating
>> pipe mode, then not supporting it altogether. First was when we added an
>> optional step to peek through all of the data to help with an edge case.
>> Then we added a requirement to receive a HW_ID packet before decoding
>> which necessitated the peek. You can't peek in pipe mode because you
>> need to be able to seek, so it's not supported at all anymore.
>>
>> For Arm SPE I never tested it with piped data. I suppose I could add a
>> test at some point, but I don't really see the usecase.
> 
> Yeah, it'd be great if we can have a test for Arm SPE.
> 

Ok thanks I will put it on the list of things to do.

> Anyway, my work env (Google) requires the pipe mode due to the
> restriction in disk usage.  Without the pipe support, it's not possible
> to run `perf record` in production.
> 

Makes sense. Unfortunately at the moment with Coresight, because of the
lack of appropriate timestamps we're waiting for the end of the file
before starting decoding. So you're not really any better off using
piped mode, unless you have a lot more memory than disk space?

Since this commit [1] and Arm v8.4 we can actually start making use of
the timestamps and do a streaming decode again. So I will also add it to
the list to look into that for Coresight again. Are you using an old
version of Perf or not using Coresight at all? I know Denis at Google is
using Coresight, but only with files rather than pipes.

One other thing, have you used the --switch-output mode to perf record
before? I would have said it would give you some of the benefits of
piped mode, but is more likely to work with Coresight. But last time I
checked it's not working either. Not very helpful I know, but something
to keep in mind.

James

[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=a7fe9a443b6064c68f86a2ee09bdfa7736660ef3
  

Em Fri, Jan 27, 2023 at 02:54:36PM -0800, Namhyung Kim escreveu:
> Hi Adrian,
> 
> On Thu, Jan 26, 2023 at 11:22 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >
> > On 27/01/23 02:19, Namhyung Kim wrote:
> > > Hello,
> > >
> > > I found some problems in Intel-PT and auxtrace in general with pipe.
> > > In the past it used to work with pipe, but recent code fails.
> >
> > Pipe mode is a problem for Intel PT and possibly other auxtrace users.
> > 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:
> >     - 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.
> 
> Ok, IIUC we want to process the main buffer and auxtrace buffer
> together in time order but there's no guarantee to get the auxtrace
> data in time, right?
> 
> I wonder if it's possible to use 2 pass processing for pipe mode.
> We may keep the events in the ordered queue and auxtrace queue
> in the first pass, and process together from the beginning in the
> second pass. But I guess the data size would be a problem.
> 
> Or, assuming that the auxtrace buffer comes later than (or equal to)
> the main buffer, we may start processing the main buffer as soon as
> every auxtrace queue gets some data.  Thoughts?
> 
> >
> > 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.
> >
> > For Intel PT, it is a bit of a surprise that there is not
> > validation to error out in pipe mode.
> 
> What kind of validation do you have in mind?  Checking pid/tid?
> 
> >
> > At the least, a warning is needed, and the above explanation needs
> > to be added to the documentation.
> 
> Thanks, I'll add it to the documentation.

Ok, so I'll wait for v2 of this patch series, Adrian, apart from what
you mentioned, are you ok with the patches, or a subset of them? The
first ones looks ok, right?

- Arnaldo
 
> How about showing something like this for pipe mode?
> 
>   WARNING: Intel-PT with pipe mode may not work correctly.
> 
> Thanks,
> Namhyung
> 
> 
> >
> > >                                                                As it
> > > also touches the generic code, other auxtrace users like ARM SPE will
> > > be affected too.  I added a test case to verify it works with pipes.
> > >
> > > At last, I can run this command without a problem.
> > >
> > >   $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000
> > >
> > > The code is available at 'perf/auxtrace-pipe-v1' branch in
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >
> > > Thanks,
> > > Namhyung
> > >
> > > Namhyung Kim (4):
> > >   perf inject: Use perf_data__read() for auxtrace
> > >   perf intel-pt: Do not try to queue auxtrace data on pipe
> > >   perf session: Avoid calling lseek(2) for pipe
> > >   perf test: Add pipe mode test to the Intel PT test suite
> > >
> > >  tools/perf/builtin-inject.c             |  6 +++---
> > >  tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
> > >  tools/perf/util/auxtrace.c              |  3 +++
> > >  tools/perf/util/session.c               |  9 +++++++--
> > >  4 files changed, 30 insertions(+), 5 deletions(-)
> > >
> > >
> > > base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
> > > prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
> >
  

On 30/01/23 16:15, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jan 27, 2023 at 02:54:36PM -0800, Namhyung Kim escreveu:
>> Hi Adrian,
>>
>> On Thu, Jan 26, 2023 at 11:22 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>
>>> On 27/01/23 02:19, Namhyung Kim wrote:
>>>> Hello,
>>>>
>>>> I found some problems in Intel-PT and auxtrace in general with pipe.
>>>> In the past it used to work with pipe, but recent code fails.
>>>
>>> Pipe mode is a problem for Intel PT and possibly other auxtrace users.
>>> 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:
>>>     - 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.
>>
>> Ok, IIUC we want to process the main buffer and auxtrace buffer
>> together in time order but there's no guarantee to get the auxtrace
>> data in time, right?

Yes

>>
>> I wonder if it's possible to use 2 pass processing for pipe mode.
>> We may keep the events in the ordered queue and auxtrace queue
>> in the first pass, and process together from the beginning in the
>> second pass. But I guess the data size would be a problem.
>>
>> Or, assuming that the auxtrace buffer comes later than (or equal to)
>> the main buffer, we may start processing the main buffer as soon as
>> every auxtrace queue gets some data.  Thoughts?

That sounds like it would require figuring out a timestamp up to
which there is Intel PT trace data in all queues.  That would
be very complicated.

>>
>>>
>>> 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.
>>>
>>> For Intel PT, it is a bit of a surprise that there is not
>>> validation to error out in pipe mode.
>>
>> What kind of validation do you have in mind?  Checking pid/tid?

Validation to kill pipe mode for Intel PT entirely.  But a warning
is ok.

>>
>>>
>>> At the least, a warning is needed, and the above explanation needs
>>> to be added to the documentation.
>>
>> Thanks, I'll add it to the documentation.
> 
> Ok, so I'll wait for v2 of this patch series, Adrian, apart from what
> you mentioned, are you ok with the patches, or a subset of them? The
> first ones looks ok, right?

Yes they are ok.

> 
> - Arnaldo
>  
>> How about showing something like this for pipe mode?
>>
>>   WARNING: Intel-PT with pipe mode may not work correctly.

Perhaps:

WARNING: Intel PT with pipe mode is not recommended. The output cannot be relied upon. In particular, time stamps and the order of events may be incorrect.

>>
>> Thanks,
>> Namhyung
>>
>>
>>>
>>>>                                                                As it
>>>> also touches the generic code, other auxtrace users like ARM SPE will
>>>> be affected too.  I added a test case to verify it works with pipes.
>>>>
>>>> At last, I can run this command without a problem.
>>>>
>>>>   $ perf record -o- -e intel_pt// true | perf inject -b | perf report -i- --itrace=i1000
>>>>
>>>> The code is available at 'perf/auxtrace-pipe-v1' branch in
>>>>
>>>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>>>
>>>> Thanks,
>>>> Namhyung
>>>>
>>>> Namhyung Kim (4):
>>>>   perf inject: Use perf_data__read() for auxtrace
>>>>   perf intel-pt: Do not try to queue auxtrace data on pipe
>>>>   perf session: Avoid calling lseek(2) for pipe
>>>>   perf test: Add pipe mode test to the Intel PT test suite
>>>>
>>>>  tools/perf/builtin-inject.c             |  6 +++---
>>>>  tools/perf/tests/shell/test_intel_pt.sh | 17 +++++++++++++++++
>>>>  tools/perf/util/auxtrace.c              |  3 +++
>>>>  tools/perf/util/session.c               |  9 +++++++--
>>>>  4 files changed, 30 insertions(+), 5 deletions(-)
>>>>
>>>>
>>>> base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9
>>>> prerequisite-patch-id: 4ccdf9c974a3909075051f4ffe498faecab7567b
>>>
>
  

On Mon, Jan 30, 2023 at 2:56 AM James Clark <james.clark@arm.com> wrote:
>
>
>
> On 27/01/2023 23:08, Namhyung Kim wrote:
> > Hi James,
> >
> > On Fri, Jan 27, 2023 at 6:42 AM James Clark <james.clark@arm.com> wrote:
> >>
> >>
> >>
> >> On 27/01/2023 07:22, Adrian Hunter wrote:
> >>> On 27/01/23 02:19, Namhyung Kim wrote:
> >>>> Hello,
> >>>>
> >>>> I found some problems in Intel-PT and auxtrace in general with pipe.
> >>>> In the past it used to work with pipe, but recent code fails.
> >>>
> >>> Pipe mode is a problem for Intel PT and possibly other auxtrace users.
> >>
> >> Just some info from my side: For Arm Coresight we ended up deprecating
> >> pipe mode, then not supporting it altogether. First was when we added an
> >> optional step to peek through all of the data to help with an edge case.
> >> Then we added a requirement to receive a HW_ID packet before decoding
> >> which necessitated the peek. You can't peek in pipe mode because you
> >> need to be able to seek, so it's not supported at all anymore.
> >>
> >> For Arm SPE I never tested it with piped data. I suppose I could add a
> >> test at some point, but I don't really see the usecase.
> >
> > Yeah, it'd be great if we can have a test for Arm SPE.
> >
>
> Ok thanks I will put it on the list of things to do.
>
> > Anyway, my work env (Google) requires the pipe mode due to the
> > restriction in disk usage.  Without the pipe support, it's not possible
> > to run `perf record` in production.
> >
>
> Makes sense. Unfortunately at the moment with Coresight, because of the
> lack of appropriate timestamps we're waiting for the end of the file
> before starting decoding. So you're not really any better off using
> piped mode, unless you have a lot more memory than disk space?
>
> Since this commit [1] and Arm v8.4 we can actually start making use of
> the timestamps and do a streaming decode again. So I will also add it to
> the list to look into that for Coresight again. Are you using an old
> version of Perf or not using Coresight at all? I know Denis at Google is
> using Coresight, but only with files rather than pipes.

I'm not aware of usage of Coresight yet in my boundary, but others
may be using it.

>
> One other thing, have you used the --switch-output mode to perf record
> before? I would have said it would give you some of the benefits of
> piped mode, but is more likely to work with Coresight. But last time I
> checked it's not working either. Not very helpful I know, but something
> to keep in mind.

I don't think it'd work because it still occupies the same space.
So far, the pipe mode worked well but I think it needs some
more improvements.

Anyway, thanks for your suggestion and review!

Thanks,
Namhyung
  

On Mon, Jan 30, 2023 at 9:36 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 30/01/23 16:15, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Jan 27, 2023 at 02:54:36PM -0800, Namhyung Kim escreveu:
> >> Hi Adrian,
> >>
> >> On Thu, Jan 26, 2023 at 11:22 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>>
> >>> On 27/01/23 02:19, Namhyung Kim wrote:
> >>>> Hello,
> >>>>
> >>>> I found some problems in Intel-PT and auxtrace in general with pipe.
> >>>> In the past it used to work with pipe, but recent code fails.
> >>>
> >>> Pipe mode is a problem for Intel PT and possibly other auxtrace users.
> >>> 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:
> >>>     - 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.
> >>
> >> Ok, IIUC we want to process the main buffer and auxtrace buffer
> >> together in time order but there's no guarantee to get the auxtrace
> >> data in time, right?
>
> Yes
>
> >>
> >> I wonder if it's possible to use 2 pass processing for pipe mode.
> >> We may keep the events in the ordered queue and auxtrace queue
> >> in the first pass, and process together from the beginning in the
> >> second pass. But I guess the data size would be a problem.
> >>
> >> Or, assuming that the auxtrace buffer comes later than (or equal to)
> >> the main buffer, we may start processing the main buffer as soon as
> >> every auxtrace queue gets some data.  Thoughts?
>
> That sounds like it would require figuring out a timestamp up to
> which there is Intel PT trace data in all queues.  That would
> be very complicated.

Yeah, I don't think it's a simple change.  Just think out loud how
we can handle it.  I'll think about it more..

>
> >>
> >>>
> >>> 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.
> >>>
> >>> For Intel PT, it is a bit of a surprise that there is not
> >>> validation to error out in pipe mode.
> >>
> >> What kind of validation do you have in mind?  Checking pid/tid?
>
> Validation to kill pipe mode for Intel PT entirely.  But a warning
> is ok.

I see.

>
> >>
> >>>
> >>> At the least, a warning is needed, and the above explanation needs
> >>> to be added to the documentation.
> >>
> >> Thanks, I'll add it to the documentation.
> >
> > Ok, so I'll wait for v2 of this patch series, Adrian, apart from what
> > you mentioned, are you ok with the patches, or a subset of them? The
> > first ones looks ok, right?
>
> Yes they are ok.
>
> >
> > - Arnaldo
> >
> >> How about showing something like this for pipe mode?
> >>
> >>   WARNING: Intel-PT with pipe mode may not work correctly.
>
> Perhaps:
>
> WARNING: Intel PT with pipe mode is not recommended. The output cannot be relied upon. In particular, time stamps and the order of events may be incorrect.

Ok, will add this.

Thanks,
Namhyung