The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: Mike Rapoport <rppt@kernel.org>
Cc: Muchun Song <songmuchun@bytedance.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oscar Salvador <osalvador@suse.de>,
	David Hildenbrand <david@kernel.org>,
	linux-mm@kvack.org, Vlastimil Babka <vbabka@kernel.org>,
	Lorenzo Stoakes <ljs@kernel.org>, Michal Hocko <mhocko@suse.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages
Date: Thu, 9 Jul 2026 21:23:47 +0800	[thread overview]
Message-ID: <CFAE1475-6104-483A-86AE-511FB0A1CA2B@linux.dev> (raw)
In-Reply-To: <ak-ctr06UDUlNCJZ@kernel.org>



> On Jul 9, 2026, at 21:05, Mike Rapoport <rppt@kernel.org> wrote:
> 
> On Thu, Jul 09, 2026 at 08:31:31PM +0800, Muchun Song wrote:
>>> On Jul 9, 2026, at 18:45, Mike Rapoport <rppt@kernel.org> wrote:
>>> On Thu, Jul 02, 2026 at 05:38:08PM +0800, Muchun Song wrote:
>>>> memmap_init_range() initializes every struct page in the target range.
>>>> For compound pages with vmemmap optimization, the tail struct pages are
>>>> backed by a shared vmemmap page.
>>>> 
>>>> Initializing those tail struct pages would overwrite the shared
>>>> vmemmap page contents, so users such as HugeTLB have to open-code
>>>> follow-up handling to restore the metadata afterwards.
>>>> 
>>>> Use the section's compound page order to detect struct pages that fall
>>>> into the shared tail vmemmap range and skip their initialization in
>>>> memmap_init_range().  Still initialize the pageblock migratetypes for
>>>> the skipped range so the surrounding setup remains intact.
>>>> 
>>>> This is a preparatory change for consolidating handling across users of
>>>> vmemmap optimization, and it also avoids redundant initialization of
>>>> shared tail vmemmap pages during early boot.
>>>> 
>>>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>>>> ---
>>>> include/linux/mmzone.h |  4 ++++
>>>> mm/internal.h          | 16 ++++++++++++++++
>>>> mm/mm_init.c           | 25 +++++++++++++++++++------
>>>> 3 files changed, 39 insertions(+), 6 deletions(-)
>>>> 
>>>> @@ -673,19 +673,21 @@ static inline void fixup_hashdist(void)
>>>> static inline void fixup_hashdist(void) {}
>>>> #endif /* CONFIG_NUMA */
>>>> 
>>>> -#if defined(CONFIG_ZONE_DEVICE) || defined(CONFIG_DEFERRED_STRUCT_PAGE_INIT)
>>>> static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
>>>> - unsigned long nr_pages, int migratetype, bool atomic)
>>>> + unsigned long nr_pages, int migratetype, bool isolate, bool atomic)
>>> 
>>> What is isolate parameter for?
>> 
>> I've re-examined the code, and you're right that the isolate parameter is technically
>> redundant for our current use case, as memmap_init_zone_range() passes false.
>> 
>> The rationale behind keeping it is future-proofing. The ultimate goal of a generic
>> HVO is to support arbitrary huge pages, not just HugeTLB. I decoupled this as a
>> parameter to prevent potential regressions down the road; if a developer leverages
>> this for other huge page types in the future, they won't inadvertently break
>> things by forgetting to update a hardcoded false in init_pageblock_migratetype(),
>> especially since memmap_init_range() natively accepts an isolate parameter.
>> 
>> Of course, we could also just delete this parameter for now and add it back later if
>> needed. I think both approaches work.
>> 
>> Which way are you leaning?
> 
> I'd drop it for now and would revisit when there would be a new HVO user. 
> Even one boolean means it's hard to tell from a call site what is the
> intention of the flag, two make it completely confusing :)

No problem. I'll drop it next version.

Muchun,
Thanks.

