Received-SPF: pass (google.com: domain of linux-kernel+bounces-182640-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240515054443.2824147-1-weilin.wang@intel.com>
 <20240515054443.2824147-2-weilin.wang@intel.com> <CAP-5=fVJmJ0MapzadOcBfQj69FKhaFA9YtonMy4W7kUfDSXS9Q@mail.gmail.com>
 <CO6PR11MB5635F7C3C6928C2F3E5D732FEEED2@CO6PR11MB5635.namprd11.prod.outlook.com>
In-Reply-To: <CO6PR11MB5635F7C3C6928C2F3E5D732FEEED2@CO6PR11MB5635.namprd11.prod.outlook.com>
From: Namhyung Kim <namhyung@kernel.org>
Date: Fri, 17 May 2024 14:28:09 -0700
Message-ID: <CAM9d7cj5kc3RVpn9uffojkHuBb6Ui4HeRWB9WC6CKPSJhJtf0A@mail.gmail.com>
Subject: Re: [RFC PATCH v8 1/7] perf Document: Add TPEBS to Documents
To: "Wang, Weilin" <weilin.wang@intel.com>
Cc: Ian Rogers <irogers@google.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, 
	Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, 
	"Hunter, Adrian" <adrian.hunter@intel.com>, Kan Liang <kan.liang@linux.intel.com>, 
	"linux-perf-users@vger.kernel.org" <linux-perf-users@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Taylor, Perry" <perry.taylor@intel.com>, 
	"Alt, Samantha" <samantha.alt@intel.com>, "Biggers, Caleb" <caleb.biggers@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hello,

On Thu, May 16, 2024 at 10:37=E2=80=AFAM Wang, Weilin <weilin.wang@intel.co=
m> wrote:
>
>
>
> > -----Original Message-----
> > From: Ian Rogers <irogers@google.com>
> > Sent: Thursday, May 16, 2024 9:11 AM
> > To: Wang, Weilin <weilin.wang@intel.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>; Arnaldo Carvalho de Melo
> > <acme@kernel.org>; Peter Zijlstra <peterz@infradead.org>; Ingo Molnar
> > <mingo@redhat.com>; Alexander Shishkin
> > <alexander.shishkin@linux.intel.com>; Jiri Olsa <jolsa@kernel.org>; Hun=
ter,
> > Adrian <adrian.hunter@intel.com>; Kan Liang <kan.liang@linux.intel.com>=
;
> > linux-perf-users@vger.kernel.org; linux-kernel@vger.kernel.org; Taylor,=
 Perry
> > <perry.taylor@intel.com>; Alt, Samantha <samantha.alt@intel.com>; Bigge=
rs,
> > Caleb <caleb.biggers@intel.com>
> > Subject: Re: [RFC PATCH v8 1/7] perf Document: Add TPEBS to Documents
> >
> > On Tue, May 14, 2024 at 10:44=E2=80=AFPM <weilin.wang@intel.com> wrote:
> > >
> > > From: Weilin Wang <weilin.wang@intel.com>
> > >
> > > TPEBS is a new feature in next generation of Intel PMU. It will be us=
ed in new
> > > TMA releases. Adding related introduction to documents while adding n=
ew
> > code to
> > > support it in perf stat.
> > >
> > > Signed-off-by: Weilin Wang <weilin.wang@intel.com>
> > > ---
> > >  tools/perf/Documentation/perf-list.txt |  1 +
> > >  tools/perf/Documentation/topdown.txt   | 18 ++++++++++++++++++
> > >  2 files changed, 19 insertions(+)
> > >
> > > diff --git a/tools/perf/Documentation/perf-list.txt
> > b/tools/perf/Documentation/perf-list.txt
> > > index 6bf2468f59d3..dea005410ec0 100644
> > > --- a/tools/perf/Documentation/perf-list.txt
> > > +++ b/tools/perf/Documentation/perf-list.txt
> > > @@ -72,6 +72,7 @@ counted. The following modifiers exist:
> > >   W - group is weak and will fallback to non-group if not schedulable=
,
> > >   e - group or event are exclusive and do not share the PMU
> > >   b - use BPF aggregration (see perf stat --bpf-counters)
> > > + R - retire latency value of the event
> > >
> > >  The 'p' modifier can be used for specifying how precise the instruct=
ion
> > >  address should be. The 'p' modifier can be specified multiple times:
> > > diff --git a/tools/perf/Documentation/topdown.txt
> > b/tools/perf/Documentation/topdown.txt
> > > index ae0aee86844f..e6c4424e8bf2 100644
> > > --- a/tools/perf/Documentation/topdown.txt
> > > +++ b/tools/perf/Documentation/topdown.txt
> > > @@ -325,6 +325,24 @@ other four level 2 metrics by subtracting
> > corresponding metrics as below.
> > >      Fetch_Bandwidth =3D Frontend_Bound - Fetch_Latency
> > >      Core_Bound =3D Backend_Bound - Memory_Bound
> > >
> > > +TPEBS in TopDown
> > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > > +
> > > +TPEBS is one of the features provided by the next generation of Inte=
l PMU.
> > The
> >
> > As this documentation will live a while "next generation" could become
> > ambiguous. I think it would be better to mention core ultra or some
> > other term to more specifically describe which PMUs have TPEBS.
>
> Hi Ian,
>
> Yes, you are right, I will update it.

Also I'd be nice if you can tell what 'T' means in TPEBS. :)

> >
> > > +TPEBS feature adds a 16 bit retire latency field in the Basic Info g=
roup of the
> > > +PEBS record. It records the Core cycles since the retirement of the =
previous
> > > +instruction to the retirement of current instruction. Please refer t=
o Section
> > > +8.4.1 of "Intel=C2=AE Architecture Instruction Set Extensions Progra=
mming
> > Reference"
> > > +for more details about this feature.
> >
> > Perhaps capture that this is placed in the perf event sample in the
> > weights section as TPEBS isn't exposed except within the kernel PMU
> > driver.
> >
> > > +
> > > +In the most recent release of TMA, the metrics begin to use event
> > retire_latency
> > > +values in some of the metrics=E2=80=99 formulas on processors that s=
upport TPEBS
> > feature.
> > > +For previous generations that do not support TPEBS, the values are s=
tatic
> > and
> > > +predefined per processor family by the hardware architects. Due to t=
he
> > diversity
> > > +of workloads in execution environments, retire latency values measur=
ed at
> > real
> > > +time are more accurate. Therefore, new TMA metrics that use TPEBS wi=
ll
> > provide
> > > +more accurate performance analysis results.
> >
> > Do you want to capture what the value will be when there hasn't been a
> > sample? This corner case could be considered broken in the new
> > approach.
>
>
> When there is no sample, we should expect it to be 0 or use default value=
 I will
> add this information here. I don=E2=80=99t think this is broken from the =
approach's perspective.
> We do need to add code to return this value when there is no sample.

More importantly, I think the documentation should say that the
retire_latency is in PEBS which means it needs samples in the
precise events.  So it would run perf record in the background
for events with retire_latency even if users just want to see the
value of counters or metric in perf stat.

Thanks,
Namhyung