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:   Sat, 29 Jan 2022 00:27:38 +0000
From:   Wei Yang <richard.weiyang@gmail.com>
To:     David Hildenbrand <david@redhat.com>
Cc:     Wei Yang <richard.weiyang@gmail.com>, akpm@linux-foundation.org,
        mgorman@techsingularity.net, mhocko@suse.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/memory_hotplug: build zonelist for managed_zone
Message-ID: <20220129002738.ofoewhgb4mwfwqfj@master>
Reply-To: Wei Yang <richard.weiyang@gmail.com>
References: <20220127012023.18095-1-richard.weiyang@gmail.com>
 <f23028db-a351-e6b0-6c17-f14191ab3985@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f23028db-a351-e6b0-6c17-f14191ab3985@redhat.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Precedence: bulk

On Thu, Jan 27, 2022 at 09:39:56AM +0100, David Hildenbrand wrote:
>On 27.01.22 02:20, Wei Yang wrote:
>> During memory hotplug, when online/offline a zone, we need to rebuild
>> the zonelist for all node. There are two checks to decide whether a zone
>> would be added to zonelist:
>> 
>>   * one in online_pages/offline_pages to decide necessity
>>   * one in build_zonerefs_node to do real add
>> 
>> Currently we use different criteria at these two places, which is
>> different from the original behavior.
>> 
>> Originally during memory hotplug, zonelist is re-built when zone hasn't
>> been populated. This in introduced in 'commit 6811378e7d8b ("[PATCH]
>> wait_table and zonelist initializing for memory hotadd: update zonelists")'.
>> And at that moment, build_zonelists_node() also use populated_zone() to
>> decide whether the zone should be added to zonelist.
>> 
>> While in 'commit 6aa303defb74 ("mm, vmscan: only allocate and reclaim
>> from zones with pages managed by the buddy allocator")',
>> build_zonelists_node() changed to use managed_zone() to add zonelist.
>> But we still use populated_zone() to decide the necessity.
>> 
>> This patch restore the original behavior by using the same criteria to
>> add a zone in zonelist during memory hotplug.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Fixes: 6aa303defb74 ("mm, vmscan: only allocate and reclaim from zones with pages managed by the buddy allocator")
>> CC: Mel Gorman <mgorman@techsingularity.net>
>> ---
>>  mm/memory_hotplug.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 2a9627dc784c..8f1906b33937 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1102,11 +1102,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
>>  	spin_unlock_irqrestore(&zone->lock, flags);
>>  
>>  	/*
>> -	 * If this zone is not populated, then it is not in zonelist.
>> +	 * If this zone is not managed, then it is not in zonelist.
>>  	 * This means the page allocator ignores this zone.
>>  	 * So, zonelist must be updated after online.
>>  	 */
>> -	if (!populated_zone(zone)) {
>> +	if (!managed_zone(zone)) {
>>  		need_zonelists_rebuild = 1;
>>  		setup_zone_pageset(zone);
>>  	}
>> @@ -1985,7 +1985,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages,
>>  	/* reinitialise watermarks and update pcp limits */
>>  	init_per_zone_wmark_min();
>>  
>> -	if (!populated_zone(zone)) {
>> +	if (!managed_zone(zone)) {
>>  		zone_pcp_reset(zone);
>>  		build_all_zonelists(NULL);
>>  	}
>
>A note that managed_zone() is a moving target w.r.t. memory ballooning.
>In extreme cases, we can have whole zones (temporarily) be completely
>!managed for that reason.
>
>IMHO memory hot(un)plug is usually the wrong place to check for
>managed_zone(), it cares about populated_zone().
>

So we need to check populated_zone when building zonelist?

>-- 
>Thanks,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me