From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Yicong Yang <yangyicong@huawei.com>
Cc: <yangyicong@hisilicon.com>, <acme@kernel.org>,
<namhyung@kernel.org>, <irogers@google.com>,
<john.g.garry@oracle.com>, <will@kernel.org>,
<james.clark@linaro.org>, <peterz@infradead.org>,
<mingo@redhat.com>, <mark.rutland@arm.com>,
<linux-perf-users@vger.kernel.org>, <prime.zeng@hisilicon.com>,
<hejunhao3@huawei.com>, <linuxarm@huawei.com>
Subject: Re: [PATCH] perf vender events arm64: Use "Topdown" as topdown metric group name
Date: Thu, 12 Sep 2024 11:25:59 +0100 [thread overview]
Message-ID: <20240912112559.00001485@Huawei.com> (raw)
In-Reply-To: <39921dd0-8827-31c2-6032-37a149af6b77@huawei.com>
On Thu, 12 Sep 2024 09:59:43 +0800
Yicong Yang <yangyicong@huawei.com> wrote:
> On 2024/9/11 19:28, Jonathan Cameron wrote:
> > On Mon, 9 Sep 2024 21:06:03 +0800
> > Yicong Yang <yangyicong@huawei.com> wrote:
> >
> >> another ping...
> >>
> >> This patch should be very straightforward, due to the changes to use "Topdown" after [1].
> >>
> >> [1] 1647cd5b8802 ("perf stat: Implement --topdown using json metrics")
> >
> > Hi Yicong,
> >
> > That reference definitely helps make this obvious given it clearly states
> > the pattern match is TopdownL.
>
> ok, I thounght the code link in the commit should be enough. Will refer to the commit
> as well.
Sure. Fine as is. I was just observing it made it easy to review
despite not knowing the code that well.
Jonathan
>
> Thanks.
>
> >
> > As such this LGTM (but I'm not a perf tool expert by any means!)
> > FWIW.
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> >>
> >> Thanks.
> >>
> >> On 2024/7/30 10:01, Yicong Yang wrote:
> >>> From: Yicong Yang <yangyicong@hisilicon.com>
> >>>
> >>> HiSilicon HIP08 does support Topdown metrics but perf tool complains
> >>> when trying to count Topdown metrics:
> >>> [root@localhost tracing]# perf stat --topdown
> >>> Topdown requested but the topdown metric groups aren't present.
> >>> (See perf list the metric groups have names like TopdownL1)
> >>>
> >>> It's because tool's using "Topdown" as the metric group name[1] rather
> >>> than "TopDown", so follow the convention.
> >>>
> >>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/builtin-stat.c?h=v6.11-rc1#n1994
> >>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> >>> ---
> >>> .../arch/arm64/hisilicon/hip08/metrics.json | 74 +++++++++----------
> >>> 1 file changed, 37 insertions(+), 37 deletions(-)
> >>>
> >>> diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
> >>> index 6463531b9941..b6a0d2de8534 100644
> >>> --- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
> >>> +++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
> >>> @@ -3,235 +3,235 @@
> >>> "MetricExpr": "FETCH_BUBBLE / (4 * CPU_CYCLES)",
> >>> "PublicDescription": "Frontend bound L1 topdown metric",
> >>> "BriefDescription": "Frontend bound L1 topdown metric",
> >>> - "DefaultMetricgroupName": "TopDownL1",
> >>> - "MetricGroup": "Default;TopDownL1",
> >>> + "DefaultMetricgroupName": "TopdownL1",
> >>> + "MetricGroup": "Default;TopdownL1",
> >>> "MetricName": "frontend_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "(INST_SPEC - INST_RETIRED) / (4 * CPU_CYCLES)",
> >>> "PublicDescription": "Bad Speculation L1 topdown metric",
> >>> "BriefDescription": "Bad Speculation L1 topdown metric",
> >>> - "DefaultMetricgroupName": "TopDownL1",
> >>> - "MetricGroup": "Default;TopDownL1",
> >>> + "DefaultMetricgroupName": "TopdownL1",
> >>> + "MetricGroup": "Default;TopdownL1",
> >>> "MetricName": "bad_speculation"
> >>> },
> >>> {
> >>> "MetricExpr": "INST_RETIRED / (CPU_CYCLES * 4)",
> >>> "PublicDescription": "Retiring L1 topdown metric",
> >>> "BriefDescription": "Retiring L1 topdown metric",
> >>> - "DefaultMetricgroupName": "TopDownL1",
> >>> - "MetricGroup": "Default;TopDownL1",
> >>> + "DefaultMetricgroupName": "TopdownL1",
> >>> + "MetricGroup": "Default;TopdownL1",
> >>> "MetricName": "retiring"
> >>> },
> >>> {
> >>> "MetricExpr": "1 - (frontend_bound + bad_speculation + retiring)",
> >>> "PublicDescription": "Backend Bound L1 topdown metric",
> >>> "BriefDescription": "Backend Bound L1 topdown metric",
> >>> - "DefaultMetricgroupName": "TopDownL1",
> >>> - "MetricGroup": "Default;TopDownL1",
> >>> + "DefaultMetricgroupName": "TopdownL1",
> >>> + "MetricGroup": "Default;TopdownL1",
> >>> "MetricName": "backend_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x201d@ / CPU_CYCLES",
> >>> "PublicDescription": "Fetch latency bound L2 topdown metric",
> >>> "BriefDescription": "Fetch latency bound L2 topdown metric",
> >>> - "MetricGroup": "TopDownL2",
> >>> + "MetricGroup": "TopdownL2",
> >>> "MetricName": "fetch_latency_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "frontend_bound - fetch_latency_bound",
> >>> "PublicDescription": "Fetch bandwidth bound L2 topdown metric",
> >>> "BriefDescription": "Fetch bandwidth bound L2 topdown metric",
> >>> - "MetricGroup": "TopDownL2",
> >>> + "MetricGroup": "TopdownL2",
> >>> "MetricName": "fetch_bandwidth_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "(bad_speculation * BR_MIS_PRED) / (BR_MIS_PRED + armv8_pmuv3_0@event\\=0x2013@)",
> >>> "PublicDescription": "Branch mispredicts L2 topdown metric",
> >>> "BriefDescription": "Branch mispredicts L2 topdown metric",
> >>> - "MetricGroup": "TopDownL2",
> >>> + "MetricGroup": "TopdownL2",
> >>> "MetricName": "branch_mispredicts"
> >>> },
> >>> {
> >>> "MetricExpr": "bad_speculation - branch_mispredicts",
> >>> "PublicDescription": "Machine clears L2 topdown metric",
> >>> "BriefDescription": "Machine clears L2 topdown metric",
> >>> - "MetricGroup": "TopDownL2",
> >>> + "MetricGroup": "TopdownL2",
> >>> "MetricName": "machine_clears"
> >>> },
> >>> {
> >>> "MetricExpr": "(EXE_STALL_CYCLE - (MEM_STALL_ANYLOAD + armv8_pmuv3_0@event\\=0x7005@)) / CPU_CYCLES",
> >>> "PublicDescription": "Core bound L2 topdown metric",
> >>> "BriefDescription": "Core bound L2 topdown metric",
> >>> - "MetricGroup": "TopDownL2",
> >>> + "MetricGroup": "TopdownL2",
> >>> "MetricName": "core_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "(MEM_STALL_ANYLOAD + armv8_pmuv3_0@event\\=0x7005@) / CPU_CYCLES",
> >>> "PublicDescription": "Memory bound L2 topdown metric",
> >>> "BriefDescription": "Memory bound L2 topdown metric",
> >>> - "MetricGroup": "TopDownL2",
> >>> + "MetricGroup": "TopdownL2",
> >>> "MetricName": "memory_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "(((L2I_TLB - L2I_TLB_REFILL) * 15) + (L2I_TLB_REFILL * 100)) / CPU_CYCLES",
> >>> "PublicDescription": "Idle by itlb miss L3 topdown metric",
> >>> "BriefDescription": "Idle by itlb miss L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "idle_by_itlb_miss"
> >>> },
> >>> {
> >>> "MetricExpr": "(((L2I_CACHE - L2I_CACHE_REFILL) * 15) + (L2I_CACHE_REFILL * 100)) / CPU_CYCLES",
> >>> "PublicDescription": "Idle by icache miss L3 topdown metric",
> >>> "BriefDescription": "Idle by icache miss L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "idle_by_icache_miss"
> >>> },
> >>> {
> >>> "MetricExpr": "(BR_MIS_PRED * 5) / CPU_CYCLES",
> >>> "PublicDescription": "BP misp flush L3 topdown metric",
> >>> "BriefDescription": "BP misp flush L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "bp_misp_flush"
> >>> },
> >>> {
> >>> "MetricExpr": "(armv8_pmuv3_0@event\\=0x2013@ * 5) / CPU_CYCLES",
> >>> "PublicDescription": "OOO flush L3 topdown metric",
> >>> "BriefDescription": "OOO flush L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "ooo_flush"
> >>> },
> >>> {
> >>> "MetricExpr": "(armv8_pmuv3_0@event\\=0x1001@ * 5) / CPU_CYCLES",
> >>> "PublicDescription": "Static predictor flush L3 topdown metric",
> >>> "BriefDescription": "Static predictor flush L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "sp_flush"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x1010@ / BR_MIS_PRED",
> >>> "PublicDescription": "Indirect branch L3 topdown metric",
> >>> "BriefDescription": "Indirect branch L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "indirect_branch"
> >>> },
> >>> {
> >>> "MetricExpr": "(armv8_pmuv3_0@event\\=0x1013@ + armv8_pmuv3_0@event\\=0x1016@) / BR_MIS_PRED",
> >>> "PublicDescription": "Push branch L3 topdown metric",
> >>> "BriefDescription": "Push branch L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "push_branch"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x100d@ / BR_MIS_PRED",
> >>> "PublicDescription": "Pop branch L3 topdown metric",
> >>> "BriefDescription": "Pop branch L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "pop_branch"
> >>> },
> >>> {
> >>> "MetricExpr": "(BR_MIS_PRED - armv8_pmuv3_0@event\\=0x1010@ - armv8_pmuv3_0@event\\=0x1013@ - armv8_pmuv3_0@event\\=0x1016@ - armv8_pmuv3_0@event\\=0x100d@) / BR_MIS_PRED",
> >>> "PublicDescription": "Other branch L3 topdown metric",
> >>> "BriefDescription": "Other branch L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "other_branch"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x2012@ / armv8_pmuv3_0@event\\=0x2013@",
> >>> "PublicDescription": "Nuke flush L3 topdown metric",
> >>> "BriefDescription": "Nuke flush L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "nuke_flush"
> >>> },
> >>> {
> >>> "MetricExpr": "1 - nuke_flush",
> >>> "PublicDescription": "Other flush L3 topdown metric",
> >>> "BriefDescription": "Other flush L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "other_flush"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x2010@ / CPU_CYCLES",
> >>> "PublicDescription": "Sync stall L3 topdown metric",
> >>> "BriefDescription": "Sync stall L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "sync_stall"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x2004@ / CPU_CYCLES",
> >>> "PublicDescription": "Rob stall L3 topdown metric",
> >>> "BriefDescription": "Rob stall L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "rob_stall"
> >>> },
> >>> {
> >>> "MetricExpr": "(armv8_pmuv3_0@event\\=0x2006@ + armv8_pmuv3_0@event\\=0x2007@ + armv8_pmuv3_0@event\\=0x2008@) / CPU_CYCLES",
> >>> "PublicDescription": "Ptag stall L3 topdown metric",
> >>> "BriefDescription": "Ptag stall L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "ptag_stall"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x201e@ / CPU_CYCLES",
> >>> "PublicDescription": "SaveOpQ stall L3 topdown metric",
> >>> "BriefDescription": "SaveOpQ stall L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "saveopq_stall"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x2005@ / CPU_CYCLES",
> >>> "PublicDescription": "PC buffer stall L3 topdown metric",
> >>> "BriefDescription": "PC buffer stall L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "pc_buffer_stall"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x7002@ / CPU_CYCLES",
> >>> "PublicDescription": "Divider L3 topdown metric",
> >>> "BriefDescription": "Divider L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "divider"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x7003@ / CPU_CYCLES",
> >>> "PublicDescription": "FSU stall L3 topdown metric",
> >>> "BriefDescription": "FSU stall L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "fsu_stall"
> >>> },
> >>> {
> >>> "MetricExpr": "core_bound - divider - fsu_stall",
> >>> "PublicDescription": "EXE ports util L3 topdown metric",
> >>> "BriefDescription": "EXE ports util L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "exe_ports_util"
> >>> },
> >>> {
> >>> "MetricExpr": "(MEM_STALL_ANYLOAD - MEM_STALL_L1MISS) / CPU_CYCLES",
> >>> "PublicDescription": "L1 bound L3 topdown metric",
> >>> "BriefDescription": "L1 bound L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "l1_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "(MEM_STALL_L1MISS - MEM_STALL_L2MISS) / CPU_CYCLES",
> >>> "PublicDescription": "L2 bound L3 topdown metric",
> >>> "BriefDescription": "L2 bound L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "l2_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "MEM_STALL_L2MISS / CPU_CYCLES",
> >>> "PublicDescription": "Mem bound L3 topdown metric",
> >>> "BriefDescription": "Mem bound L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "mem_bound"
> >>> },
> >>> {
> >>> "MetricExpr": "armv8_pmuv3_0@event\\=0x7005@ / CPU_CYCLES",
> >>> "PublicDescription": "Store bound L3 topdown metric",
> >>> "BriefDescription": "Store bound L3 topdown metric",
> >>> - "MetricGroup": "TopDownL3",
> >>> + "MetricGroup": "TopdownL3",
> >>> "MetricName": "store_bound"
> >>> }
> >>> ]
> >>>
> >
> > .
> >
next prev parent reply other threads:[~2024-09-12 10:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-30 2:01 [PATCH] perf vender events arm64: Use "Topdown" as topdown metric group name Yicong Yang
2024-08-14 9:31 ` Yicong Yang
2024-09-09 13:06 ` Yicong Yang
2024-09-11 11:28 ` Jonathan Cameron
2024-09-12 1:59 ` Yicong Yang
2024-09-12 10:25 ` Jonathan Cameron [this message]
2024-09-13 12:55 ` James Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240912112559.00001485@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=acme@kernel.org \
--cc=hejunhao3@huawei.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=john.g.garry@oracle.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=prime.zeng@hisilicon.com \
--cc=will@kernel.org \
--cc=yangyicong@hisilicon.com \
--cc=yangyicong@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.