From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by kanga.kvack.org (Postfix) with ESMTP id 0616A6B0008 for ; Tue, 13 Nov 2018 03:08:38 -0500 (EST) Received: by mail-ed1-f69.google.com with SMTP id d41so20269eda.12 for ; Tue, 13 Nov 2018 00:08:37 -0800 (PST) Received: from mx1.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id c19-v6si7087105ejb.322.2018.11.13.00.08.36 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Nov 2018 00:08:36 -0800 (PST) Date: Tue, 13 Nov 2018 09:08:34 +0100 From: Michal Hocko Subject: Re: [PATCH] mm, page_alloc: skip zone who has no managed_pages in calculate_totalreserve_pages() Message-ID: <20181113080834.GK15120@dhcp22.suse.cz> References: <20181112071404.13620-1-richard.weiyang@gmail.com> <20181112080926.GA14987@dhcp22.suse.cz> <20181112142641.6oxn4fv4pocm7fmt@master> <20181112144020.GC14987@dhcp22.suse.cz> <20181113013942.zgixlky4ojbzikbd@master> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181113013942.zgixlky4ojbzikbd@master> Sender: owner-linux-mm@kvack.org List-ID: To: Wei Yang Cc: akpm@linux-foundation.org, mgorman@techsingularity.net, linux-mm@kvack.org On Tue 13-11-18 01:39:42, Wei Yang wrote: > On Mon, Nov 12, 2018 at 03:40:20PM +0100, Michal Hocko wrote: > >On Mon 12-11-18 14:26:41, Wei Yang wrote: > >> On Mon, Nov 12, 2018 at 09:09:26AM +0100, Michal Hocko wrote: > >> >On Mon 12-11-18 15:14:04, Wei Yang wrote: > >> >> Zone with no managed_pages doesn't contribute totalreserv_pages. And the > >> >> more nodes we have, the more empty zones there are. > >> >> > >> >> This patch skip the zones to save some cycles. > >> > > >> >What is the motivation for the patch? Does it really cause any > >> >measurable difference in performance? > >> > > >> > >> The motivation here is to reduce some unnecessary work. > > > >I have guessed so even though the changelog was quite modest on the > >motivation. > > > >> Based on my understanding, almost every node has empty zones, since > >> zones within a node are ordered in monotonic increasing memory address. > > > >Yes, this is likely the case. Btw. a check for populated_zone or > >for_each_populated_zone would suite much better. > > > > Hmm... maybe not exact. > > populated_zone checks zone->present_pages > managed_zone checks zone->managed_pages > > As the comment of managed_zone says, this one records the pages managed > by buddy system. And when we look at the usage of totalreserve_pages, it > is only used in page allocation. And finally, *max* is checked with > managed_pages instead of present_pages. > > Because of this, managed_zone is more accurate at this place. Is my > understanding correct? OK, fair enough. There is a certain discrepancy here. You are right that we do not care about pages out of the page allocator scope (e.g. early bootmem allocations, struct pages) but this is likely what other callers of populated_zone are looking for as well. It seems that managed pages counter which only came in later was not considered in other places. That being said this asks for a cleanup of some sort. And I think such a cleanup wold be appreciated much more than an optimization of an unknown effect and wonder why this check is used here and not at other places. -- Michal Hocko SUSE Labs