Date:   Wed, 15 Feb 2023 09:03:16 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        "Namhyung Kim" <namhyung@kernel.org>,
        Joe Mario <jmario@redhat.com>, Leo Yan <leo.yan@linaro.org>,
        <linux-perf-users@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Kan Liang <kan.liang@linux.intel.com>,
        Xing Zhengjun <zhengjun.xing@linux.intel.com>,
        <tim.c.chen@intel.com>
Subject: Re: [PATCH v2] perf c2c: Add report option to show false sharing in
 adjacent cachelines
Message-ID: <Y+wvVNWqXb70l4uy@feng-clx>
References: <20230214075823.246414-1-feng.tang@intel.com>
 <Y+vhcAYQrX6Lv7cL@kernel.org>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y+vhcAYQrX6Lv7cL@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?v344VkT0ekI8WgJTd0Cep8XoInOlyLTit78DYtqdJbW2luCb7TbNkY+79aT/?=
 =?us-ascii?Q?xzail1UXdB7zBocRQg8ExsCD+Dgb+sr2TVHmLmrhy5kIOvVvcdAH7oyGfJ4y?=
 =?us-ascii?Q?Qbal8EyzumQBf/DlR+DfECh+SO16tPNrdeZar5Autg5fI3N2TV9r+gmArnee?=
 =?us-ascii?Q?Zi9EgtJU/NEORbavG3a/Z1l+1w/1Bymqwdy/LO4yo/mPH60Fux/SjXFSeEpq?=
 =?us-ascii?Q?JqrytaeciPMpMoJTVpY/DbuMXM93cI0W+kp34B5S6J4qZQJK2H1ys+vskWXr?=
 =?us-ascii?Q?H/BVtc2aw3faP94zjw/sTY9obxKaUj8fSSzB/fYbt9T3LdREZleBdGIs0Jvg?=
 =?us-ascii?Q?iYHv2ZYuIWiYW8N7ekV8dM4K53qld/iO0NfXytCWCFvqSFYEukraLmJVI1m0?=
 =?us-ascii?Q?XOJtnhh6IjcY9fUvcEliZKlPvQYXtDvWp9eC18/wS2KHrI5JRocXimt3rHA9?=
 =?us-ascii?Q?6MOzptAETO/2VAL6OPax+NlBSqtOfc6U8IW0nnkG7kBLGGkFV2xKfDiJZepR?=
 =?us-ascii?Q?3Lyw4B+4vUOBYHWPQXHaMjlFK1NwQFF45u4XPAqJbNss9ud3Y18zBLiPVVJo?=
 =?us-ascii?Q?GohJ+8KMASeqi/MfjthTuGOQlRmuwmfO+/+1DC19d/dlvZeDyE75IszDKSXx?=
 =?us-ascii?Q?gJXkOChfDF3GiVPEh8f2YAhx9mNBIpS1YqpfzFTWjD1PBZA/FE6spjtPYXAR?=
 =?us-ascii?Q?4qwQuzOpcbs6Mchc1928/7kdyiOJOa146EEQsCHEoZWhujLBIIjYCLjhDIRD?=
 =?us-ascii?Q?j4QBYBTAbS/XRav6dm8zNRiIloCQB9r8UPOzyYryawsiGwj211sH0azxA10V?=
 =?us-ascii?Q?CztbD7X6GHink81gnia3zZsXYcaaJM3j7K7ph12LFcB5BeyS6vnNqcAu9hOm?=
 =?us-ascii?Q?KsMGbZpqKmmcPBUJ/Wlgzze3I52XsI1H0YvOdxXZWYrgNz7D+pfyU+Pf6WQ7?=
 =?us-ascii?Q?n/hN90AJehAwQaJ38dMXQ/YUNleDVGbHW0r9CvLRiGlKK3H748vecmj0bPsH?=
 =?us-ascii?Q?gP+QKotg0iNL7vAtkpBnvGD1UDb22fGYUKTiK5YicukLnPOswhGQJXo1d9sQ?=
 =?us-ascii?Q?Qaig2I6gXbkrs4WVSMEpkTTxMInZV7piJi6k2PGXuvbCbBTzkRqiYyWim1PG?=
 =?us-ascii?Q?5ToI70Cbh28359gFf3Ut1QAvNcOjWu1A08xvGJspJw/3sGMqYwbuu2tiy3aw?=
 =?us-ascii?Q?YPQkAVlYpLJEzYvWmAVFjSU6aCk7Z3uqxbC2ze5JfPPzUeodKuZQMqShrXG9?=
 =?us-ascii?Q?HAWkth1+PxBusVxYzoSk3UgcmqxW63R53j9Ue1n9zftG6r8j0YSQ0jqASkHB?=
 =?us-ascii?Q?cETgNRZew/PDKpdSlKQcqq5pPb9E/Wu5dOnPqmwjYaqu55c02rl7q55s7yz5?=
 =?us-ascii?Q?mC1HyDJZy+q+TGmeJq/+zb+EkfAru4z6BX0wiZCGUOlInJP+GNesvKQLQZua?=
 =?us-ascii?Q?u7d5pbuTVbtCWVvk/1yqmo5c2w0siIL2AxSfPMtn3Icon7WGhWGpD3Ul3uBn?=
 =?us-ascii?Q?STfpaeo1orWrjHSzyNLXs3NSdKnj7sxaVhV1NWQ7rBtn1Du4SwDmxITCpMgt?=
 =?us-ascii?Q?xwcObO26soCqTd0CWoIBaFxTZ/+l2Q9MoK+IsbC8?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9cb4f296-7a9e-4a60-5b3d-08db0ef1126a
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2023 01:07:56.4893
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CBfEcs47Vjb4yNjNAF0ApENHDlxLccfMfoZwLEgKmaoe+nj4blyxBiQ5qqEtCJsEtDWr9XFGQB+PA2slU4d1Qg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5088
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Arnaldo,

