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:     <acme@kernel.org>, <peterz@infradead.org>, <mingo@redhat.com>,
        <mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>,
        <jolsa@kernel.org>, <namhyung@kernel.org>, <hi@alyssa.is>,
        <irogers@google.com>, <likexu@tencent.com>, <kjain@linux.ibm.com>,
        <lihuafei1@huawei.com>, <adam.li@amperecomputing.com>,
        <german.gomez@arm.com>, <james.clark@arm.com>,
        <alisaidi@amazon.com>, <linux-perf-users@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 00/12] perf c2c: Support display for Arm64
Date:   Thu, 2 Jun 2022 17:11:20 +0000
Message-ID: <20220602171120.31166-1-alisaidi@amazon.com>
In-Reply-To: <20220601102505.GA408721@leoy-ThinkPad-X240s>
References: <20220601102505.GA408721@leoy-ThinkPad-X240s>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk

Hi Leo,

On Wed, 1 Jun 2022 18:25:05 +0800 Leo Yan wrote:
> Hi Joe,
> 
> On Tue, May 31, 2022 at 02:44:07PM -0400, Joe Mario wrote:
> 
> [...]
> 
> > Hi Leo:
> > I built a new perf with your patches and ran it on a 2-numa node Neoverse platform.
> > I then ran my simple test that creates reader and writer threads to tug on the same cacheline.
> > The c2c output is appended below.
> >
> > The output looks good, especially where you've broken out the (average) cycles for local and remote peer loads.  
> > And I'm glad to see you fixed the "Node" column.  I use that a lot to help detect remote node accesses.  
> 
> Thanks a lot for your testing and suggestions, which are really helpful!
> 
> > And the "PA cnt" field is working as well,  which is important to see if numa_balance is moving the data around.
> 
> Good to know this.  To be honest, before I didn't note for "PA cnt"
> metric, I checked a bit for the code, this metrics is very useful to
> understand how it's severe that a cache line is accessed from different
> addresses, so we can get sense how a cache line is hammered.
> 
> [...]
>
> > Thanks for doing this.  It looks good.
> 
> You are welcome!  And very appreicate your helping to mature the code.

Seconding that, thanks for progressing this so much Leo. 

> 
> > I'll assume someone else is reviewing your code changes.
> 
> Yeah, let's give a bit more time for reviewing.

I've tested and given each patch a close look. I haven't found anything that
looks to change other architectures and the output on my Graviton systems looks
great. I pulled in your patch to add physical addresses to the spe records and
as expected I saw the Node properly populated and PA cnt is no longer zero.  One
nit is the documentation still says that "Total HITMs (tot) as default," while
the code now defaults to "peer" on arm64.  Other than that:

Tested-by: Ali Saidi <alisaidi@amazon.com>
Reviewed-by: Ali Saidi <alisaidi@amazon.com>

Thanks,
Ali