RE: [RFC PATCH v9 0/7] TPEBS counting mode support

From: Wang, Weilin
Date: Thu May 23 2024 - 12:45:58 EST




> -----Original Message-----
> From: Ian Rogers <irogers@xxxxxxxxxx>
> Sent: Wednesday, May 22, 2024 9:04 PM
> To: Wang, Weilin <weilin.wang@xxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>; Arnaldo Carvalho de Melo
> <acme@xxxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Ingo Molnar
> <mingo@xxxxxxxxxx>; Alexander Shishkin
> <alexander.shishkin@xxxxxxxxxxxxxxx>; Jiri Olsa <jolsa@xxxxxxxxxx>; Hunter,
> Adrian <adrian.hunter@xxxxxxxxx>; Kan Liang <kan.liang@xxxxxxxxxxxxxxx>;
> linux-perf-users@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Taylor, Perry
> <perry.taylor@xxxxxxxxx>; Alt, Samantha <samantha.alt@xxxxxxxxx>; Biggers,
> Caleb <caleb.biggers@xxxxxxxxx>
> Subject: Re: [RFC PATCH v9 0/7] TPEBS counting mode support
>
> On Tue, May 21, 2024 at 10:39 AM <weilin.wang@xxxxxxxxx> wrote:
> >
> > From: Weilin Wang <weilin.wang@xxxxxxxxx>
> >
> > Changes in v9:
> > - Update the retire_latency result print and metric calculation method. Plugin
> > the value to evsel so that no special code is required.
> > - Update --control:fifo to use pipe instead of named pipe.
> > - Add test for TPEBS counting mode.
> > - Update Document with more details.
> >
> > Changes in v8:
> > - In this revision, the code is updated to base on Ian's patch on R modifier
> > parser https://lore.kernel.org/lkml/20240428053616.1125891-3-
> irogers@xxxxxxxxxx/
>
> I pulled the first patch of these 3, to clean up the tool events into
> its own patch and hopefully addressed Namhyung's feedback:
> https://lore.kernel.org/lkml/20240503232849.17752-1-
> irogers@xxxxxxxxxx/
> It is looking for review.
>
> I think in general it'd be useful when posting the series to include
> the patches it is dependent on, that way we can also make changes to
> those patches (for example the missed perf-list documentation of the
> 'R' modifier).

Hi Ian,

I will include the 2nd patch from the series in the TPEBS patch set next time.

