Re: [PATCH 06/49] mm/mm_init: fix uninitialized pageblock migratetype for ZONE_DEVICE compound pages

[Muchun Song]

> On Apr 13, 2026, at 21:28, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
> 
> On Mon, Apr 13, 2026 at 06:07:44PM +0800, Muchun Song wrote:
>> 
>> 
>>>> On Apr 13, 2026, at 17:32, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>>> 
>>> On Sun, Apr 05, 2026 at 08:51:57PM +0800, Muchun Song wrote:
>>>> Previously, memmap_init_zone_device() only initialized the migratetype
>>>> of the first pageblock of a compound page. If the compound page size
>>>> exceeds pageblock_nr_pages (e.g., 1GB hugepages with 2MB pageblocks),
>>>> subsequent pageblocks in the compound page would remain uninitialized.
>>>> 
>>>> This patch moves the migratetype initialization out of
>>>> __init_zone_device_page() and into a separate function
>>>> pageblock_migratetype_init_range(). This function iterates over the
>>>> entire PFN range of the memory, ensuring that all pageblocks are correctly
>>>> initialized.
>>>> 
>>>> Fixes: c4386bd8ee3a ("mm/memremap: add ZONE_DEVICE support for compound pages")
>>>> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
>>>> ---
>>>> mm/mm_init.c | 41 ++++++++++++++++++++++++++---------------
>>>> 1 file changed, 26 insertions(+), 15 deletions(-)
>>>> 
>>>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>>>> index 9a44e8458fed..4936ca78966c 100644
>>>> --- a/mm/mm_init.c
>>>> +++ b/mm/mm_init.c
>>>> @@ -674,6 +674,18 @@ static inline void fixup_hashdist(void)
>>>> static inline void fixup_hashdist(void) {}
>>>> #endif /* CONFIG_NUMA */
>>>> 
>>>> +static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
>>>> +        unsigned long nr_pages,
>>>> +        int migratetype)
>>>> +{
>>>> + unsigned long end = pfn + nr_pages;
>>>> +
>>>> + for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
>>>> + init_pageblock_migratetype(pfn_to_page(pfn), migratetype, false);
>>>> + cond_resched();
>>> 
>>> Do we need to call cond_resched() every iteration here?
>> 
>> Of course not.
>> 
>>> 
>>>> + }
>>>> +}
>>>> +
>>>> /*
>>>> * Initialize a reserved page unconditionally, finding its zone first.
>>>> */
>>>> @@ -1011,21 +1023,6 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
>>>> page_folio(page)->pgmap = pgmap;
>>>> page->zone_device_data = NULL;
>>>> 
>>>> - /*
>>>> -  * Mark the block movable so that blocks are reserved for
>>>> -  * movable at startup. This will force kernel allocations
>>>> -  * to reserve their blocks rather than leaking throughout
>>>> -  * the address space during boot when many long-lived
>>>> -  * kernel allocations are made.
>>>> -  *
>>>> -  * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>>>> -  * because this is done early in section_activate()
>>>> -  */
>>>> - if (pageblock_aligned(pfn)) {
>>>> - init_pageblock_migratetype(page, MIGRATE_MOVABLE, false);
>>>> - cond_resched();
>>>> - }
>>>> -
>>>> /*
>>>> * ZONE_DEVICE pages other than MEMORY_TYPE_GENERIC are released
>>>> * directly to the driver page allocator which will set the page count
>>>> @@ -1122,6 +1119,8 @@ void __ref memmap_init_zone_device(struct zone *zone,
>>>> 
>>>> __init_zone_device_page(page, pfn, zone_idx, nid, pgmap);
>>>> 
>>>> + cond_resched();
>>> 
>>> Originally we called cond_resched() once per pageblock, now it's called
>>> once per page plus for every pageblock in the tight loop that sets the
>>> migrate type. Isn't it too much?
>> 
>> There are indeed many more cond_resched() than before, but I don’t have a
>> concise way to write it, so I took the easy way out.
>> 
>> How about:
>> 
>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>> index 2d680636b67a..d13a2577c4c3 100644
>> --- a/mm/mm_init.c
>> +++ b/mm/mm_init.c
>> @@ -683,7 +683,8 @@ static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
>> 
>>        for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
>>                init_pageblock_migratetype(pfn_to_page(pfn), migratetype, isolate);
>> -               cond_resched();
>> +               if ((pfn & (pageblock_nr_pages * 512 - 1)) == 0)
>> +                       cond_resched();
> 
> TBH, I don't know if 512 is the right number :)

I picked this number out of thin air. I’m not sure as well.

> 
> pageblock boundary was a convenient point to have cond_resched() along with
> setting the migrate type.
> 
> Here we might be able to squeeze more iterations between subsequent
> cond_resched() calls.

Choosing an appropriate number is indeed difficult. We need to consider
both unnecessary cond_resched() calls and scheduling latency, and the
specific value also depends on the current CPU performance. At the very least,
I find it hard to choose a correct value. Since we all agree it should be larger
than 512, why not just pick 1024? I can't think of a good evaluation criterion
or selection method either.

Thanks.

> 
> --
> Sincerely yours,
> Mike.