[PATCH 0/4] mm: Don't set and reset page count in MEMINIT_EARLY

[PATCH 0/4] mm: Don't set and reset page count in MEMINIT_EARLY

[Yajun Deng]

__init_single_page would set page count and __free_pages_core would
reset it. A lot of pages don't need to do this when in MEMINIT_EARLY
context. It's unnecessary and time-consuming.

The first two patches are in preparation for the next two, they didn't
change anything.
The third patch only set page count for the reserved region, not all
of the region.
The fourth patch removes the set paget count in deferred_init_pages.

Yajun Deng (4):
  mm: pass set_count and set_reserved to __init_single_page
  mm: Introduce MEMINIT_LATE context
  mm: Set page count and mark page reserved in reserve_bootmem_region
  mm: don't set page count in deferred_init_pages

 include/linux/mmzone.h |  1 +
 mm/hugetlb.c           |  2 +-
 mm/internal.h          | 10 +++++---
 mm/kmsan/init.c        |  2 +-
 mm/memblock.c          |  4 +--
 mm/memory_hotplug.c    |  2 +-
 mm/mm_init.c           | 57 +++++++++++++++++++++++++-----------------
 mm/page_alloc.c        | 22 +++++++++-------
 8 files changed, 59 insertions(+), 41 deletions(-)

-- 
2.25.1

[PATCH 1/4] mm: pass set_count and set_reserved to __init_single_page

[Yajun Deng]

When we init a single page, we need to mark this page reserved if it
does. And somes page may not need to set page count, such as compound
pages.

Pass set_count and set_reserved to __init_single_page, let the caller
decide if it needs to set page count or mark page reserved.

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 mm/hugetlb.c  |  2 +-
 mm/internal.h |  3 ++-
 mm/mm_init.c  | 30 ++++++++++++++++--------------
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index e2123d1bb4a2..4f91e47430ce 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3196,7 +3196,7 @@ static void __init hugetlb_folio_init_tail_vmemmap(struct folio *folio,
 	for (pfn = head_pfn + start_page_number; pfn < end_pfn; pfn++) {
 		struct page *page = pfn_to_page(pfn);
 
-		__init_single_page(page, pfn, zone, nid);
+		__init_single_page(page, pfn, zone, nid, true, false);
 		prep_compound_tail((struct page *)folio, pfn - head_pfn);
 		ret = page_ref_freeze(page, 1);
 		VM_BUG_ON(!ret);
diff --git a/mm/internal.h b/mm/internal.h
index 7a961d12b088..8bded7f98493 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1210,7 +1210,8 @@ struct vma_prepare {
 };
 
 void __meminit __init_single_page(struct page *page, unsigned long pfn,
-				unsigned long zone, int nid);
+				  unsigned long zone, int nid, bool set_count,
+				  bool set_reserved);
 
 /* shrinker related functions */
 unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg,
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 06a72c223bce..c40042098a82 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -557,11 +557,13 @@ static void __init find_zone_movable_pfns_for_nodes(void)
 }
 
 void __meminit __init_single_page(struct page *page, unsigned long pfn,
-				unsigned long zone, int nid)
+				  unsigned long zone, int nid, bool set_count,
+				  bool set_reserved)
 {
 	mm_zero_struct_page(page);
 	set_page_links(page, zone, nid, pfn);
-	init_page_count(page);
+	if (set_count)
+		init_page_count(page);
 	page_mapcount_reset(page);
 	page_cpupid_reset_last(page);
 	page_kasan_tag_reset(page);
@@ -572,6 +574,8 @@ void __meminit __init_single_page(struct page *page, unsigned long pfn,
 	if (!is_highmem_idx(zone))
 		set_page_address(page, __va(pfn << PAGE_SHIFT));
 #endif
+	if (set_reserved)
+		__SetPageReserved(page);
 }
 
 #ifdef CONFIG_NUMA
@@ -714,7 +718,7 @@ static void __meminit init_reserved_page(unsigned long pfn, int nid)
 		if (zone_spans_pfn(zone, pfn))
 			break;
 	}
