Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932329AbdLTGr4 (ORCPT ); Wed, 20 Dec 2017 01:47:56 -0500 Received: from mga04.intel.com ([192.55.52.120]:38638 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932254AbdLTGrz (ORCPT ); Wed, 20 Dec 2017 01:47:55 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,430,1508828400"; d="scan'208";a="14132527" Subject: Re: [PATCH v2 2/5] mm: Extends local cpu counter vm_diff_nodestat from s8 to s16 To: Christopher Lameter , Michal Hocko Cc: Greg Kroah-Hartman , Andrew Morton , Vlastimil Babka , Mel Gorman , Johannes Weiner , YASUAKI ISHIMATSU , Andrey Ryabinin , Nikolay Borisov , Pavel Tatashin , David Rientjes , Sebastian Andrzej Siewior , Dave , Andi Kleen , Tim Chen , Jesper Dangaard Brouer , Ying Huang , Aaron Lu , Aubrey Li , Linux MM , Linux Kernel References: <1513665566-4465-1-git-send-email-kemi.wang@intel.com> <1513665566-4465-3-git-send-email-kemi.wang@intel.com> <20171219162029.GD2787@dhcp22.suse.cz> From: kemi Message-ID: Date: Wed, 20 Dec 2017 14:45:52 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2851 Lines: 63 On 2017年12月20日 01:21, Christopher Lameter wrote: > On Tue, 19 Dec 2017, Michal Hocko wrote: > >>> Well the reason for s8 was to keep the data structures small so that they >>> fit in the higher level cpu caches. The large these structures become the >>> more cachelines are used by the counters and the larger the performance >>> influence on the code that should not be impacted by the overhead. >> >> I am not sure I understand. We usually do not access more counters in >> the single code path (well, PGALLOC and NUMA counteres is more of an >> exception). So it is rarely an advantage that the whole array is in the >> same cache line. Besides that this is allocated by the percpu allocator >> aligns to the type size rather than cache lines AFAICS. > > I thought we are talking about NUMA counters here? > > Regardless: A typical fault, system call or OS action will access multiple > zone and node counters when allocating or freeing memory. Enlarging the > fields will increase the number of cachelines touched. > Yes, we add one more cache line footprint access theoretically. But I don't think it would be a problem. 1) Not all the counters need to be accessed in fast path of page allocation, the counters covered in a single cache line usually is enough for that, we probably don't need to access one more cache line. I tend to agree Michal's argument. Besides, in some slow path in which code is protected by zone lock or lru lock, access one more cache line would be a big problem since many other cache lines are also be accessed. 2) Enlarging vm_node_stat_diff from s8 to s16 gives an opportunity to keep more number in local cpus that provides the possibility of reducing the global counter update frequency. Thus, we can gain the benefit by reducing expensive cache bouncing. Well, if you still have some concerns, I can post some data for will-it-scale.page_fault1. What the benchmark does is: it forks nr_cpu processes and then each process does the following: 1 mmap() 128M anonymous space; 2 writes to each page there to trigger actual page allocation; 3 munmap() it. in a loop. https://github.com/antonblanchard/will-it-scale/blob/master/tests/page_fault1.c Or you can provide some other benchmarks on which you want to see performance impact. >> Maybe it used to be all different back then when the code has been added >> but arguing about cache lines seems to be a bit problematic here. Maybe >> you have some specific workloads which can prove me wrong? > > Run a workload that does some page faults? Heavy allocation and freeing of > memory? > > Maybe that is no longer relevant since the number of the counters is > large that the accesses are so sparse that each action pulls in a whole > cacheline. That would be something we tried to avoid when implementing > the differentials. > >