[v3,0/2] mm: Don't set and reset page count in MEMINIT_EARLY

Message

Yajun Deng Sept. 26, 2023, 2:33 a.m. UTC

__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(-)

Comments

David Hildenbrand Sept. 26, 2023, 7:44 a.m. UTC | #1

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?

Yajun Deng Sept. 26, 2023, 7:57 a.m. UTC | #2

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.

Mike Rapoport Sept. 28, 2023, 5:30 a.m. UTC | #3

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 <[e-mail]>
> ---
> v3: Introduce enum init_page_flags.
> v2: Introduce INIT_PAGE_COUNT and INIT_PAGE_RESERVED.
> v1: https://lore.kernel.org/all/[e-mail]/
> ---
>  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);
>