> 
>>>> {
>>>> const unsigned long end = pfn + nr_pages;
>>>> 
>>>> for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
>>>> - init_pageblock_migratetype(pfn_to_page(pfn), migratetype, false);
>>>> + init_pageblock_migratetype(pfn_to_page(pfn), migratetype, isolate);
>>>> +#ifdef CONFIG_SPARSEMEM
>>>> if (!atomic && IS_ALIGNED(pfn, PAGES_PER_SECTION))
>>>> +#else
>>>> + if (!atomic && IS_ALIGNED(pfn, MAX_FOLIO_NR_PAGES))
>>>> +#endif
>>> 
>>> Let's trigger cond_resched() on some defined number of iterations or some
>>> memory size chunk, e.g PAGES_PER_128M or even PAGES_PER_1G.
>> 
>> Yes, that's for the best. I was really struggling to choose a suitable macro
>> for this earlier, but I realized it's a difficult thing to get right. I'm leaning
>> toward selecting PAGES_PER_1G instead.
> 
> Yeah, PAGES_PER_1G makes sense to me too.
> 
>> Muchun,
>> Thanks.
>> 
>>> 
>>>> cond_resched();
> 
> -- 
> Sincerely yours,
> Mike.


  reply	other threads:[~2026-07-09 13:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02  9:38 [PATCH 00/17] mm: Introduce section-based vmemmap optimization for HugeTLB Muchun Song
2026-07-02  9:38 ` [PATCH 01/17] mm/sparse: drop power-of-2 size requirement for struct mem_section Muchun Song
2026-07-02  9:38 ` [PATCH 02/17] mm/sparse-vmemmap: track compound page order in " Muchun Song
2026-07-02  9:38 ` [PATCH 03/17] mm/sparse-vmemmap: introduce folio-oriented vmemmap optimization macros Muchun Song
2026-07-02  9:38 ` [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages Muchun Song
2026-07-09 10:45   ` Mike Rapoport
2026-07-09 12:31     ` Muchun Song
2026-07-09 13:05       ` Mike Rapoport
2026-07-09 13:23         ` Muchun Song [this message]
2026-07-02  9:38 ` [PATCH 05/17] mm/sparse-vmemmap: initialize shared tail vmemmap pages on allocation Muchun Song
2026-07-02  9:38 ` [PATCH 06/17] mm/sparse-vmemmap: support section-based vmemmap accounting Muchun Song
2026-07-02  9:38 ` [PATCH 07/17] mm/sparse-vmemmap: support section-based vmemmap optimization Muchun Song
2026-07-02  9:38 ` [PATCH 08/17] mm/sparse: mark memory sections present earlier Muchun Song
2026-07-09 10:54   ` Mike Rapoport
2026-07-09 12:35     ` Muchun Song
2026-07-02  9:38 ` [PATCH 09/17] mm/hugetlb: switch HugeTLB to section-based vmemmap optimization Muchun Song
2026-07-02  9:38 ` [PATCH 10/17] mm/mm_init: factor out pfn_to_zone() Muchun Song
2026-07-02  9:38 ` [PATCH 11/17] mm/sparse-vmemmap: remove SPARSEMEM_VMEMMAP_PREINIT support Muchun Song
2026-07-02  9:38 ` [PATCH 12/17] mm/sparse: inline usemap allocation into sparse_init_nid() Muchun Song
2026-07-02  9:38 ` [PATCH 13/17] mm/sparse: remove section_map_size() Muchun Song
2026-07-02  9:38 ` [PATCH 14/17] mm/hugetlb: remove HUGE_BOOTMEM_HVO Muchun Song
2026-07-02  9:38 ` [PATCH 15/17] mm/hugetlb: remove HUGE_BOOTMEM_CMA Muchun Song
2026-07-02  9:38 ` [PATCH 16/17] mm/hugetlb: localize struct huge_bootmem_page Muchun Song
2026-07-02  9:38 ` [PATCH 17/17] mm/hugetlb: localize HUGE_BOOTMEM_ZONES_VALID Muchun Song

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=CFAE1475-6104-483A-86AE-511FB0A1CA2B@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=osalvador@suse.de \
    --cc=rppt@kernel.org \
    --cc=songmuchun@bytedance.com \
    --cc=vbabka@kernel.org \
    /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