RE: [RFC PATCH v12 6/8] perf stat: Add command line option for enabling tpebs recording
From: Wang, Weilin
Date: Tue Jun 11 2024 - 16:08:21 EST
> -----Original Message-----
> From: Namhyung Kim <namhyung@xxxxxxxxxx>
> Sent: Tuesday, June 11, 2024 11:49 AM
> To: Wang, Weilin <weilin.wang@xxxxxxxxx>
> Cc: Ian Rogers <irogers@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 v12 6/8] perf stat: Add command line option for
> enabling tpebs recording
>
> On Tue, Jun 11, 2024 at 10:49 AM Wang, Weilin <weilin.wang@xxxxxxxxx>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Namhyung Kim <namhyung@xxxxxxxxxx>
> > > Sent: Tuesday, June 11, 2024 9:50 AM
> > > To: Wang, Weilin <weilin.wang@xxxxxxxxx>
> > > Cc: Ian Rogers <irogers@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 v12 6/8] perf stat: Add command line option for
> > > enabling tpebs recording
> > >
> > > Hello,
> > >
> > > On Mon, Jun 10, 2024 at 10:24 PM <weilin.wang@xxxxxxxxx> wrote:
> > > >
> > > > From: Weilin Wang <weilin.wang@xxxxxxxxx>
> > > >
> > > > With this command line option, tpebs recording is turned off in perf stat
> on
> > > > default. It will only be turned on when this option is given in perf stat
> > > > command.
> > > >
> > > > Exampe with --enable-tpebs-recording:
> > > >
> > > > perf stat -M tma_dtlb_store -a --enable-tpebs-recording 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 MEM_INST_RETIRED.STLB_HIT_STORES:R
> > >
> > > Here I guess we can expect a non-zero value, right?
> >
> > Hi Namhyung,
> >
> > Do you mean when we have the option, we should expect non-zero value?
> > During the execution, it's possible that we don't capture any sample on this
> > event. In this case, we would have a zero value. In the future, I think we will
> > give it the default value instead of zero.
>
> I mean there's a chance you get non-zero, but it can be zero sometimes, right?
> Then I think it's better to choose non-zero in this example otherwise it's not
> clear what's the difference of using this option or not when you look at the
> output in this commit message (they are both 0).
I see. I will create a non-zero example and update.
Thanks,
Weilin
>
> Thanks,
> Namhyung
>
> >
> > >
> > > >
> > > > 1.003105924 seconds time elapsed
> > > >
> > > > Exampe without --enable-tpebs-recording:
> > > >
> > > > perf stat -M tma_dtlb_store -a sleep 1
> > > >
> > > > 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 MEM_INST_RETIRED.STLB_HIT_STORES:R
> > > >
> > > > 1.003105924 seconds time elapsed
> > > >
> > > > Signed-off-by: Weilin Wang <weilin.wang@xxxxxxxxx>
> > > > ---
> > > > tools/perf/builtin-stat.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > > index 14be132f7b6f..055139e44763 100644
> > > > --- a/tools/perf/builtin-stat.c
> > > > +++ b/tools/perf/builtin-stat.c
> > > > @@ -1235,6 +1235,8 @@ static struct option stat_options[] = {
> > >
> > > What's the base of your patchset? I think we moved this to cmd_stat().
> > > Please make sure to rebase on the perf-tools-next.
> >
> > This was rebased on top of Ian's change for the tool event and retire_latency
> parser
> > patches. I think that was on tmp.perf-tools.next. I will rebase on perf-tools-
> next.
> >
> > >
> > >
> > > > "disable adding events for the metric threshold calculation"),
> > > > OPT_BOOLEAN(0, "topdown", &topdown_run,
> > > > "measure top-down statistics"),
> > > > + OPT_BOOLEAN(0, "enable-tpebs-recording", &tpebs_recording,
> > > > + "enable recording for tpebs when retire_latency required"),
> > >
> > > Please update the documentation (man page) for this option.
> > >
> > > Also I'm afraid it'd fail to build on non-x86 because tpebes_recording
> > > is defined in intel-tpebs.c.
> > >
> >
> > Yes, you are right. I forgot about non-x86. I will update this and also the
> > Documentation.
> >
> > Thanks,
> > Weilin
> >
> > > Thanks,
> > > Namhyung
> > >
> > >
> > > > OPT_UINTEGER(0, "td-level", &stat_config.topdown_level,
> > > > "Set the metrics level for the top-down statistics (0: max
> level)"),
> > > > OPT_BOOLEAN(0, "smi-cost", &smi_cost,
> > > > --
> > > > 2.43.0
> > > >