Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932225AbdLTGJk (ORCPT ); Wed, 20 Dec 2017 01:09:40 -0500 Received: from mga01.intel.com ([192.55.52.88]:60310 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932183AbdLTGJj (ORCPT ); Wed, 20 Dec 2017 01:09:39 -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="14125991" Subject: Re: [PATCH v2 4/5] mm: use node_page_state_snapshot to avoid deviation To: Michal Hocko Cc: Greg Kroah-Hartman , Andrew Morton , Vlastimil Babka , Mel Gorman , Johannes Weiner , Christopher Lameter , 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-5-git-send-email-kemi.wang@intel.com> <20171219124317.GP2787@dhcp22.suse.cz> From: kemi Message-ID: <94187fd5-ad70-eba7-2724-0fe5bed750d6@intel.com> Date: Wed, 20 Dec 2017 14:07:35 +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: <20171219124317.GP2787@dhcp22.suse.cz> 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: 3731 Lines: 90 On 2017年12月19日 20:43, Michal Hocko wrote: > On Tue 19-12-17 14:39:25, Kemi Wang wrote: >> To avoid deviation, this patch uses node_page_state_snapshot instead of >> node_page_state for node page stats query. >> e.g. cat /proc/zoneinfo >> cat /sys/devices/system/node/node*/vmstat >> cat /sys/devices/system/node/node*/numastat >> >> As it is a slow path and would not be read frequently, I would worry about >> it. > > The changelog doesn't explain why these counters needs any special > treatment. _snapshot variants where used only for internal handling > where the precision really mattered. We do not have any in-tree user and > Jack has removed this by http://lkml.kernel.org/r/20171122094416.26019-1-jack@suse.cz > which is already sitting in the mmotm tree. We can re-add it but that > would really require a _very good_ reason. > Assume we have *nr* cpus, and threshold size is *t*. Thus, the maximum deviation is nr*t. Currently, Skylake platform has hundreds of CPUs numbers and the number is still increasing. Also, even the threshold size is kept to 125 at maximum (32765 for NUMA counters now), the deviation is just a little too big as I have mentioned in the log. I tend to sum the number in local cpus up when query the global stats. Also, node_page_state_snapshot is only called in slow path and I don't think that would be a big problem. Anyway, it is a matter of taste. I just think it's better to have. >> Signed-off-by: Kemi Wang >> --- >> drivers/base/node.c | 17 ++++++++++------- >> mm/vmstat.c | 2 +- >> 2 files changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/base/node.c b/drivers/base/node.c >> index a045ea1..cf303f8 100644 >> --- a/drivers/base/node.c >> +++ b/drivers/base/node.c >> @@ -169,12 +169,15 @@ static ssize_t node_read_numastat(struct device *dev, >> "interleave_hit %lu\n" >> "local_node %lu\n" >> "other_node %lu\n", >> - node_page_state(NODE_DATA(dev->id), NUMA_HIT), >> - node_page_state(NODE_DATA(dev->id), NUMA_MISS), >> - node_page_state(NODE_DATA(dev->id), NUMA_FOREIGN), >> - node_page_state(NODE_DATA(dev->id), NUMA_INTERLEAVE_HIT), >> - node_page_state(NODE_DATA(dev->id), NUMA_LOCAL), >> - node_page_state(NODE_DATA(dev->id), NUMA_OTHER)); >> + node_page_state_snapshot(NODE_DATA(dev->id), NUMA_HIT), >> + node_page_state_snapshot(NODE_DATA(dev->id), NUMA_MISS), >> + node_page_state_snapshot(NODE_DATA(dev->id), >> + NUMA_FOREIGN), >> + node_page_state_snapshot(NODE_DATA(dev->id), >> + NUMA_INTERLEAVE_HIT), >> + node_page_state_snapshot(NODE_DATA(dev->id), NUMA_LOCAL), >> + node_page_state_snapshot(NODE_DATA(dev->id), >> + NUMA_OTHER)); >> } >> >> static DEVICE_ATTR(numastat, S_IRUGO, node_read_numastat, NULL); >> @@ -194,7 +197,7 @@ static ssize_t node_read_vmstat(struct device *dev, >> for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++) >> n += sprintf(buf+n, "%s %lu\n", >> vmstat_text[i + NR_VM_ZONE_STAT_ITEMS], >> - node_page_state(pgdat, i)); >> + node_page_state_snapshot(pgdat, i)); >> >> return n; >> } >> diff --git a/mm/vmstat.c b/mm/vmstat.c >> index 64e08ae..d65f28d 100644 >> --- a/mm/vmstat.c >> +++ b/mm/vmstat.c >> @@ -1466,7 +1466,7 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat, >> for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++) { >> seq_printf(m, "\n %-12s %lu", >> vmstat_text[i + NR_VM_ZONE_STAT_ITEMS], >> - node_page_state(pgdat, i)); >> + node_page_state_snapshot(pgdat, i)); >> } >> } >> seq_printf(m, >> -- >> 2.7.4 >> >