Re: [PATCH 3/4] perf vendor events arm64: Add AmpereOne metrics

From: Ilkka Koskinen
Date: Tue Aug 08 2023 - 20:01:03 EST

Next message: Tian, Kevin: "RE: [PATCH v2 03/12] iommu: Remove unrecoverable fault data"
Previous message: Kees Cook: "Re: [PATCH 1/7] netfilter: ipset: refactor deprecated strncpy"
Next in thread: Ian Rogers: "Re: [PATCH 3/4] perf vendor events arm64: Add AmpereOne metrics"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, 7 Aug 2023, John Garry wrote:

On 04/08/2023 20:59, Ilkka Koskinen wrote:

Hi John

On Fri, 4 Aug 2023, John Garry wrote:

On 03/08/2023 22:13, Ilkka Koskinen wrote:

This patch adds AmpereOne metrics. The metrics also work around
the issue related to some of the events.

Would these events be any metrics added which are not a "Topdown"? I guess no, since there are many, but I just don't know.

Just curious, are these events/metrics described in some publically-available document?

I quickly checked that and there are a spreadsheet and a document available, which list the supported PMUs, their events and metrics in the customer connect website but that requires registering.

OK, thanks for the info. I ask is it always worthwhile mentioning a link in the changelog if publicly available.

I can certainly add a comment that the events are available at the customer connect website.

Just a few minor comments:

On 03/08/2023 22:13, Ilkka Koskinen wrote:

This patch adds AmpereOne metrics. The metrics also work around
the issue related to some of the events.

Signed-off-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
---
.../arch/arm64/ampere/ampereone/metrics.json | 362 ++++++++++++++++++
1 file changed, 362 insertions(+)

...

+ {
+ "MetricExpr": "CRYPTO_SPEC / OP_SPEC",
+ "BriefDescription": "Proportion of crypto data processing
operations",

+ "MetricGroup": "Instruction",
+ "MetricName": "Crypto mix"
+ },
+ {
+ "MetricExpr": "VFP_SPEC / (duration_time *1000000000)",
+ "BriefDescription": "Giga-floating point operations per second",
+ "MetricGroup": "Instruction",
+ "MetricName": "GFLOPS_ISSUED"
+ },
+ {
+ "MetricExpr": "DP_SPEC / OP_SPEC",
+ "BriefDescription": "Proportion of integer data processing
operations",

+ "MetricGroup": "Instruction",
+ "MetricName": "Integer mix"
+ },
+ {
+ "MetricExpr": "INST_RETIRED / CPU_CYCLES",
+ "BriefDescription": "Instructions per cycle",
+ "MetricGroup": "Instruction",
+ "MetricName": "IPC"
+ },
+ {
+ "MetricExpr": "LD_SPEC / OP_SPEC",
+ "BriefDescription": "Proportion of load operations",
+ "MetricGroup": "Instruction",
+ "MetricName": "Load mix"
+ },
+ {
+ "MetricExpr": "LDST_SPEC/ OP_SPEC",

mega nit: missing whitespace before '/'

I'll fix it.

+ "BriefDescription": "Proportion of load & store operations",
+ "MetricGroup": "Instruction",
+ "MetricName": "Load-store mix"
+ },
+ {
+ "MetricExpr": "INST_RETIRED / (duration_time * 1000000)",

I think that we may use 1e6 here for shorthand - it helps avoid mistakes with too few or many '0's :)

Oh, that's great. I don't think anyone needed to use those in arm64 and I guess I didn't realize to take a look at other architectures. I'll change all the numbers.

