linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Usama Arif <usama.arif@bytedance.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: Wed, 6 Sep 2023 14:59:27 -0700	[thread overview]
Message-ID: <20230906215927.GE3612@monkey> (raw)
In-Reply-To: <57c8dd7f-d1a0-37c4-1d3b-d6374e92ffa1@bytedance.com>

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.
-- 
Mike Kravetz


  reply	other threads:[~2023-09-06 22:00 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-06 11:26 [v4 0/4] " 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 [this message]
2023-09-07 10:14         ` Usama Arif
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=20230906215927.GE3612@monkey \
    --to=mike.kravetz@oracle.com \
    --cc=fam.zheng@bytedance.com \
    --cc=liangma@liangbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=muchun.song@linux.dev \
    --cc=punit.agrawal@bytedance.com \
    --cc=rppt@kernel.org \
    --cc=songmuchun@bytedance.com \
    --cc=usama.arif@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox