Re: [igt-dev] [PATCH 4/5] tools/i915-perf: Add mmapped OA buffer support to i915-perf-recorder

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 24 Aug 2021 12:40:29 -0700, Dixit, Ashutosh wrote:
>
> > > I haven't looked at everything so correct me if I am wrong but I have this
> > > other concern about INTEL_PERF_RECORD_TYPE_MULTIPLE_SAMPLE. What is the
> > > reason for introducing this? I would have thought that whether OA data has
> > > been collected using read's or mmap is a property only of the recorder and
> > > should not be exposed to the reader or gpuvis. So in the mmap case the
> > > recorder should basically do what the kernel does but not introduce a new
> > > perf record type. But now we are seeing changes both in the reader and
> > > gpuvis because we have introduced INTEL_PERF_RECORD_TYPE_MULTIPLE_SAMPLE?
> > > Thanks.
> >
> > Data coming from the mmaped buffer is different from that coming from the
> > read. When we issue a read, the kernel attaches a record header to each
> > report. The data from the mmapped buffer is just raw reports without this
> > header. Lionel had mentioned that we add a new record here with multiple
> > reports in a record rather than add a header to each report at this stage
>
> My point is that if the recorder adds the same header which the kernel does
> we can have all changes done in one place, in the recorder, rather than
> multiple places (reader and gpuvis). I also think this should avoid making
> any changes to the reader and gpuvis and so will actually be simpler.
>
> > (which I think might make us a little slow in reading the OA buffer).
>
> Well we will just be doing in userspace what the kernel is already doing so
> it shouldn't be any slower than the kernel.
>
> > Also note that this does not replace the old mechanism of capturing reports
> > using read. Instead it's another way to capture reports.
>
> Yes that is clear and it also seems (from that drain code) that we can be
> doing both (reads and mmap) simultaneously.

Since this seems already implemented and probably also working we may still
go with what we have, but at least I wanted to indicate my preference and
also see if @Lionel has any further input. Thanks.