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

[Liu, Yuan1]

> -----Original Message-----
> From: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> Sent: Wednesday, August 19, 2026 11:54 PM
> To: Liu, Yuan1 <yuan1.liu@xxxxxxxxx>; Oscar Salvador <osalvador@xxxxxxx>;
> Mike Rapoport <rppt@xxxxxxxxxx>; Wei Yang <richard.weiyang@xxxxxxxxx>
> Cc: linux-mm@xxxxxxxxx; Zou, Nanhai <nanhai.zou@xxxxxxxxx>; Chen Zhang
> <zhangchen.kidd@xxxxxx>; Zeng, Jason <jason.zeng@xxxxxxxxx>; Chen, Yu C
> <yu.c.chen@xxxxxxxxx>; Deng, Pan <pan.deng@xxxxxxxxx>; Li, Tianyou
> <tianyou.li@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v7 2/2] mm/memory_hotplug: optimize zone contiguous
> check when changing pfn range
> 
> 
> >  /*
> >   * Only struct pages that correspond to ranges defined by
> memblock.memory
> >   * are zeroed and initialized by going through __init_single_page()
> during
> > @@ -822,22 +844,27 @@ void __meminit init_deferred_page(unsigned long
> pfn, int nid)
> >   *   zone/node above the hole except for the trailing pages in the last
> >   *   section that will be appended to the zone/node below.
> >   */
> > -static void __init init_unavailable_range(unsigned long spfn,
> > -					  unsigned long epfn,
> > -					  int zone, int node)
> > +static unsigned long __init init_unavailable_range(unsigned long spfn,
> > +						   unsigned long epfn,
> > +						   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) {
> >  		__init_single_page(pfn_to_page(pfn), pfn, zone, node);
> >  		__SetPageReserved(pfn_to_page(pfn));
> > +		if (unavailable_pfn_is_online(pfn, &last_subsection,
> &is_online))
> > +			online_pgcnt++;
> 
> Can we avoid these helpers?
> 
> 		const unsigned long subsection = pfn & PAGE_SUBSECTION_MASK;
> 
> 		/* We can have section-sized online holes with VMEMMAP. */
> 		if (IS_ENMABLED(CONFIG_SPARSEMEM_VMEMMAP) &&
> 		    subsection != last_subsection) {
> 			is_online = pfn_to_online_page(pfn);
> 			last_subsection = subsection;
> 		}
> 		if (is_online)
> 			online_pgcnt++;

I think there may be an issue here: PAGE_SUBSECTION_MASK is not
defined when CONFIG_FLATMEM is enabled, which would result in a
build failure.

> An alternative is an inner loop that just walks in SUBSECTION chunks until
> epfn.
> That would probably be even cleaner and faster.
> 
> I remember !vmemmap always only has early sections when they are actually
> online. We could extent the comment to clarify that.

Hi David

What about the following approach? It removes the helper and changes
the per-PFN online check to a per-chunk check.

+       u64 pgcnt = 0, online_pgcnt = 0;
+#ifdef CONFIG_SPARSEMEM_VMEMMAP
+       const unsigned long chunk = PAGES_PER_SUBSECTION;
+#else
+       const unsigned long chunk = epfn - spfn;
+#endif

-       for_each_valid_pfn(pfn, spfn, epfn) {
-               __init_single_page(pfn_to_page(pfn), pfn, zone, node);
-               __SetPageReserved(pfn_to_page(pfn));
-               pgcnt++;
+       /*
+        * With VMEMMAP, subsection-sized holes can exist, and PFNs within
+        * these holes can fail pfn_to_online_page(). Without VMEMMAP, we
+        * always only have early sections when they are actually online.
+        */
+       for (pfn = spfn; pfn < epfn; pfn += chunk) {
+               const unsigned long chunk_epfn = min(pfn + chunk, epfn);
+               const bool is_online = !IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP) ||
+                                      pfn_to_online_page(pfn);
+               unsigned long p;
+
+               for_each_valid_pfn(p, pfn, chunk_epfn) {
+                       __init_single_page(pfn_to_page(p), p, zone, node);
+                       __SetPageReserved(pfn_to_page(p));
+                       if (is_online)
+                               online_pgcnt++;
+                       pgcnt++;
+               }
 
> (I have patches to clean that init code up)
> 
> 
> --
> Cheers,
> 
> David