MIME-Version: 1.0
References: <20230127001951.3432374-1-namhyung@kernel.org> <bda606c2-2b1b-de9f-1386-8ee2bf925b4b@intel.com>
 <CAM9d7cg_7LNKrXtBuo2QUR1Voi9NKM98qYLC+pznMf4-5yo4Dg@mail.gmail.com>
 <Y9fREY3BxROqYYBO@kernel.org> <5676a1b7-885c-e8d9-1809-8bedcf1ff995@intel.com>
In-Reply-To: <5676a1b7-885c-e8d9-1809-8bedcf1ff995@intel.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Mon, 30 Jan 2023 18:19:54 -0800
Message-ID: <CAM9d7chWEtgvJXgD3qM=iQ+Vr7r9ow6nzOXpouwHpnxbP8mn9w@mail.gmail.com>
Subject: Re: [PATCH 0/4] perf intel-pt: Fix the pipe mode (v1)
To:     Adrian Hunter <adrian.hunter@intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ian Rogers <irogers@google.com>,
        linux-perf-users@vger.kernel.org, Leo Yan <leo.yan@linaro.org>,
        James Clark <james.clark@arm.com>,
        Stephane Eranian <eranian@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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