Re: [PATCH v4 2/4] mm/page_isolation: remove migratetype from move_freepages_block_isolate()

[David Hildenbrand <david@redhat.com>]

On 20.05.25 01:06, Zi Yan wrote:
> On 19 May 2025, at 4:21, David Hildenbrand wrote:
> 
>> On 09.05.25 22:01, Zi Yan wrote:
>>> Since migratetype is no longer overwritten during pageblock isolation,
>>> moving pageblocks to and from MIGRATE_ISOLATE no longer needs migratetype.
>>>
>>> Add MIGRATETYPE_NO_ISO_MASK to allow read before-isolation migratetype
>>> when a pageblock is isolated. It is used by move_freepages_block_isolate().
>>>
>>> Add pageblock_isolate_and_move_free_pages() and
>>> pageblock_unisolate_and_move_free_pages() to be explicit about the page
>>> isolation operations. Both share the common code in
>>> __move_freepages_block_isolate(), which is renamed from
>>> move_freepages_block_isolate().
>>>
>>> Make set_pageblock_migratetype() only accept non MIGRATE_ISOLATE types,
>>> so that one should use set_pageblock_isolate() to isolate pageblocks.
>>>
>>> Two consequential changes:
>>> 1. move pageblock migratetype code out of __move_freepages_block().
>>> 2. in online_pages() from mm/memory_hotplug.c, move_pfn_range_to_zone() is
>>>      called with MIGRATE_MOVABLE instead of MIGRATE_ISOLATE and all affected
>>>      pageblocks are isolated afterwards. Otherwise, all online pageblocks
>>>      will have non-determined migratetype.
>>>
>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>> ---
>>>    include/linux/mmzone.h         |  4 +-
>>>    include/linux/page-isolation.h |  5 ++-
>>>    mm/memory_hotplug.c            |  7 +++-
>>>    mm/page_alloc.c                | 73 +++++++++++++++++++++++++---------
>>>    mm/page_isolation.c            | 27 ++++++++-----
>>>    5 files changed, 82 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>>> index 7ef01fe148ce..f66895456974 100644
>>> --- a/include/linux/mmzone.h
>>> +++ b/include/linux/mmzone.h
>>> @@ -107,8 +107,10 @@ static inline bool migratetype_is_mergeable(int mt)
>>>    extern int page_group_by_mobility_disabled;
>>>     #ifdef CONFIG_MEMORY_ISOLATION
>>> -#define MIGRATETYPE_MASK ((BIT(PB_migratetype_bits) - 1) | PB_migrate_isolate_bit)
>>> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
>>> +#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit)
>>>    #else
>>> +#define MIGRATETYPE_NO_ISO_MASK MIGRATETYPE_MASK
>>>    #define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
>>>    #endif
>>>   diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
>>> index 898bb788243b..b0a2af0a5357 100644
>>> --- a/include/linux/page-isolation.h
>>> +++ b/include/linux/page-isolation.h
>>> @@ -26,9 +26,10 @@ static inline bool is_migrate_isolate(int migratetype)
>>>    #define REPORT_FAILURE	0x2
>>>     void set_pageblock_migratetype(struct page *page, int migratetype);
>>> +void set_pageblock_isolate(struct page *page);
>>>   -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>>> -				  int migratetype);
>>> +bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page);
>>> +bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page);
>>>     int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>>>    			     int migratetype, int flags);
>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>> index b1caedbade5b..c86c47bba019 100644
>>> --- a/mm/memory_hotplug.c
>>> +++ b/mm/memory_hotplug.c
>>> @@ -1178,6 +1178,7 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
>>>    	const int nid = zone_to_nid(zone);
>>>    	int ret;
>>>    	struct memory_notify arg;
>>> +	unsigned long isol_pfn;
>>>     	/*
>>>    	 * {on,off}lining is constrained to full memory sections (or more
>>> @@ -1192,7 +1193,11 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
>>>       	/* associate pfn range with the zone */
>>> -	move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_ISOLATE);
>>> +	move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_MOVABLE);
>>> +	for (isol_pfn = pfn;
>>> +	     isol_pfn < pfn + nr_pages;
>>> +	     isol_pfn += pageblock_nr_pages)
>>> +		set_pageblock_isolate(pfn_to_page(isol_pfn));
>>
>> Can we move that all the way into memmap_init_range(), where we do the
>> set_pageblock_migratetype()?
>>
>> The MIGRATE_UNMOVABLE in mhp_init_memmap_on_memory() is likely fine: all
>> pages in that pageblock will be used for the memmap. Everything is unmovable,
>> but no free pages so ... nobody cares? :)
> 
> My approach is similar, but a new init_pageblock_migratetype() like
> below. Then, I added "bool isolate" instead of replacing the existing
> "int migratetype". The advantage is that it saves a call to
> set_pfnblock_flags_mask() for each pageblock. Like the alternative
> you suggested below.
> 
> +void __meminit init_pageblock_migratetype(struct page *page, int migratetype,
> +		bool isolate)
> +{
> +	if (unlikely(page_group_by_mobility_disabled &&
> +		     migratetype < MIGRATE_PCPTYPES))
> +		migratetype = MIGRATE_UNMOVABLE;
> +
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	if (migratetype == MIGRATE_ISOLATE) {
> +		VM_WARN(1,
> +			"Set isolate=true to isolate pageblock with a migratetype");
> +		return;
> +	}
> +	if (isolate)
> +		migratetype |= PB_migrate_isolate_bit;
> +#endif
> +	set_pfnblock_flags_mask(page, (unsigned long)migratetype,
> +				page_to_pfn(page), MIGRATETYPE_MASK);
> +}
> +

See my other reply on maybe introducing a "struct pageblock_info" where 
we embed these things, to decouple the actual migratetype from flags.

-- 
Cheers,

David / dhildenb