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@xxxxxxxxx>
>> Fixes: 6aa303defb74 ("mm, vmscan: only allocate and reclaim from zones with pages managed by the buddy allocator")
>> CC: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
>> ---
>> 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
