2023-09-26 05:00:38

by Yajun Deng

[permalink] [raw]
Subject: [PATCH v3 0/2] mm: Don't set and reset page count in MEMINIT_EARLY

__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 1st patch is pass page count and reserved to __init_single_page.
It's in preparation for the 2nd patch, it didn't change anything.

The 2nd patch only set page count for the reserved region, not all
of the region.

Yajun Deng (2):
mm: pass page count and reserved to __init_single_page
mm: Init page count in reserve_bootmem_region when MEMINIT_EARLY

mm/hugetlb.c | 2 +-
mm/internal.h | 8 +++++++-
mm/mm_init.c | 45 ++++++++++++++++++++++++++++-----------------
mm/page_alloc.c | 20 ++++++++++++--------
4 files changed, 48 insertions(+), 27 deletions(-)

--
2.25.1


2023-09-26 08:44:21

by Yajun Deng

[permalink] [raw]
Subject: [PATCH v3 1/2] mm: pass page count and reserved to __init_single_page

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

Introduce enum init_page_flags, the caller init page count and mark page
reserved by passing INIT_PAGE_COUNT and INIT_PAGE_RESERVED.

Signed-off-by: Yajun Deng <[email protected]>
---
v3: Introduce enum init_page_flags.
v2: Introduce INIT_PAGE_COUNT and INIT_PAGE_RESERVED.
v1: https://lore.kernel.org/all/[email protected]/
---
mm/hugetlb.c | 2 +-
mm/internal.h | 8 +++++++-
mm/mm_init.c | 31 +++++++++++++++++--------------
3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index de220e3ff8be..cb282eadc729 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, INIT_PAGE_COUNT);
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 d7916f1e9e98..449891ad7fdb 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1209,8 +1209,14 @@ struct vma_prepare {
struct vm_area_struct *remove2;
};

+enum init_page_flags {
+ INIT_PAGE_COUNT = (1 << 0),
+ INIT_PAGE_RESERVED = (1 << 1),
+};
+
void __meminit __init_single_page(struct page *page, unsigned long pfn,
- unsigned long zone, int nid);
+ unsigned long zone, int nid,
+ enum init_page_flags flags);

/* 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..07fe7e489769 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -557,11 +557,11 @@ 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,
+ enum init_page_flags flags)
{
mm_zero_struct_page(page);
set_page_links(page, zone, nid, pfn);
- init_page_count(page);
page_mapcount_reset(page);
page_cpupid_reset_last(page);
page_kasan_tag_reset(page);
@@ -572,6 +572,10 @@ 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 (flags & INIT_PAGE_COUNT)
+ init_page_count(page);
+ if (flags & INIT_PAGE_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, INIT_PAGE_COUNT);
}
#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,
+ INIT_PAGE_COUNT | INIT_PAGE_RESERVED);
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, INIT_PAGE_COUNT);
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,
+ unsigned int flags)
{
-
- __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, flags | INIT_PAGE_RESERVED);

/*
* 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, 0);
prep_compound_tail(head, pfn - head_pfn);
set_page_count(page, 0);

@@ -1084,7 +1086,8 @@ 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,
+ INIT_PAGE_COUNT);

if (pfns_per_compound == 1)
continue;
@@ -2058,7 +2061,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, INIT_PAGE_COUNT);
nr_pages++;
}
return (nr_pages);
--
2.25.1

2023-09-26 09:19:43

by Yajun Deng

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] mm: pass page count and reserved to __init_single_page


On 2023/9/26 15:44, David Hildenbrand wrote:
> On 26.09.23 04:33, Yajun Deng wrote:
>> When we init a single page, we need to mark this page reserved if it
>> does.
>
> I failed to parse the last part of this sentence.
>
>> And some pages may not need to set page count, such as compound
>> pages.
>
> Usually, the refcount of all tail pages *must* be zero. Otherwise,
> get_page_unless_zero() would work on tail pages.
>
> Can you elaborate why it should be okay here?
>
>
It means the following code in memmap_init_compound().

-               __init_zone_device_page(page, pfn, zone_idx, nid, pgmap);
+               __init_zone_device_page(page, pfn, zone_idx, nid, pgmap, 0);
                prep_compound_tail(head, pfn - head_pfn);
                set_page_count(page, 0);

As we can see, it will reset the page count by 'set_page_count(page, 0)'.

Therefore, we don't need to set page count in __init_zone_device_page().
I wasn't clear enough in the commit.

Maybe we can remove the 'set_page_count(page, 0)',  But I didn't do
that, just to be safe.

2023-09-26 14:02:23

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] mm: pass page count and reserved to __init_single_page

On 26.09.23 04:33, Yajun Deng wrote:
> When we init a single page, we need to mark this page reserved if it
> does.

I failed to parse the last part of this sentence.

> And some pages may not need to set page count, such as compound
> pages.

Usually, the refcount of all tail pages *must* be zero. Otherwise,
get_page_unless_zero() would work on tail pages.

Can you elaborate why it should be okay here?


--
Cheers,

David / dhildenb

2023-09-28 05:42:19

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] mm: pass page count and reserved to __init_single_page

On Tue, Sep 26, 2023 at 10:33:40AM +0800, Yajun Deng wrote:
> When we init a single page, we need to mark this page reserved if it
> does. And some pages may not need to set page count, such as compound
> pages.
>
> Introduce enum init_page_flags, the caller init page count and mark page
> reserved by passing INIT_PAGE_COUNT and INIT_PAGE_RESERVED.
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
> v3: Introduce enum init_page_flags.
> v2: Introduce INIT_PAGE_COUNT and INIT_PAGE_RESERVED.
> v1: https://lore.kernel.org/all/[email protected]/
> ---
> mm/hugetlb.c | 2 +-
> mm/internal.h | 8 +++++++-
> mm/mm_init.c | 31 +++++++++++++++++--------------
> 3 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 06a72c223bce..07fe7e489769 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -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, 0);

I think the first patch should not contain any functional changes, but only
add the flags parameter to __init_single_page().

> prep_compound_tail(head, pfn - head_pfn);
> set_page_count(page, 0);
>

--
Sincerely yours,
Mike.