linux-kernel - Re: [PATCH RFC 4/9] perf jevents: Add sys_events_find_events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Jul 2023 11:55:21 +0100
From:   John Garry <john.g.garry@...cle.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     acme@...nel.org, namhyung@...nel.org, jolsa@...nel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        renyu.zj@...ux.alibaba.com, shangxiaojing@...wei.com,
        kjain@...ux.ibm.com, kan.liang@...ux.intel.com
Subject: Re: [PATCH RFC 4/9] perf jevents: Add sys_events_find_events_table()


>>>> MetricExpr: "cpu_core@...tructions@ / cpu_core@...les@",
>> I did not know that it was possible to state that an event is for a
>> specific PMU type in this fashion - is this feature new? Does it work
>> only for known terms, like cycles and instructions?
> It has been in metrics a long time (I didn't choose that @ was the /
> replacement 😄 ). It should work for all events.
> 

Good to know.

>>>> The @ is used to avoid parsing confusion with / meaning divide. The
>>>> PMUs for the events are explicitly listed here. We could say the PMU
>>>> is implied but then it gets complex for uncore events, for metrics
>>>> that mix core and uncore events.
>> So this works ok for IPC and CPU PMUs as we want the same event for many
>> PMU types and naturally it would have the same name.
>>
>> I am still not sure that sys event metrics need to specify a PMU.
> There was a similar thought for hybrid metrics. The PMU could be
> implied from the PMU of the metric. I think there can be confusion
> from an implied PMU, for example the cycles event without a PMU will
> open two events on a hybrid CPU. If we imply the PMU then it can mean
> just 1 PMU, but if the PMU doesn't have the event presumably it means
> the multiple PMU behavior.
> 
> In parse-events there is existing logic to wildcard events but to
> ignore those that don't match a given PMU. This is used to support the
> --cputype option in builtin-stat.c, there is a similar option for
> builtin-list.c. We can use this so that events in a metric only match
> the PMU of the metric. Currently there are core metrics but whose
> events are all uncore like:
> https://urldefense.com/v3/__https://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*n1802__;Iw!!ACWV5N9M2RV99hQ!MjhanGd4AVsAl6d8weFktNHgeOptrgeBDyooXlpeW-J1TQ0e2BwzvqO4BTFEjs_gRzuWTPfnhW_jLx1pIJc$  
> 
> So we'd need to move these metrics to be on the appropriate uncore
> PMU. Supporting >1 PMU

To be crystal clear, when you say ">1 PMU", do you mean ">1 PMU instance 
of the same type" or ">1 PMU type"?

  in a metric wouldn't work though as it would
> appear the event was missing. Having the metric specify the PMU avoids
> these problems, but is verbose.

The message I'm getting - correct me if I am wrong - is that you would 
still prefer the PMU specified per event in metric expr, right? We don't 
do that exactly for sys PMU metrics today - we specify "Unit" instead, like:

MetricExpr: "sys_pmu_bar_event_foo1 + sys_pmu_bar_event_foo2"
Compat: "baz"
Unit:"sys_pmu_bar"

And so you prefer something like the following, right?
MetricExpr: "sys_pmu_foo@...1@ + sys_pmu_foo@...2@"

If so, I think that is ok - I just want to get rid of Unit and Compat.

Thanks,
John