Messages in this thread Patch in this message | | | Date | Thu, 3 Feb 2022 17:19:34 +0800 | From | Leo Yan <> | Subject | Re: [PATCH] perf arm-spe: Use SPE data source for neoverse cores |
| |
On Wed, Feb 02, 2022 at 07:51:15PM +0000, Ali Saidi wrote:
[...]
> >> >> + data_src.mem_lvl = PERF_MEM_LVL_L3 | PERF_MEM_LVL_HIT; > >> > > >> >This one also adds PERF_MEM_LVL_HIT even though the check of "if (record->type & ARM_SPE_LLC_MISS)" > >> >hasn't happened yet. Maybe some comments would make it a bit clearer, but at the moment it's > >> >not obvious how the result is derived because there are some things that don't add up like > >> >ARM_SPE_LLC_MISS == PERF_MEM_LVL_HIT. > >> > >> Assuming the above is correct, my reading of the existing code that creates the > >> c2c output is that when an access is marked as an LLC hit, that doesn't > >> necessarily mean that the data was present in the LLC. I don't see how it could > >> given that LLC_HIT + HITM means the line was dirty in another CPUs cache, and so > >> LLC_HIT + HITM seems to mean that it was a hit in the LLC snoop filter and > >> required a different core to provide the line. This and the above certainly > >> deserve a comment as to why the miss is being attributed in this way if it's > >> otherwise acceptable. > > > >As James pointed out, this might introduce confusion. I am wanderding > >if we can extract two functions for synthesizing the data source, one is > >for Neoverse platform and another is for generic purpose (which > >without data source packets), below code is to demonstrate the basic > >idea. > > The code below is cleaner, and I'm happy to rework the patches in this way, but > I think the question still remains about unifying behavior of the tool. If we > mark something with a data source of ARM_SPE_NV_PEER_CORE as at L1 hit + HITM > certainly c2c won't show the correct thing today, but i think it also hides the > intent. The line in question missed the L1, L2, and got to the LLC where we did > find a record that it was in another cores cache (L1 or L2). Looking at the way > that c2c works today, it seems like marking this as a hit in the LLC snoop > filter is the best way to unify behaviors among architectures?
Thanks a lot for pointing out this. I looked into the code and compared the memory trace data from x86, I found the HITM tag is always sticking to LLC from x86's memory events. This would be the main reason why current code in perf is only support HITM for LLC.
I don't think it's a good way to always mark LLC snoop, even it's a snooping operation in L1 or L2 cache on Arm64 platforms; this would introduce confusion for users when using Arm SPE for profiling.
Alternatively, we can support HITM tag for L1/L2 cache levels in perf, this can allow us to match memory topology on Arm64 arch, and it should not introduce any regression on x86 arch.
Could you confirm if below code can fix the issue or not?
From 56e70a9ca974ad062788cfccd2ae9ddafa13d3ae Mon Sep 17 00:00:00 2001 From: Leo Yan <leo.yan@linaro.org> Date: Thu, 3 Feb 2022 16:53:34 +0800 Subject: [PATCH] perf mem: Support HITM statistics for L1/L2 caches
Current code only support HITM statistics for last level cache (LLC) and remote node's cache. This works for x86 architecture since the HITM tag is associated with LLC but not with L1/L2 cache.
On Arm64 architectures, due to the different memory hierarchy and topology, the snooping can happen on L1 or L2 cache line, and thus it's possible that coherency protocol fetches data from peer core or cluster's L1/L2 cache. For this reason, HITM tag is not necessarily bound to LLC anymore.
For a general solution, this patch extends to set HITM tag for L1 and L2 cache, thus this can allow perf c2c tool to work properly for Arm64 architecture. On the other hand, since x86 architecture doesn't set HITM tag for L1/L2 cache, thus this patch should not introduce any functionality change for x86 platforms.
Signed-off-by: Leo Yan <leo.yan@linaro.org> --- tools/perf/util/mem-events.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c index ed0ab838bcc5..7a0ab3d26843 100644 --- a/tools/perf/util/mem-events.c +++ b/tools/perf/util/mem-events.c @@ -527,8 +527,18 @@ do { \ if (lvl & P(LVL, UNC)) stats->ld_uncache++; if (lvl & P(LVL, IO)) stats->ld_io++; if (lvl & P(LVL, LFB)) stats->ld_fbhit++; - if (lvl & P(LVL, L1 )) stats->ld_l1hit++; - if (lvl & P(LVL, L2 )) stats->ld_l2hit++; + if (lvl & P(LVL, L1 )) { + if (snoop & P(SNOOP, HITM)) + HITM_INC(lcl_hitm); + else + stats->ld_l1hit++; + } + if (lvl & P(LVL, L2 )) { + if (snoop & P(SNOOP, HITM)) + HITM_INC(lcl_hitm); + else + stats->ld_l2hit++; + } if (lvl & P(LVL, L3 )) { if (snoop & P(SNOOP, HITM)) HITM_INC(lcl_hitm); > I'll send you a perf.data file OOB.
Very appreciate! I will look into it and will let you know if I have any new finding.
Thanks, Leo
| |