Thanks,
Weilin
>
> Thanks,
> Ian
>
>
>
> > After this change, there is no special code required for R modifier in
> > metricgroup.c and metricgroup.h files.
> >
> > Caveat of this change:
> > Ideally, we will need to add special handling to skip counting events with R
> > modifier in evsel. Currently, this is not implemented so the event with :R will
> > be both counted and sampled. Usually, in a metric formula that uses
> retire_latency,
> > it would already require to count the event. As a result, we will endup count
> the
> > same event twice. This should be able to be handled properly when we
> finalize our
> > design on evsel R modifier support.
> >
> > - Move TPEBS specific code out from main perf stat code to separate files in
> > util/intel-tpebs.c and util/intel-tpebs.h. [Namhyung]
> > - Use --control:fifo to ack perf stat from forked perf record instead of
> sleep(2) [Namhyung]
> > - Add introductions about TPEBS and R modifier in Documents. [Namhyung]
> >
> >
> > Changes in v7:
> > - Update code and comments for better code quality [Namhyung]
> > - Add a separate commit for perf data [Namhyung]
> > - Update retire latency print function to improve alignment [Namhyung]
> >
> > Changes in v6:
> > - Update code and add comments for better code quality [Namhyung]
> > - Remove the added fd var and directly pass the opened fd to data.file.fd
> [Namhyung]
> > - Add kill() to stop perf record when perf stat exists early [Namhyung]
> > - Add command opt check to ensure only start perf record when -a/-C given
> [Namhyung]
> > - Squash commits [Namhyung]
> >
> > Changes in v5:
> > - Update code and add comments for better code quality [Ian]
> >
> > Changes in v4:
> > - Remove uncessary debug print and update code and comments for better
> > readability and quality [Namhyung]
> > - Update mtl metric json file with consistent TmaL1 and TopdownL1
> metricgroup
> >
> > Changes in v3:
> > - Remove ':' when event name has '@' [Ian]
> > - Use 'R' as the modifier instead of "retire_latency" [Ian]
> >
> > Changes in v2:
> > - Add MTL metric file
> > - Add more descriptions and example to the patch [Arnaldo]
> >
> > Here is an example of running perf stat to collect a metric that uses
> > retire_latency value of event MEM_INST_RETIRED.STLB_HIT_STORES on a
> MTL system.
> >
> > In this simple example, there is no MEM_INST_RETIRED.STLB_HIT_STORES
> sample.
> > Therefore, the MEM_INST_RETIRED.STLB_HIT_STORES:p count and
> retire_latency value
> > are all 0.
> >
> > ./perf stat -M tma_dtlb_store -a -- sleep 1
> >
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.000 MB - ]
> >
> > Performance counter stats for 'system wide':
> >
> > 181,047,168 cpu_core/TOPDOWN.SLOTS/ # 0.6 %
> tma_dtlb_store
> > 3,195,608 cpu_core/topdown-retiring/
> > 40,156,649 cpu_core/topdown-mem-bound/
> > 3,550,925 cpu_core/topdown-bad-spec/
> > 117,571,818 cpu_core/topdown-fe-bound/
> > 57,118,087 cpu_core/topdown-be-bound/
> > 69,179 cpu_core/EXE_ACTIVITY.BOUND_ON_STORES/
> > 4,582 cpu_core/MEM_INST_RETIRED.STLB_HIT_STORES/
> > 30,183,104 cpu_core/CPU_CLK_UNHALTED.DISTRIBUTED/
> > 30,556,790 cpu_core/CPU_CLK_UNHALTED.THREAD/
> > 168,486 cpu_core/DTLB_STORE_MISSES.WALK_ACTIVE/
> > 0.00 MEM_INST_RETIRED.STLB_HIT_STORES:p 0 0
> >
> > 1.003105924 seconds time elapsed
> >
> > v1:
> > TPEBS is one of the features provided by the next generation of Intel PMU.
> > Please refer to Section 8.4.1 of "Intel® Architecture Instruction Set
> Extensions
> > Programming Reference" [1] for more details about this feature.
> >
> > This set of patches supports TPEBS in counting mode. The code works in the
> > following way: it forks a perf record process from perf stat when
> retire_latency
> > of one or more events are used in a metric formula. Perf stat would send a
> > SIGTERM signal to perf record before it needs the retire latency value for
> > metric calculation. Perf stat will then process sample data to extract the
> > retire latency data for metric calculations. Currently, the code uses the
> > arithmetic average of retire latency values.
> >
> > [1] https://www.intel.com/content/www/us/en/content-
> details/812218/intel-architecture-instruction-set-extensions-programming-
> reference.html?wapkw=future%20features
> >
> >
> > Weilin Wang (7):
> > perf vendor events intel: Add MTL metric json files
> > perf data: Allow to use given fd in data->file.fd
> > perf stat: Fork and launch perf record when perf stat needs to get
> > retire latency value for a metric.
> > perf stat: Plugin retire_lat value from sampled data to evsel
> > perf stat: Add command line option for enabling tpebs recording
> > perf Document: Add TPEBS to Documents
> > perf test: Add test for Intel TPEBS counting mode
> >
> > tools/perf/Documentation/perf-list.txt | 1 +
> > tools/perf/Documentation/topdown.txt | 30 +
> > tools/perf/arch/x86/util/evlist.c | 6 +
> > tools/perf/builtin-stat.c | 24 +
> > .../arch/x86/meteorlake/metricgroups.json | 127 +
> > .../arch/x86/meteorlake/mtl-metrics.json | 2551 +++++++++++++++++
> > .../perf/tests/shell/test_stat_intel_tpebs.sh | 27 +
> > tools/perf/util/Build | 1 +
> > tools/perf/util/data.c | 7 +-
> > tools/perf/util/evsel.c | 53 +
> > tools/perf/util/evsel.h | 5 +
> > tools/perf/util/intel-tpebs.c | 301 ++
> > tools/perf/util/intel-tpebs.h | 30 +
> > tools/perf/util/stat.h | 1 +
> > 14 files changed, 3163 insertions(+), 1 deletion(-)
> > create mode 100644 tools/perf/pmu-
> events/arch/x86/meteorlake/metricgroups.json
> > create mode 100644 tools/perf/pmu-events/arch/x86/meteorlake/mtl-
> metrics.json
> > create mode 100755 tools/perf/tests/shell/test_stat_intel_tpebs.sh
> > create mode 100644 tools/perf/util/intel-tpebs.c
> > create mode 100644 tools/perf/util/intel-tpebs.h
> >
> > --
> > 2.43.0
> >