-	__init_single_page(pfn_to_page(pfn), pfn, zid, nid);
+	__init_single_page(pfn_to_page(pfn), pfn, zid, nid, true, false);
 }
 #else
 static inline void pgdat_set_deferred_range(pg_data_t *pgdat) {}
@@ -821,8 +825,8 @@ static void __init init_unavailable_range(unsigned long spfn,
 			pfn = pageblock_end_pfn(pfn) - 1;
 			continue;
 		}
-		__init_single_page(pfn_to_page(pfn), pfn, zone, node);
-		__SetPageReserved(pfn_to_page(pfn));
+		__init_single_page(pfn_to_page(pfn), pfn, zone, node,
+				   true, true);
 		pgcnt++;
 	}
 
@@ -884,7 +888,7 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
 		}
 
 		page = pfn_to_page(pfn);
-		__init_single_page(page, pfn, zone, nid);
+		__init_single_page(page, pfn, zone, nid, true, false);
 		if (context == MEMINIT_HOTPLUG)
 			__SetPageReserved(page);
 
@@ -965,11 +969,9 @@ static void __init memmap_init(void)
 #ifdef CONFIG_ZONE_DEVICE
 static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
 					  unsigned long zone_idx, int nid,
-					  struct dev_pagemap *pgmap)
+					  struct dev_pagemap *pgmap,
+					  bool set_count)
 {
-
-	__init_single_page(page, pfn, zone_idx, nid);
-
 	/*
 	 * Mark page reserved as it will need to wait for onlining
 	 * phase for it to be fully associated with a zone.
@@ -977,7 +979,7 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
 	 * We can use the non-atomic __set_bit operation for setting
 	 * the flag as we are still initializing the pages.
 	 */
-	__SetPageReserved(page);
+	__init_single_page(page, pfn, zone_idx, nid, set_count, true);
 
 	/*
 	 * ZONE_DEVICE pages union ->lru with a ->pgmap back pointer
@@ -1041,7 +1043,7 @@ static void __ref memmap_init_compound(struct page *head,
 	for (pfn = head_pfn + 1; pfn < end_pfn; pfn++) {
 		struct page *page = pfn_to_page(pfn);
 
-		__init_zone_device_page(page, pfn, zone_idx, nid, pgmap);
+		__init_zone_device_page(page, pfn, zone_idx, nid, pgmap, false);
 		prep_compound_tail(head, pfn - head_pfn);
 		set_page_count(page, 0);
 
@@ -1084,7 +1086,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 	for (pfn = start_pfn; pfn < end_pfn; pfn += pfns_per_compound) {
 		struct page *page = pfn_to_page(pfn);
 
-		__init_zone_device_page(page, pfn, zone_idx, nid, pgmap);
+		__init_zone_device_page(page, pfn, zone_idx, nid, pgmap, true);
 
 		if (pfns_per_compound == 1)
 			continue;
@@ -2058,7 +2060,7 @@ static unsigned long  __init deferred_init_pages(struct zone *zone,
 		} else {
 			page++;
 		}
-		__init_single_page(page, pfn, zid, nid);
+		__init_single_page(page, pfn, zid, nid, true, false);
 		nr_pages++;
 	}
 	return (nr_pages);
-- 
2.25.1

[PATCH 2/4] mm: Introduce MEMINIT_LATE context

[Yajun Deng]

__free_pages_core() will always reset pages count and clear reserved flag.
It will consume a lot of time if there are a lot of pages.

Introduce MEMINIT_LATE context, if the context is MEMINIT_EARLY, we
don't need reset pages count and clear reserved flag.

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 include/linux/mmzone.h |  1 +
 mm/internal.h          |  7 ++++---
 mm/kmsan/init.c        |  2 +-
 mm/memblock.c          |  4 ++--
 mm/memory_hotplug.c    |  2 +-
 mm/mm_init.c           | 11 ++++++-----
 mm/page_alloc.c        | 14 ++++++++------
 7 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 1e9cf3aa1097..253e792d409f 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1442,6 +1442,7 @@ bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
  */
 enum meminit_context {
 	MEMINIT_EARLY,
+	MEMINIT_LATE,
 	MEMINIT_HOTPLUG,
 };
 
diff --git a/mm/internal.h b/mm/internal.h
index 8bded7f98493..31737196257c 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -394,9 +394,10 @@ static inline void clear_zone_contiguous(struct zone *zone)
 extern int __isolate_free_page(struct page *page, unsigned int order);
 extern void __putback_isolated_page(struct page *page, unsigned int order,
 				    int mt);
-extern void memblock_free_pages(struct page *page, unsigned long pfn,
-					unsigned int order);
-extern void __free_pages_core(struct page *page, unsigned int order);
+extern void memblock_free_pages(unsigned long pfn, unsigned int order,
+				enum meminit_context context);
+extern void __free_pages_core(struct page *page, unsigned int order,
+			      enum meminit_context context);
 
 /*
  * This will have no effect, other than possibly generating a warning, if the
diff --git a/mm/kmsan/init.c b/mm/kmsan/init.c
index ffedf4dbc49d..b7ed98b854a6 100644
--- a/mm/kmsan/init.c
+++ b/mm/kmsan/init.c
@@ -172,7 +172,7 @@ static void do_collection(void)
 		shadow = smallstack_pop(&collect);
 		origin = smallstack_pop(&collect);
 		kmsan_setup_meta(page, shadow, origin, collect.order);
-		__free_pages_core(page, collect.order);
+		__free_pages_core(page, collect.order, MEMINIT_LATE);
 	}
 }
 
diff --git a/mm/memblock.c b/mm/memblock.c
index 5a88d6d24d79..a32364366bb2 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1685,7 +1685,7 @@ void __init memblock_free_late(phys_addr_t base, phys_addr_t size)
 	end = PFN_DOWN(base + size);
 
 	for (; cursor < end; cursor++) {
-		memblock_free_pages(pfn_to_page(cursor), cursor, 0);
+		memblock_free_pages(cursor, 0, MEMINIT_LATE);
 		totalram_pages_inc();
 	}
 }
@@ -2089,7 +2089,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end)
 		while (start + (1UL << order) > end)
 			order--;
 
-		memblock_free_pages(pfn_to_page(start), start, order);
+		memblock_free_pages(start, order, MEMINIT_LATE);
 
 		start += (1UL << order);
 	}
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 3b301c4023ff..d38548265f26 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -634,7 +634,7 @@ void generic_online_page(struct page *page, unsigned int order)
 	 * case in page freeing fast path.
 	 */
 	debug_pagealloc_map_pages(page, 1 << order);
-	__free_pages_core(page, order);
+	__free_pages_core(page, order, MEMINIT_HOTPLUG);
 	totalram_pages_add(1UL << order);
 }
 EXPORT_SYMBOL_GPL(generic_online_page);
