[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <92868969-5f65-4e50-84c1-c50fe18656e8@redhat.com>
Date: Tue, 20 May 2025 10:58:59 +0200
From: David Hildenbrand <david@...hat.com>
To: Zi Yan <ziy@...dia.com>
Cc: Oscar Salvador <osalvador@...e.de>, Johannes Weiner <hannes@...xchg.org>,
linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Vlastimil Babka <vbabka@...e.cz>, Baolin Wang
<baolin.wang@...ux.alibaba.com>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
Mel Gorman <mgorman@...hsingularity.net>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
Brendan Jackman <jackmanb@...gle.com>, Richard Chang
<richardycc@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/4] mm/page_isolation: remove migratetype from
move_freepages_block_isolate()
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@...dia.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
Powered by blists - more mailing lists