All of lore.kernel.org
 help / color / mirror / Atom feed
From: Usama Arif <usama.arif@bytedance.com>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: linux-mm@kvack.org, muchun.song@linux.dev, rppt@kernel.org,
	linux-kernel@vger.kernel.org, songmuchun@bytedance.com,
	fam.zheng@bytedance.com, liangma@liangbit.com,
	punit.agrawal@bytedance.com
Subject: Re: [External] Re: [v4 4/4] mm: hugetlb: Skip initialization of gigantic tail struct pages if freed by HVO
Date: Thu, 7 Sep 2023 11:14:38 +0100	[thread overview]
Message-ID: <92fc88ba-3e3a-2648-4232-1d3f9bed5bb6@bytedance.com> (raw)
In-Reply-To: <20230906215927.GE3612@monkey>



On 06/09/2023 22:59, Mike Kravetz wrote:
> On 09/06/23 22:27, Usama Arif wrote:
>>
>>
>> On 06/09/2023 19:10, Mike Kravetz wrote:
>>> On 09/06/23 12:26, Usama Arif wrote:
>>>> The new boot flow when it comes to initialization of gigantic pages
>>>> is as follows:
>>>> - At boot time, for a gigantic page during __alloc_bootmem_hugepage,
>>>> the region after the first struct page is marked as noinit.
>>>> - This results in only the first struct page to be
>>>> initialized in reserve_bootmem_region. As the tail struct pages are
>>>> not initialized at this point, there can be a significant saving
>>>> in boot time if HVO succeeds later on.
>>>> - Later on in the boot, the head page is prepped and the first
>>>> HUGETLB_VMEMMAP_RESERVE_SIZE / sizeof(struct page) - 1 tail struct pages
>>>> are initialized.
>>>> - HVO is attempted. If it is not successful, then the rest of the
>>>> tail struct pages are initialized. If it is successful, no more
>>>> tail struct pages need to be initialized saving significant boot time.
>>>
>>> Code looks reasonable.  Quick question.
>>>
>>> On systems where HVO is disabled, we will still go through this new boot
>>> flow and init hugetlb tail pages later in boot (gather_bootmem_prealloc).
>>> Correct?
>>> If yes, will there be a noticeable change in performance from the current
>>> flow with HVO disabled?  My concern would be allocating a large number of
>>> gigantic pages at boot (TB or more).
>>>
>>
>> Thanks for the review.
>>
>> The patch moves the initialization of struct pages backing hugepage from
>> reserve_bootmem_region to a bit later on in the boot to
>> gather_bootmem_prealloc. When HVO is disabled, there will be no difference
>> in time taken to boot with or without this patch series, as 262144 struct
>> pages per gigantic page (for x86) are still going to be initialized, just in
>> a different place.
> 
> I seem to recall that 'normal' deferred struct page initialization was
> done in parallel as the result of these series:
> https://lore.kernel.org/linux-mm/20171013173214.27300-1-pasha.tatashin@oracle.com/
> https://lore.kernel.org/linux-mm/20200527173608.2885243-1-daniel.m.jordan@oracle.com/#t
> and perhaps others.
> 
> My thought is that we lose that parallel initialization when it is being
> done as part of hugetlb fall back initialization.
> 
> Does that make sense?  Or am I missing something?  I do not have any proof
> that things will be slower.  That is just something I was thinking about.

The patches for deferring struct page initialization did not cover the 
struct pages for gigantic pages.

With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled, the function call taken 
during boot without these patches is:

[A1] mm_core_init-> mem_init-> memblock_free_all-> 
free_low_memory_core_early-> memmap_init_reserved_pages-> 
reserve_bootmem_region-> initialize *all* struct pages of a gigantic 
page serially (DEFERRED_STRUCT_PAGE_INIT is enabled).
The pfn of the struct pages > NODE_DATA(nid)->first_deferred_pfn which 
means this cannot be deferred.

then later on in the boot:

[A2] hugetlb_init-> gather_bootmem_prealloc-> 
prep_compound_gigantic_folio-> prepare *all* the struct pages to be part 
of a gigantic page (freezing page ref count, setting compound head, etc 
for all struct pages)

With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled, the function call taken 
during boot with these patches is:

[B1] mm_core_init->...reserve_bootmem_region-> initialize head struct 
page only.

then later on in the boot:

[B2] hugetlb_init-> gather_bootmem_prealloc-> [B21] initialize only 64 
tail struct pages if HVO passes. [B22] If HVO fails initialize all tail 
struct pages.


Each of A1, A2 and B22 are for loops going over 262144 struct pages per 
hugepage. So without these patches, the work done is 262144*2 (A1+A2) 
per hugepage during boot, even with CONFIG_DEFERRED_STRUCT_PAGE_INIT as 
its not deferred. With these patches, the work done is either 1 + 64 
(B1+B21) if HVO is enabled or 1 + 262144 (B1+B22) if HVO is disabled.

With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled, the times taken to boot 
till init process when allocating 500 1G hugeppages are:
- with these patches, HVO enabled: 1.32 seconds [B1 + B21]
- with patches, HVO disabled: 2.15 seconds [B1 + B22]
- without patches, HVO enabled: 3.90  seconds [A1 + A2 + HVO]
- without patches, HVO disabled: 3.58 seconds [A1 + A2]

  reply	other threads:[~2023-09-07 17:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-06 11:26 [v4 0/4] mm: hugetlb: Skip initialization of gigantic tail struct pages if freed by HVO Usama Arif
2023-09-06 11:26 ` [v4 1/4] mm: hugetlb_vmemmap: Use nid of the head page to reallocate it Usama Arif
2023-09-06 11:26 ` [v4 2/4] memblock: pass memblock_type to memblock_setclr_flag Usama Arif
2023-09-06 11:26 ` [v4 3/4] memblock: introduce MEMBLOCK_RSRV_NOINIT flag Usama Arif
2023-09-06 11:35   ` Muchun Song
2023-09-06 12:01   ` Mike Rapoport
2023-09-06 11:26 ` [v4 4/4] mm: hugetlb: Skip initialization of gigantic tail struct pages if freed by HVO Usama Arif
2023-09-06 18:10   ` Mike Kravetz
2023-09-06 21:27     ` [External] " Usama Arif
2023-09-06 21:59       ` Mike Kravetz
2023-09-07 10:14         ` Usama Arif [this message]
2023-09-07 18:24           ` Mike Kravetz
2023-09-07 18:37   ` Mike Kravetz
2023-09-08  2:39     ` Muchun Song
2023-09-08 18:29       ` Mike Kravetz
2023-09-08 20:48         ` [External] " Usama Arif
2023-09-22 14:42 ` [v4 0/4] " Pasha Tatashin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=92fc88ba-3e3a-2648-4232-1d3f9bed5bb6@bytedance.com \
    --to=usama.arif@bytedance.com \
    --cc=fam.zheng@bytedance.com \
    --cc=liangma@liangbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=muchun.song@linux.dev \
    --cc=punit.agrawal@bytedance.com \
    --cc=rppt@kernel.org \
    --cc=songmuchun@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.