diff --git a/mm/mm_init.c b/mm/mm_init.c
index c40042098a82..0a4437aae30d 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1976,7 +1976,7 @@ static void __init deferred_free_range(unsigned long pfn,
 	if (nr_pages == MAX_ORDER_NR_PAGES && IS_MAX_ORDER_ALIGNED(pfn)) {
 		for (i = 0; i < nr_pages; i += pageblock_nr_pages)
 			set_pageblock_migratetype(page + i, MIGRATE_MOVABLE);
-		__free_pages_core(page, MAX_ORDER);
+		__free_pages_core(page, MAX_ORDER, MEMINIT_LATE);
 		return;
 	}
 
@@ -1986,7 +1986,7 @@ static void __init deferred_free_range(unsigned long pfn,
 	for (i = 0; i < nr_pages; i++, page++, pfn++) {
 		if (pageblock_aligned(pfn))
 			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
-		__free_pages_core(page, 0);
+		__free_pages_core(page, 0, MEMINIT_LATE);
 	}
 }
 
@@ -2568,9 +2568,10 @@ void __init set_dma_reserve(unsigned long new_dma_reserve)
 	dma_reserve = new_dma_reserve;
 }
 
-void __init memblock_free_pages(struct page *page, unsigned long pfn,
-							unsigned int order)
+void __init memblock_free_pages(unsigned long pfn, unsigned int order,
+				enum meminit_context context)
 {
+	struct page *page = pfn_to_page(pfn);
 
 	if (IS_ENABLED(CONFIG_DEFERRED_STRUCT_PAGE_INIT)) {
 		int nid = early_pfn_to_nid(pfn);
@@ -2583,7 +2584,7 @@ void __init memblock_free_pages(struct page *page, unsigned long pfn,
 		/* KMSAN will take care of these pages. */
 		return;
 	}
-	__free_pages_core(page, order);
+	__free_pages_core(page, order, context);
 }
 
 DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_ALLOC_DEFAULT_ON, init_on_alloc);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 06be8821d833..6c4f4531bee0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1278,7 +1278,7 @@ static void __free_pages_ok(struct page *page, unsigned int order,
 	__count_vm_events(PGFREE, 1 << order);
 }
 
