Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ali Saidi <alisaidi@amazon.com>
To:     <leo.yan@linaro.org>
CC:     <Al.Grant@arm.com>, <acme@kernel.org>,
        <alexander.shishkin@linux.intel.com>, <alisaidi@amazon.com>,
        <andrew.kilroy@arm.com>, <benh@kernel.crashing.org>,
        <german.gomez@arm.com>, <james.clark@arm.com>,
        <john.garry@huawei.com>, <jolsa@redhat.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-perf-users@vger.kernel.org>,
        <mark.rutland@arm.com>, <mathieu.poirier@linaro.org>,
        <mingo@redhat.com>, <namhyung@kernel.org>, <peterz@infradead.org>,
        <will@kernel.org>
Subject: Re: [PATCH] perf arm-spe: Use SPE data source for neoverse cores
Date:   Wed, 2 Feb 2022 19:51:15 +0000
Message-ID: <20220202195115.12924-1-alisaidi@amazon.com>
In-Reply-To: <20220202150104.GB1939745@leoy-ThinkPad-X240s>
References: <20220202150104.GB1939745@leoy-ThinkPad-X240s>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk

Hi Leo,

>Hi Ali, James,
>
> [...]
>> 
>> I'd really like someone familiar with perf c2c output to also end up getting
>> similar output when running on an Arm system with SPE. There are obviously large
>> micro-architectural differences that have been abstracted away by the data_src
>> abstraction but fundamentally my understanding of x86 HITM is that the line
>> was found in the snoop filter of the LLC as being owned by another core and
>> therefore the request needs to go to another core to get the line.  I'm not
>> 100% sure if on x86 it's really guaranteed to be dirty or not and it's not
>> always going to be dirty in a Neoverse system, but since the SPE source
>> indicates it was sourced from another core it is a core-2-core transfer of a
>> line which is currently owned by another cpu core and that is the interesting
>> data point that would be used to drive optimization and elimination of frequent
>> core-2-core transfers (true or false sharing).
>
>Though I don't know the implementation for the hardware conherency
>protocols, here I have the same understanding with Ali.
>
>For x86 arch, it uses HITM to indicate the cache line is "modified"
>state; on Arm64 Neoverse platforms, there have two data source values
>can tell us if the cache line is "modified" state:
>ARM_SPE_NV_PEER_CLSTR and ARM_SPE_NV_PEER_CORE.  The snooping can
>happen either within the cluster or cross clusters.

Yes, although it depends on the system topology if there are clusters.

>> >> +			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?

I'll send you a perf.data file OOB.

Thanks,
Ali