Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 31 Aug 2020 09:55:17 +0100
From:   Mel Gorman <mgorman@suse.com>
To:     Feng Tang <feng.tang@intel.com>
Cc:     Borislav Petkov <bp@suse.de>, "Luck, Tony" <tony.luck@intel.com>,
        kernel test robot <rong.a.chen@intel.com>,
        LKML <linux-kernel@vger.kernel.org>, lkp@lists.01.org
Subject: Re: [LKP] Re: [x86/mce] 1de08dccd3: will-it-scale.per_process_ops
 -14.1% regression
Message-ID: <20200831085516.GE2976@suse.com>
References: <20200819020437.GA2605@shbuild999.sh.intel.com>
 <20200821020259.GA90000@shbuild999.sh.intel.com>
 <20200824151425.GF4794@zn.tnic>
 <20200824153300.GA56944@shbuild999.sh.intel.com>
 <20200824161238.GI4794@zn.tnic>
 <20200825062305.GA83850@shbuild999.sh.intel.com>
 <20200828174839.GD19448@zn.tnic>
 <20200831021638.GB65971@shbuild999.sh.intel.com>
 <20200831075611.GA2976@suse.com>
 <20200831082306.GA61340@shbuild999.sh.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <20200831082306.GA61340@shbuild999.sh.intel.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Aug 31, 2020 at 04:23:06PM +0800, Feng Tang wrote:
> On Mon, Aug 31, 2020 at 08:56:11AM +0100, Mel Gorman wrote:
> > On Mon, Aug 31, 2020 at 10:16:38AM +0800, Feng Tang wrote:
> > > > So why don't you define both variables with DEFINE_PER_CPU_ALIGNED and
> > > > check if all your bad measurements go away this way?
> > > 
> > > For 'arch_freq_scale', there are other percpu variables in the same
> > > smpboot.c: 'arch_prev_aperf' and 'arch_prev_mperf', and in hot path
> > > arch_scale_freq_tick(), these 3 variables are all accessed, so I didn't 
> > > touch it. Or maybe we can align the first of these 3 variables, so
> > > that they sit in one cacheline.
> > > 
> > > > You'd also need to check whether there's no detrimental effect from
> > > > this change on other, i.e., !KNL platforms, and I think there won't
> > > > be because both variables will be in separate cachelines then and all
> > > > should be good.
> > > 
> > > Yes, these kind of changes should be verified on other platforms.
> > > 
> > > One thing still puzzles me, that the 2 variables are per-cpu things, and
> > > there is no case of many CPU contending, why the cacheline layout matters?
> > > I doubt it is due to the contention of the same cache set, and am trying
> > > to find some way to test it.
> > > 
> > 
> > Because if you have two structures that are per-cpu and not cache-aligned
> > then a write in one can bounce the cache line in another due to
> > cache coherency protocol. It's generally called "false cache line
> > sharing". https://en.wikipedia.org/wiki/False_sharing has basic examples
> > (lets not get into whether wikipedia is a valid citation source, there
> > are books on the topic if someone really cared).
> 
> For 'arch_freq_scale' and 'tsc_adjust' percpu variable, they are only
> accessed by their own CPU, and usually no other CPU will touch them,

Read "false sharing again". Two adjacent per-CPU structures can still
interfere with each other if the structures happen to cross a cache line
boundary and are not cache aligned.

> the
> hot node path only use this_cpu_read/write/ptr. And each CPU's static
> percpu variables are all packed together in one area (256KB for one CPU on
> this test box),

If the structure is not cache aligned (probably 64KB) then there is a
boundary when cache line bounces can occur.

-- 
Mel Gorman
SUSE Labs