-void __free_pages_core(struct page *page, unsigned int order)
+void __free_pages_core(struct page *page, unsigned int order, enum meminit_context context)
 {
 	unsigned int nr_pages = 1 << order;
 	struct page *p = page;
@@ -1289,14 +1289,16 @@ void __free_pages_core(struct page *page, unsigned int order)
 	 * of all pages to 1 ("allocated"/"not free"). We have to set the
 	 * refcount of all involved pages to 0.
 	 */
-	prefetchw(p);
-	for (loop = 0; loop < (nr_pages - 1); loop++, p++) {
-		prefetchw(p + 1);
+	if (context != MEMINIT_EARLY) {
+		prefetchw(p);
+		for (loop = 0; loop < (nr_pages - 1); loop++, p++) {
+			prefetchw(p + 1);
+			__ClearPageReserved(p);
+			set_page_count(p, 0);
+		}
 		__ClearPageReserved(p);
 		set_page_count(p, 0);
 	}
-	__ClearPageReserved(p);
-	set_page_count(p, 0);
 
 	atomic_long_add(nr_pages, &page_zone(page)->managed_pages);
 
-- 
2.25.1

[PATCH 3/4] mm: Set page count and mark page reserved in reserve_bootmem_region

[Yajun Deng]

memmap_init_range() would set page count of all pages, but the free
pages count would be reset in __free_pages_core(). These two are
opposite operations. It's unnecessary and time-consuming when it's
in MEMINIT_EARLY context.

Set page count and mark page reserved in reserve_bootmem_region when
in MEMINIT_EARLY context, and change the context from MEMINIT_LATE
to MEMINIT_EARLY in __free_pages_memory.

At the same time, the init list head in reserve_bootmem_region isn't
need. As it already done in __init_single_page.

The following data was tested on an x86 machine with 190GB of RAM.

before:
free_low_memory_core_early()	342ms

after:
free_low_memory_core_early()	286ms

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 mm/memblock.c   |  2 +-
 mm/mm_init.c    | 20 ++++++++++++++------
 mm/page_alloc.c |  8 +++++---
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index a32364366bb2..9276f1819982 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -2089,7 +2089,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end)
 		while (start + (1UL << order) > end)
 			order--;
 
-		memblock_free_pages(start, order, MEMINIT_LATE);
+		memblock_free_pages(start, order, MEMINIT_EARLY);
 
 		start += (1UL << order);
 	}
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 0a4437aae30d..1cc310f706a9 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -718,7 +718,7 @@ static void __meminit init_reserved_page(unsigned long pfn, int nid)
 		if (zone_spans_pfn(zone, pfn))
 			break;
 	}
-	__init_single_page(pfn_to_page(pfn), pfn, zid, nid, true, false);
+	__init_single_page(pfn_to_page(pfn), pfn, zid, nid, false, false);
 }
 #else
 static inline void pgdat_set_deferred_range(pg_data_t *pgdat) {}