+ "BriefDescription": "Millions of instructions per second",
+ "MetricGroup": "Instruction",
+ "MetricName": "MIPS_RETIRED"
+ },
+ {
+ "MetricExpr": "INST_SPEC / (duration_time * 1000000)",
+ "BriefDescription": "Millions of instructions per second",
+ "MetricGroup": "Instruction",
+ "MetricName": "MIPS_UTILIZATION"
+ },
+ {
+ "MetricExpr": "PC_WRITE_SPEC / OP_SPEC",
+ "BriefDescription": "Proportion of software change of PC operations",
+ "MetricGroup": "Instruction",
+ "MetricName": "PC write mix"
+ },
+ {
+ "MetricExpr": "ST_SPEC / OP_SPEC",
+ "BriefDescription": "Proportion of store operations",
+ "MetricGroup": "Instruction",
+ "MetricName": "Store mix"
+ },
+ {
+ "MetricExpr": "VFP_SPEC / OP_SPEC",
+ "BriefDescription": "Proportion of FP operations",
+ "MetricGroup": "Instruction",
+ "MetricName": "VFP mix"
+ },
+ {
+ "MetricExpr": "1 - (OP_RETIRED/ (CPU_CYCLES * 4))",
+ "BriefDescription": "Proportion of slots lost",
+ "MetricGroup": "Speculation / TDA",
+ "MetricName": "CPU lost"
+ },
+ {
+ "MetricExpr": "OP_RETIRED/ (CPU_CYCLES * 4)",
+ "BriefDescription": "Proportion of slots retiring",
+ "MetricGroup": "Speculation / TDA",
+ "MetricName": "CPU utilization"
+ },
+ {
+ "MetricExpr": "OP_RETIRED - OP_SPEC",
+ "BriefDescription": "Operations lost due to misspeculation",
+ "MetricGroup": "Speculation / TDA",
+ "MetricName": "Operations lost"
+ },
+ {
+ "MetricExpr": "1 - (OP_RETIRED / OP_SPEC)",
+ "BriefDescription": "Proportion of operations lost",
+ "MetricGroup": "Speculation / TDA",
+ "MetricName": "Operations lost (ratio)"
+ },
+ {
+ "MetricExpr": "OP_RETIRED / OP_SPEC",
+ "BriefDescription": "Proportion of operations retired",
+ "MetricGroup": "Speculation / TDA",
+ "MetricName": "Operations retired"
+ },
+ {
+ "MetricExpr": "STALL_BACKEND_CACHE / CPU_CYCLES",
+ "BriefDescription": "Proportion of cycles stalled and no operations
issued to backend and cache miss",

+ "MetricGroup": "Stall",
+ "MetricName": "Stall backend cache cycles"
+ },
+ {
+ "MetricExpr": "STALL_BACKEND_RESOURCE / CPU_CYCLES",
+ "BriefDescription": "Proportion of cycles stalled and no operations
issued to backend and resource full",

+ "MetricGroup": "Stall",
+ "MetricName": "Stall backend resource cycles"
+ },
+ {
+ "MetricExpr": "STALL_BACKEND_TLB / CPU_CYCLES",
+ "BriefDescription": "Proportion of cycles stalled and no operations
issued to backend and TLB miss",

+ "MetricGroup": "Stall",
+ "MetricName": "Stall backend tlb cycles"
+ },
+ {
+ "MetricExpr": "STALL_FRONTEND_CACHE / CPU_CYCLES",
+ "BriefDescription": "Proportion of cycles stalled and no ops
delivered from frontend and cache miss",

+ "MetricGroup": "Stall",
+ "MetricName": "Stall frontend cache cycles"
+ },
+ {
+ "MetricExpr": "STALL_FRONTEND_TLB / CPU_CYCLES",
+ "BriefDescription": "Proportion of cycles stalled and no ops
delivered from frontend and TLB miss",

+ "MetricGroup": "Stall",
+ "MetricName": "Stall frontend tlb cycles"
+ },
+ {
+ "MetricExpr": "DTLB_WALK / L1D_TLB",
+ "BriefDescription": "D-side walk per d-side translation request",
+ "MetricGroup": "TLB",
+ "MetricName": "DTLB walks"
+ },
+ {
+ "MetricExpr": "ITLB_WALK / L1I_TLB",
+ "BriefDescription": "I-side walk per i-side translation request",
+ "MetricGroup": "TLB",
+ "MetricName": "ITLB walks"
+ },
+ {
+ "MetricExpr": "STALL_SLOT_BACKEND / (CPU_CYCLES * 4)",
+ "BriefDescription": "Fraction of slots backend bound",
+ "MetricGroup": "TopDownL1",

@Ian, should this be "Default;TopDownL1"?

+ "MetricName": "backend"

How about use consistent names with other other archs and arm64 platforms, like "backend_bound"? I did not check all names, but please consider this.

If 'perf topdown' is ever supported for arm64, we would prob rely on metricgroups, so would need use a fixed standard name here. Note that x86 uses custom kernel events for this instead.

That's an excellent point. I'll reach out to our architect and we'll change the names and groups in the patch and the document to be more consistent to the existing ones.

+ },
+ {
+ "MetricExpr": "1 - (retiring + lost + backend)",
+ "BriefDescription": "Fraction of slots frontend bound",
+ "MetricGroup": "TopDownL1",
+ "MetricName": "frontend"

As above, it would be "frontend_bound"

I'll fix it.

Cheers, Ilkka

+ },
+ {
+ "MetricExpr": "((OP_SPEC - OP_RETIRED) / (CPU_CYCLES * 4))",
+ "BriefDescription": "Fraction of slots lost due to
misspeculation",

+ "MetricGroup": "TopDownL1",
+ "MetricName": "lost"
+ },
+ {

Next message: Tian, Kevin: "RE: [PATCH v2 03/12] iommu: Remove unrecoverable fault data"
Previous message: Kees Cook: "Re: [PATCH 1/7] netfilter: ipset: refactor deprecated strncpy"
Next in thread: Ian Rogers: "Re: [PATCH 3/4] perf vendor events arm64: Add AmpereOne metrics"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]