From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935650AbcLTOmH (ORCPT ); Tue, 20 Dec 2016 09:42:07 -0500 Received: from outbound-smtp10.blacknight.com ([46.22.139.15]:37356 "EHLO outbound-smtp10.blacknight.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935211AbcLTOmE (ORCPT ); Tue, 20 Dec 2016 09:42:04 -0500 Date: Tue, 20 Dec 2016 14:42:02 +0000 From: Mel Gorman To: Michal Hocko Cc: Jia He , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Vlastimil Babka , Johannes Weiner , Joonsoo Kim , Taku Izumi Subject: Re: [PATCH RFC 1/1] mm, page_alloc: fix incorrect zone_statistics data Message-ID: <20161220144202.sgb5cqyt67vruosw@techsingularity.net> References: <1481522347-20393-1-git-send-email-hejianet@gmail.com> <1481522347-20393-2-git-send-email-hejianet@gmail.com> <20161220091814.GC3769@dhcp22.suse.cz> <20161220131040.f5ga5426dduh3mhu@techsingularity.net> <20161220132643.GG3769@dhcp22.suse.cz> <20161220142845.drbedcibjcggdxk7@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20161220142845.drbedcibjcggdxk7@techsingularity.net> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 20, 2016 at 02:28:45PM +0000, Mel Gorman wrote: > On Tue, Dec 20, 2016 at 02:26:43PM +0100, Michal Hocko wrote: > > On Tue 20-12-16 13:10:40, Mel Gorman wrote: > > > On Tue, Dec 20, 2016 at 10:18:14AM +0100, Michal Hocko wrote: > > > > On Mon 12-12-16 13:59:07, Jia He wrote: > > > > > In commit b9f00e147f27 ("mm, page_alloc: reduce branches in > > > > > zone_statistics"), it reconstructed codes to reduce the branch miss rate. > > > > > Compared with the original logic, it assumed if !(flag & __GFP_OTHER_NODE) > > > > > z->node would not be equal to preferred_zone->node. That seems to be > > > > > incorrect. > > > > > > > > I am sorry but I have hard time following the changelog. It is clear > > > > that you are trying to fix a missed NUMA_{HIT,OTHER} accounting > > > > but it is not really clear when such thing happens. You are adding > > > > preferred_zone->node check. preferred_zone is the first zone in the > > > > requested zonelist. So for the most allocations it is a node from the > > > > local node. But if something request an explicit numa node (without > > > > __GFP_OTHER_NODE which would be the majority I suspect) then we could > > > > indeed end up accounting that as a NUMA_MISS, NUMA_FOREIGN so the > > > > referenced patch indeed caused an unintended change of accounting AFAIU. > > > > > > > > > > This is a similar concern to what I had. If the preferred zone, which is > > > the first valid usable zone, is not a "hit" for the statistics then I > > > don't know what "hit" is meant to mean. > > > > But the first valid usable zone is defined based on the requested numa > > node. Unless the requested node is memoryless then we should have a hit, > > no? > > > > Should be. If the local node is memoryless then there would be a difference > between hit and whether it's local or not but that to me is a little > useless. A local vs remote page allocated has a specific meaning and > consequence. It's hard to see how hit can be meaningfully interpreted if > there are memoryless nodes. I don't have a strong objection to the patch > so I didn't nak it, I'm just not convinced it matters. > That said, it's also hard to interpret "hit" when memory policies forbid a memory node but allows the CPUs to be used. Local and remote can be interpreted regardless of machine, "hit" has meaning depending on the configuration and interpreting it is weird. It had some value when zone_reclaim_mode was always enabled because a low hit rate would imply zone_reclaim_mode was broken but that's a corner case. I would prefer to get rid of it because interpreting it is such a mess if there wasn't tools already collecting the data. -- Mel Gorman SUSE Labs From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f72.google.com (mail-wm0-f72.google.com [74.125.82.72]) by kanga.kvack.org (Postfix) with ESMTP id BC0C06B0327 for ; Tue, 20 Dec 2016 09:42:04 -0500 (EST) Received: by mail-wm0-f72.google.com with SMTP id m203so25401912wma.2 for ; Tue, 20 Dec 2016 06:42:04 -0800 (PST) Received: from outbound-smtp10.blacknight.com (outbound-smtp10.blacknight.com. [46.22.139.15]) by mx.google.com with ESMTPS id n13si19220223wmg.164.2016.12.20.06.42.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Dec 2016 06:42:03 -0800 (PST) Received: from mail.blacknight.com (pemlinmail01.blacknight.ie [81.17.254.10]) by outbound-smtp10.blacknight.com (Postfix) with ESMTPS id 216DC1C235D for ; Tue, 20 Dec 2016 14:42:03 +0000 (GMT) Date: Tue, 20 Dec 2016 14:42:02 +0000 From: Mel Gorman Subject: Re: [PATCH RFC 1/1] mm, page_alloc: fix incorrect zone_statistics data Message-ID: <20161220144202.sgb5cqyt67vruosw@techsingularity.net> References: <1481522347-20393-1-git-send-email-hejianet@gmail.com> <1481522347-20393-2-git-send-email-hejianet@gmail.com> <20161220091814.GC3769@dhcp22.suse.cz> <20161220131040.f5ga5426dduh3mhu@techsingularity.net> <20161220132643.GG3769@dhcp22.suse.cz> <20161220142845.drbedcibjcggdxk7@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20161220142845.drbedcibjcggdxk7@techsingularity.net> Sender: owner-linux-mm@kvack.org List-ID: To: Michal Hocko Cc: Jia He , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Vlastimil Babka , Johannes Weiner , Joonsoo Kim , Taku Izumi On Tue, Dec 20, 2016 at 02:28:45PM +0000, Mel Gorman wrote: > On Tue, Dec 20, 2016 at 02:26:43PM +0100, Michal Hocko wrote: > > On Tue 20-12-16 13:10:40, Mel Gorman wrote: > > > On Tue, Dec 20, 2016 at 10:18:14AM +0100, Michal Hocko wrote: > > > > On Mon 12-12-16 13:59:07, Jia He wrote: > > > > > In commit b9f00e147f27 ("mm, page_alloc: reduce branches in > > > > > zone_statistics"), it reconstructed codes to reduce the branch miss rate. > > > > > Compared with the original logic, it assumed if !(flag & __GFP_OTHER_NODE) > > > > > z->node would not be equal to preferred_zone->node. That seems to be > > > > > incorrect. > > > > > > > > I am sorry but I have hard time following the changelog. It is clear > > > > that you are trying to fix a missed NUMA_{HIT,OTHER} accounting > > > > but it is not really clear when such thing happens. You are adding > > > > preferred_zone->node check. preferred_zone is the first zone in the > > > > requested zonelist. So for the most allocations it is a node from the > > > > local node. But if something request an explicit numa node (without > > > > __GFP_OTHER_NODE which would be the majority I suspect) then we could > > > > indeed end up accounting that as a NUMA_MISS, NUMA_FOREIGN so the > > > > referenced patch indeed caused an unintended change of accounting AFAIU. > > > > > > > > > > This is a similar concern to what I had. If the preferred zone, which is > > > the first valid usable zone, is not a "hit" for the statistics then I > > > don't know what "hit" is meant to mean. > > > > But the first valid usable zone is defined based on the requested numa > > node. Unless the requested node is memoryless then we should have a hit, > > no? > > > > Should be. If the local node is memoryless then there would be a difference > between hit and whether it's local or not but that to me is a little > useless. A local vs remote page allocated has a specific meaning and > consequence. It's hard to see how hit can be meaningfully interpreted if > there are memoryless nodes. I don't have a strong objection to the patch > so I didn't nak it, I'm just not convinced it matters. > That said, it's also hard to interpret "hit" when memory policies forbid a memory node but allows the CPUs to be used. Local and remote can be interpreted regardless of machine, "hit" has meaning depending on the configuration and interpreting it is weird. It had some value when zone_reclaim_mode was always enabled because a low hit rate would imply zone_reclaim_mode was broken but that's a corner case. I would prefer to get rid of it because interpreting it is such a mess if there wasn't tools already collecting the data. -- Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org