@@ -756,8 +756,8 @@ void __meminit reserve_bootmem_region(phys_addr_t start,
 
 			init_reserved_page(start_pfn, nid);
 
-			/* Avoid false-positive PageTail() */
-			INIT_LIST_HEAD(&page->lru);
+			/* Set page count for the reserve region */
+			init_page_count(page);
 
 			/*
 			 * no need for atomic set_bit because the struct
@@ -888,9 +888,17 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
 		}
 
 		page = pfn_to_page(pfn);
-		__init_single_page(page, pfn, zone, nid, true, false);
-		if (context == MEMINIT_HOTPLUG)
-			__SetPageReserved(page);
+
+		/* If the context is MEMINIT_EARLY, we will set page count and
+		 * mark page reserved in reserve_bootmem_region, the free region
+		 * wouldn't have page count and reserved flag and we don't
+		 * need to reset pages count and clear reserved flag in
+		 * __free_pages_core.
+		 */
+		if (context == MEMINIT_EARLY)
+			__init_single_page(page, pfn, zone, nid, false, false);
+		else
+			__init_single_page(page, pfn, zone, nid, true, true);
 
 		/*
 		 * Usually, we want to mark the pageblock MIGRATE_MOVABLE,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6c4f4531bee0..6ac58c5f3b00 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1285,9 +1285,11 @@ void __free_pages_core(struct page *page, unsigned int order, enum meminit_conte
 	unsigned int loop;
 
 	/*
-	 * When initializing the memmap, __init_single_page() sets the refcount
-	 * of all pages to 1 ("allocated"/"not free"). We have to set the
-	 * refcount of all involved pages to 0.
+	 * When initializing the memmap, memmap_init_range sets the refcount
+	 * of all pages to 1 ("allocated"/"not free") in hotplug context. We
+	 * have to set the refcount of all involved pages to 0. Otherwise,
+	 * we don't do it, as reserve_bootmem_region only set the refcount on
+	 * reserve region ("allocated") in early context.
 	 */
 	if (context != MEMINIT_EARLY) {
 		prefetchw(p);
-- 
2.25.1

[PATCH 4/4] mm: don't set page count in deferred_init_pages

[Yajun Deng]

The operations of page count in deferred_init_pages and deferred_free_range
is the opposite operation. It's unnecessary and time-consuming.

Don't set page count in deferred_init_pages, as it'll be reset later.

The following data was tested on an x86 machine with 190GB of RAM.

before:
node 0 deferred pages initialised in 78ms

after:
node 0 deferred pages initialised in 72ms

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 mm/mm_init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 1cc310f706a9..fe78f6916c66 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1984,7 +1984,7 @@ static void __init deferred_free_range(unsigned long pfn,
 	if (nr_pages == MAX_ORDER_NR_PAGES && IS_MAX_ORDER_ALIGNED(pfn)) {
 		for (i = 0; i < nr_pages; i += pageblock_nr_pages)
 			set_pageblock_migratetype(page + i, MIGRATE_MOVABLE);
-		__free_pages_core(page, MAX_ORDER, MEMINIT_LATE);
+		__free_pages_core(page, MAX_ORDER, MEMINIT_EARLY);
 		return;
 	}
 
@@ -1994,7 +1994,7 @@ static void __init deferred_free_range(unsigned long pfn,
 	for (i = 0; i < nr_pages; i++, page++, pfn++) {
 		if (pageblock_aligned(pfn))
 			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
-		__free_pages_core(page, 0, MEMINIT_LATE);
+		__free_pages_core(page, 0, MEMINIT_EARLY);
 	}
 }
 
@@ -2068,7 +2068,7 @@ static unsigned long  __init deferred_init_pages(struct zone *zone,
 		} else {
 			page++;
 		}
-		__init_single_page(page, pfn, zid, nid, true, false);
+		__init_single_page(page, pfn, zid, nid, false, false);
 		nr_pages++;
 	}
 	return (nr_pages);
-- 
2.25.1

Re: [PATCH 1/4] mm: pass set_count and set_reserved to __init_single_page

[Matthew Wilcox]

On Fri, Sep 22, 2023 at 03:09:20PM +0800, Yajun Deng wrote:
> -		__init_single_page(page, pfn, zone, nid);
> +		__init_single_page(page, pfn, zone, nid, true, false);

So Linus has just had a big rant about not doing bool flags to
functions.  And in particular _multiple_ bool flags to functions.

ie this should be:

#define INIT_PAGE_COUNT		(1 << 0)
#define INIT_PAGE_RESERVED	(1 << 1)

		__init_single_page(page, pfn, zone, nid, INIT_PAGE_COUNT);

or something similar.

I have no judgement on the merits of this patch so far.  Do you have
performance numbers for each of these patches?  Some of them seem quite
unlikely to actually help, at least on a machine which is constrained
by cacheline fetches.

Re: [PATCH 1/4] mm: pass set_count and set_reserved to __init_single_page

[David Hildenbrand]

On 22.09.23 09:47, Matthew Wilcox wrote:
> On Fri, Sep 22, 2023 at 03:09:20PM +0800, Yajun Deng wrote:
>> -		__init_single_page(page, pfn, zone, nid);
>> +		__init_single_page(page, pfn, zone, nid, true, false);
> 
> So Linus has just had a big rant about not doing bool flags to
> functions.  And in particular _multiple_ bool flags to functions.
> 
> ie this should be:
> 
> #define INIT_PAGE_COUNT		(1 << 0)
> #define INIT_PAGE_RESERVED	(1 << 1)
> 
> 		__init_single_page(page, pfn, zone, nid, INIT_PAGE_COUNT);
> 
> or something similar.
> 
> I have no judgement on the merits of this patch so far.  Do you have
> performance numbers for each of these patches?  Some of them seem quite
> unlikely to actually help, at least on a machine which is constrained
> by cacheline fetches.

The last patch contains

before:
node 0 deferred pages initialised in 78ms

after:
node 0 deferred pages initialised in 72ms

Not earth-shattering :D Maybe with much bigger machines relevant?

-- 
Cheers,

David / dhildenb

Re: [PATCH 1/4] mm: pass set_count and set_reserved to __init_single_page

[Mike Rapoport]

On Fri, Sep 22, 2023 at 09:48:59AM +0200, David Hildenbrand wrote:
> On 22.09.23 09:47, Matthew Wilcox wrote:
> > On Fri, Sep 22, 2023 at 03:09:20PM +0800, Yajun Deng wrote:
> > > -		__init_single_page(page, pfn, zone, nid);
> > > +		__init_single_page(page, pfn, zone, nid, true, false);
> > 
> > So Linus has just had a big rant about not doing bool flags to
> > functions.  And in particular _multiple_ bool flags to functions.
> > 
> > ie this should be:
> > 
> > #define INIT_PAGE_COUNT		(1 << 0)
> > #define INIT_PAGE_RESERVED	(1 << 1)
> > 
> > 		__init_single_page(page, pfn, zone, nid, INIT_PAGE_COUNT);
> > 
> > or something similar.
> > 
> > I have no judgement on the merits of this patch so far.  Do you have
> > performance numbers for each of these patches?  Some of them seem quite
> > unlikely to actually help, at least on a machine which is constrained
> > by cacheline fetches.
> 
> The last patch contains
> 
> before:
> node 0 deferred pages initialised in 78ms
> 
> after:
> node 0 deferred pages initialised in 72ms
> 
> Not earth-shattering :D Maybe with much bigger machines relevant?

Patch 3 contains

The following data was tested on an x86 machine with 190GB of RAM.

before:
free_low_memory_core_early()    342ms

after:
free_low_memory_core_early()    286ms

Which is more impressive, but still I'm not convinced that it's worth the
added complexity and potential subtle bugs.

> -- 
> Cheers,
> 
> David / dhildenb
> 

-- 
Sincerely yours,
Mike.