Re: [PATCH v1 2/2] perf parse-events: Corrections to topdown sorting

[Ian Rogers]

On Wed, Mar 5, 2025 at 5:44 AM James Clark <james.clark@xxxxxxxxxx> wrote:
>
>
>
> On 05/03/2025 8:37 am, Ian Rogers wrote:
> > In the case of '{instructions,slots},faults,topdown-retiring' the
> > first event that must be grouped, slots, is ignored causing the
> > topdown-retiring event not to be adjacent to the group it needs to be
> > inserted into. Don't ignore the group members when computing the
> > force_grouped_index.
> >
> > Make the force_grouped_index be for the leader of the group it is
> > within and always use it first rather than a group leader index so
> > that topdown events may be sorted from one group into another.
> >
> > Reported-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/lkml/20250224083306.71813-2-dapeng1.mi@xxxxxxxxxxxxxxx/
> > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> Testing on Arm seems ok, but presumably this doesn't change anything
> there because arch_evsel__must_be_in_group() is always false.
>
> On x86 I ran into the topdown metrics not opening on cpu_core at all, so
> I'm not sure if I'm able to test that the original issue is fixed on my
> machine. From looking at the link the issue is that the ungrouped
> topdown event is "<not supported>", but I always see that regardless of
> grouping despite perf list saying it exists:
>
>   $ perf list --unit cpu_core | grep -i topdown
>    topdown-bad-spec OR cpu_core/topdown-bad-spec/     [Kernel PMU event]
>    topdown-be-bound OR cpu_core/topdown-be-bound/     [Kernel PMU event]
>    topdown-br-mispredict OR cpu_core/topdown-br-mispredict/[Kernel PMU
> event]
>    topdown-fe-bound OR cpu_core/topdown-fe-bound/     [Kernel PMU event]
>    topdown-fetch-lat OR cpu_core/topdown-fetch-lat/   [Kernel PMU event]
>    topdown-heavy-ops OR cpu_core/topdown-heavy-ops/   [Kernel PMU event]
>    topdown-mem-bound OR cpu_core/topdown-mem-bound/   [Kernel PMU event]
>    topdown-retiring OR cpu_core/topdown-retiring/     [Kernel PMU event]
>    topdown.backend_bound_slots
>    topdown.bad_spec_slots
>    topdown.br_mispredict_slots
>    topdown.memory_bound_slots
>         [TOPDOWN.MEMORY_BOUND_SLOTS. Unit: cpu_core]
>
>
>   $ sudo perf stat -e topdown-retiring -- true
>   Performance counter stats for 'true':
>       <not counted>   cpu_atom/topdown-retiring/           (0.00%)
>     <not supported>   cpu_core/topdown-retiring/
>
>
>   $ sudo perf stat -e topdown-retiring -vvv -- true
> Control descriptor is not initialized
> Opening: topdown-retiring
> ------------------------------------------------------------
> perf_event_attr:
>    type                             10 (cpu_atom)
>    size                             136
>    config                           0xc2 (topdown-retiring)
>    sample_type                      IDENTIFIER
>    read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>    disabled                         1
>    inherit                          1
>    enable_on_exec                   1
> ------------------------------------------------------------
> sys_perf_event_open: pid 151404  cpu -1  group_fd -1  flags 0x8 = 3
> Opening: topdown-retiring
> ------------------------------------------------------------
> perf_event_attr:
>    type                             4 (cpu_core)
>    size                             136
>    config                           0x8000 (topdown-retiring)
>    sample_type                      IDENTIFIER
>    read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
>    disabled                         1
>    inherit                          1
>    enable_on_exec                   1
> ------------------------------------------------------------
> sys_perf_event_open: pid 151404  cpu -1  group_fd -1  flags 0x8
> sys_perf_event_open failed, error -22
> switching off exclude_guest for PMU cpu_core
> Using PERF_SAMPLE_READ / :S modifier is not compatible with inherit,
> falling back to no-inherit.
> Warning:
> topdown-retiring event is not supported by the kernel.

Yep, unfortunately there is a requirement that a topdown event like
topdown-retiring is always programmed with slots on performance cores.
The slots event must be the group leader. You can see in metrics the
slots event as "+ 0 * slots":
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/alderlake/adl-metrics.json?h=perf-tools-next#n754
```
        "MetricExpr": "topdown\\-be\\-bound / (topdown\\-fe\\-bound +
topdown\\-bad\\-spec + topdown\\-retiring + topdown\\-be\\-bound) + 0
* slots",
```
and making it the group leader is done by the sorting:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/x86/util/evlist.c?h=perf-tools-next#n67

We could probably add something to
parse_events__sort_events_and_fix_groups to inject the slots event,
but this hasn't been done yet.

My main concern with this change is there is some sensitivity to the
event ordering when parsing them in scripts. There's some context in:
https://lore.kernel.org/all/20230719001836.198363-1-irogers@xxxxxxxxxx/
This change makes the topdown events appear first in the group always,
but as you say you only see that if you use those events, otherwise
things are unchanged.

Thanks for testing!
Ian