RE: [PATCH v6 1/2] mm/memory_hotplug: optimize zone contiguous check when changing pfn range

> -----Original Message-----
> From: David Hildenbrand (Arm) <[email protected]>
> Sent: Tuesday, August 11, 2026 8:23 PM
> To: Liu, Yuan1 <[email protected]>; Oscar Salvador <[email protected]>;
> Mike Rapoport <[email protected]>; Wei Yang <[email protected]>
> Cc: [email protected]; Zou, Nanhai <[email protected]>; Deng, Pan
> <[email protected]>; Li, Tianyou <[email protected]>; Chen Zhang
> <[email protected]>; Zeng, Jason <[email protected]>; linux-
> [email protected]
> Subject: Re: [PATCH v6 1/2] mm/memory_hotplug: optimize zone contiguous
> check when changing pfn range
> 
> On 8/10/26 16:04, David Hildenbrand (Arm) wrote:
> >>
> >> Hi David
> >>
> >> My understanding is that init_unavailable_range() initializes all
> >> PFNs that satisfy pfn_valid(), but not all of them satisfy
> >> pfn_to_online_page(), since some PFNs belong to subsections that are
> >> not online.
> >
> > Thanks for reminding me. I think the right direction is to finally clean
> up the
> > pfn_valid() handling.
> >
> >>
> >> You previously mentioned:
> >>
> >>   pfn_valid() says early sections always have a full memmap, so even
> invalid
> >>   subsections have a memmap. pfn_to_online_page() says an invalid
> subsection
> >>   cannot be online and its content must be stale. for_each_valid_pfn()
> follows
> >>   pfn_valid() semantics, and we use it to initialize memmap that is not
> going
> >>   to be online and account it as pages_with_online_memmap, which is
> wrong.
> >>
> >>   The cleanest approach is to avoid allocating memmap for subsections,
> which
> >>   also removes the special early-section handling from pfn_valid() and
> >>   for_each_valid_pfn().
> >>
> >> I also share the concerns raised by Sashiko in the analysis below [1]:
> >>
> >>   Scanners like isolate_migratepages_block() will then blindly iterate
> through
> >>   the pageblock and access the completely uninitialized struct pages of
> the hole,
> >>   leading to functional errors or kernel panics when reading these
> zero-filled
> >>   structures via macros like PageHuge() or page_zone().
> >>
> >> That's why we went with the current approach in v6 instead of your
> >> earlier suggestion. I'd really appreciate your guidance on which
> >> direction you think would be more appropriate.
> > Let me take a stab at just having pfn_valid() / for_each_valid_pfn()
> respecting
> > the subsection map.
> 
> ... and that turns complicated very quickly. The problem is that we have
> some users,
> in particular the buddy, that just assumes that MAX_PAGE_ORDER regions are
> fully
> accessible.
> 
> The fun begins once we have MAX_PAGE_ORDER span multiple subsections. So
> we'd actually
> want to initialize the memmap.
> 
> The pfn_valid() vs. pfn_to_online_page() inconsistency is really nasty :(
> 
> I mean, in init_unavailable_range() we could actually figure out fairly
> easily
> whether we are dealing with holes where pfn_to_online_page() would
> succeed.
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index e9c4204b73adb..54e71e17f2c3c 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -843,11 +843,13 @@ static void __init init_unavailable_range(unsigned
> long spfn,
>                                           int zone, int node)
>  {
>         unsigned long pfn;
> -       u64 pgcnt = 0;
> +       u64 pgcnt = 0, online_pgcnt = 0;
> 
>         for_each_valid_pfn(pfn, spfn, epfn) {
>                 __init_single_page(pfn_to_page(pfn), pfn, zone, node);
>                 __SetPageReserved(pfn_to_page(pfn));
> +               if (pfn_to_online_page(pfn))
> +                       online_pgcnt++;
>                 pgcnt++;
>         }
> 
> If it's a problem performance-wise, we can always try optimizing by
> skipping
> checks within the same (sub)section.
 
Hi David

What about the following approach to skipping the check within the same
(sub)section?

@@ -827,11 +827,21 @@ static void __init init_unavailable_range(unsigned long spfn,
                                          int zone, int node)
{
        unsigned long pfn;
-       u64 pgcnt = 0;
+       u64 pgcnt = 0, online_pgcnt = 0;
+       unsigned long last_subsection = -1;
+       bool is_online = false;

        for_each_valid_pfn(pfn, spfn, epfn) {
+               unsigned long subsection = pfn & PAGE_SUBSECTION_MASK;
+
                __init_single_page(pfn_to_page(pfn), pfn, zone, node);
                __SetPageReserved(pfn_to_page(pfn));
+               if (subsection != last_subsection) {
+                       is_online = !!pfn_to_online_page(pfn);
+                       last_subsection = subsection;
+               }
+               if (is_online)
+                       online_pgcnt++;
                pgcnt++;

If this looks good, let me prepare and send out the next version series.

Best Regards,
Liu, Yuan

> --
> Cheers,
> 
> David