On Tue, Feb 14, 2023 at 04:30:56PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 14, 2023 at 03:58:23PM +0800, Feng Tang escreveu:
> > Many platforms have feature of adjacent cachelines prefetch, when it
> > is enabled, for data in RAM of 2 cachelines (2N and 2N+1) granularity,
> > if one is fetched to cache, the other one could likely be fetched too,
> > which sort of extends the cacheline size to double, thus the false
> > sharing could happens in adjacent cachelines.
> > 
> > 0Day has captured performance changed related with this [1], and some
> > commercial software explicitly makes its hot global variables 128 bytes
> > aligned (2 cache lines) to avoid this kind of extended false sharing.
> > 
> > So add an option "-a" or "--double-cl" for c2c report to show false
> > sharing in double cache line granularity, which acts just like the
> > cacheline size is doubled. There is no change to c2c record. The
> > hardware events of shared cacheline are still per cacheline, and
> > this option just changes the granularity of how events are grouped
> > and displayed.
> > 
> > In the c2c report below (will-it-scale's 'pagefault2' case on old kernel):
> > 
> >   ----------------------------------------------------------------------
> >      26       31        2        0        0        0  0xffff888103ec6000
> >   ----------------------------------------------------------------------
> >    35.48%   50.00%    0.00%    0.00%    0.00%   0x10     0       1  0xffffffff8133148b      1153        66       971     3748        74  [k] get_mem_cgroup_from_mm
> >     6.45%    0.00%    0.00%    0.00%    0.00%   0x10     0       1  0xffffffff813396e4       570         0      1531      879        75  [k] mem_cgroup_charge
> >    25.81%   50.00%    0.00%    0.00%    0.00%   0x54     0       1  0xffffffff81331472       949        70       593     3359        74  [k] get_mem_cgroup_from_mm
> >    19.35%    0.00%    0.00%    0.00%    0.00%   0x54     0       1  0xffffffff81339686      1352         0      1073     1022        74  [k] mem_cgroup_charge
> >     9.68%    0.00%    0.00%    0.00%    0.00%   0x54     0       1  0xffffffff813396d6      1401         0       863      768        74  [k] mem_cgroup_charge
> >     3.23%    0.00%    0.00%    0.00%    0.00%   0x54     0       1  0xffffffff81333106       618         0       804       11         9  [k] uncharge_batch
> > 
> > The offset 0x10 and 0x54 used to displayed in 2 groups, and now they
> > are listed together to give users a hint of extended false sharing.
> > 
> > [1]. https://lore.kernel.org/lkml/20201102091543.GM31092@shao2-debian/
> > 
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > Reviewed-by: Andi Kleen <ak@linux.intel.com>
> > Reviewed-by: Leo Yan <leo.yan@linaro.org>
> > Tested-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > Changelog:
> > 
> >   v1 -> v2:
> >   * Refine comments and fix typos (Leo Yan)
> >   * Add reviewd-by and tested-by(for Arm64) tag from Leo Yan
> >   * Refine cmd description and commit log to avoid using
> >     architecture specific name (Joe Mario)
> > 
> >  tools/perf/Documentation/perf-c2c.txt |  7 +++++++
> >  tools/perf/builtin-c2c.c              | 22 +++++++++++++---------
> >  tools/perf/util/cacheline.h           | 25 ++++++++++++++++++++-----
> >  tools/perf/util/sort.c                | 13 ++++++++++---
> >  tools/perf/util/sort.h                |  1 +
> >  5 files changed, 51 insertions(+), 17 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-c2c.txt b/tools/perf/Documentation/perf-c2c.txt
> > index 4e8c263e1721..cc18d61ec5d5 100644
> > --- a/tools/perf/Documentation/perf-c2c.txt
> > +++ b/tools/perf/Documentation/perf-c2c.txt
> > @@ -130,6 +130,13 @@ REPORT OPTIONS
> >  	The known limitations include exception handing such as
> >  	setjmp/longjmp will have calls/returns not match.
> >  
> > +-a::
> > +--double-cl::
> > +	Group the detection of shared cacheline events into double cacheline
> > +	granularity. Some architectures have an Adjacent Cacheline Prefetch
> > +	feature, which causes cacheline sharing to behave like the cacheline
> > +	size is doubled.
> > +
> 
> Humm, this is something not that usual, so I think we should have it
> just as --double-cl, ok?
> 
> I can do the adjustment here if you agree.

Sure. Many thanks for the review and suggestion!

- Feng

> Thanks,
> 
> - Arnaldo