Re: [PATCH v4 00/18]

[Ian Rogers]

On Tue, Nov 11, 2025 at 2:42 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> On Tue, Nov 11, 2025 at 01:21:48PM -0800, Ian Rogers wrote:
> > Prior to this series stat-shadow would produce hard coded metrics if
> > certain events appeared in the evlist. This series produces equivalent
> > json metrics and cleans up the consequences in tests and display
> > output. A before and after of the default display output on a
> > tigerlake is:
> >
> > Before:
> > ```
> > $ perf stat -a sleep 1
> >
> >  Performance counter stats for 'system wide':
> >
> >     16,041,816,418      cpu-clock                        #   15.995 CPUs utilized
> >              5,749      context-switches                 #  358.376 /sec
> >                121      cpu-migrations                   #    7.543 /sec
> >              1,806      page-faults                      #  112.581 /sec
> >        825,965,204      instructions                     #    0.70  insn per cycle
> >      1,180,799,101      cycles                           #    0.074 GHz
> >        168,945,109      branches                         #   10.532 M/sec
> >          4,629,567      branch-misses                    #    2.74% of all branches
> >  #     30.2 %  tma_backend_bound
> >                                                   #      7.8 %  tma_bad_speculation
> >                                                   #     47.1 %  tma_frontend_bound
> >  #     14.9 %  tma_retiring
> > ```
> >
> > After:
> > ```
> > $ perf stat -a sleep 1
> >
> >  Performance counter stats for 'system wide':
> >
> >              2,890      context-switches                 #    179.9 cs/sec  cs_per_second
> >     16,061,923,339      cpu-clock                        #     16.0 CPUs  CPUs_utilized
> >                 43      cpu-migrations                   #      2.7 migrations/sec  migrations_per_second
> >              5,645      page-faults                      #    351.5 faults/sec  page_faults_per_second
> >          5,708,413      branch-misses                    #      1.4 %  branch_miss_rate         (88.83%)
> >        429,978,120      branches                         #     26.8 M/sec  branch_frequency     (88.85%)
> >      1,626,915,897      cpu-cycles                       #      0.1 GHz  cycles_frequency       (88.84%)
> >      2,556,805,534      instructions                     #      1.5 instructions  insn_per_cycle  (88.86%)
> >                         TopdownL1                 #     20.1 %  tma_backend_bound
> >                                                   #     40.5 %  tma_bad_speculation      (88.90%)
> >                                                   #     17.2 %  tma_frontend_bound       (78.05%)
> >                                                   #     22.2 %  tma_retiring             (88.89%)
> >
> >        1.002994394 seconds time elapsed
> > ```
> >
> > Having the metrics in json brings greater uniformity, allows events to
> > be shared by metrics, and it also allows descriptions like:
> > ```
> > $ perf list cs_per_second
> > ...
> >   cs_per_second
> >        [Context switches per CPU second]
> > ```
> >
> > A thorn in the side of doing this work was that the hard coded metrics
> > were used by perf script with '-F metric'. This functionality didn't
> > work for me (I was testing `perf record -e instructions,cycles`
> > with/without leader sampling and then `perf script -F metric` but saw
> > nothing but empty lines) but anyway I decided to fix it to the best of
> > my ability in this series. So the script side counters were removed
> > and the regular ones associated with the evsel used. The json metrics
> > were all searched looking for ones that have a subset of events
> > matching those in the perf script session, and all metrics are
> > printed. This is kind of weird as the counters are being set by the
> > period of samples, but I carried the behavior forward. I suspect there
> > needs to be follow up work to make this better, but what is in the
> > series is superior to what is currently in the tree. Follow up work
> > could include finding metrics for the machine in the perf.data rather
> > than using the host, allowing multiple metrics even if the metric ids
> > of the events differ, fixing pre-existing `perf stat record/report`
> > issues, etc.
> >
> > There is a lot of stat tests that, for example, assume '-e
> > instructions,cycles' will produce an IPC metric. These things needed
> > tidying as now the metric must be explicitly asked for and when doing
> > this ones using software events were preferred to increase
> > compatibility. As the test updates were numerous they are distinct to
> > the patches updating the functionality causing periods in the series
> > where not all tests are passing. If this is undesirable the test fixes
> > can be squashed into the functionality updates, but this will be kind
> > of messy, especially as at some points in the series both the old
> > metrics and the new metrics will be displayed.
> >
> > v4: K/sec to M/sec on branch frequency (Namhyung), perf script -F
> >     metric to-done a system-wide calculation (Namhyung) and don't
> >     crash because of the CPU map index couldn't be found. Regenerate
> >     commit messages but the cpu-clock was always yielding 0 on my
> >     machine leading to a lot of nan metric values.
>
> This is strange.  The cpu-clock should not be 0 as long as you ran it.
> Do you think it's related to the scale unit change?  I tested v3 and
> didn't see the problem.

It looked like a kernel issue. The raw counts were 0 before being
scaled. All metrics always work on unscaled values. It is only the
commit messages and the formatting is more important than the numeric
values - which were correct for a cpu-clock of 0.

Thanks,
Ian