Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: Huacai Chen <chenhuacai@kernel.org>
Cc: sashiko-reviews@lists.linux.dev,
	Muchun Song <songmuchun@bytedance.com>,
	Oscar Salvador <osalvador@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Rapoport <rppt@kernel.org>, Qi Zheng <qi.zheng@linux.dev>,
	Lance Yang <lance.yang@linux.dev>,
	James Houghton <jthoughton@google.com>,
	David Hildenbrand <david@kernel.org>,
	linux-mm@kvack.org, WANG Xuerui <kernel@xen0n.name>,
	loongarch@lists.linux.dev
Subject: Re: [PATCH v2 07/12] mm: hugetlb_vmemmap: simplify shared tail page handling
Date: Tue, 3 Nov 2026 11:26:11 +0800	[thread overview]
Message-ID: <06A12352-99CF-4611-96FB-9943A2E6BD50@linux.dev> (raw)
In-Reply-To: <CAAhV-H4BPOJJy367p0MZcEWk3WziV3rPiCtoKPSGOqrQt5W1Pw@mail.gmail.com>



> On Nov 2, 2026, at 21:48, Huacai Chen <chenhuacai@kernel.org> wrote:
> 
> On Mon, Nov 2, 2026 at 2:12 PM Muchun Song <muchun.song@linux.dev> wrote:
>> 
>> 
>> 
>>> On Nov 2, 2026, at 11:52, sashiko-bot@kernel.org wrote:
>>> 
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>> - [High] mm: hugetlb_vmemmap: uninitialized tail page read in vmemmap_optimize_pte()
>>> --
>>> 
>>> commit 32511c4a4d8ec3d45e0f629f89666a78dc9372ee
>>> Author: Muchun Song <muchun.song@linux.dev>
>>> 
>>> mm: hugetlb_vmemmap: simplify shared tail page handling
>>> 
>>> This commit removes the vmemmap_tail field from struct vmemmap_remap_walk and
>>> updates vmemmap_optimize_pte() and vmemmap_restore_pte() to derive the shared
>>> tail page directly from the folio being processed.
>>> 
>>>> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
>>>> index 5e84004db4b87..d0af251aa533c 100644
>>>> --- a/mm/hugetlb_vmemmap.c
>>>> +++ b/mm/hugetlb_vmemmap.c
>>> 
>>> [ ... ]
>>> 
>>>> @@ -213,53 +211,53 @@ static struct page *vmemmap_dup_page_from_list(const struct page *src,
>>>>     return dst;
>>>> }
>>>> 
>>>> -static void vmemmap_optimize_pte(pte_t *pte, unsigned long addr,
>>>> -                             struct vmemmap_remap_walk *walk)
>>>> +static inline struct page *folio_vmemmap_shared_tail_page(const struct folio *folio)
>>>> {
>>>> -    struct page *page = pte_page(ptep_get(pte));
>>>> -    pte_t entry;
>>>> -
>>>> -    /* Remapping the head page requires r/w */
>>>> -    if (unlikely(walk->nr_walked == 0)) {
>>>> -            struct page *head;
>>>> +    return vmemmap_shared_tail_page(folio_order(folio), folio_zone(folio));
>>>> +}
>>>> 
>>>> -            VM_WARN_ON_ONCE(!PageHead((const struct page *)addr));
>>>> +static int vmemmap_optimize_pte(pte_t *pte, unsigned long addr,
>>>> +                            struct vmemmap_remap_walk *walk)
>>>> +{
>>>> +    const struct page *page = (const struct page *)addr;
>>>> +    const struct folio *folio = page_folio(page);
>>>> +    struct page *src = pte_page(ptep_get(pte)), *dst;
>>>> 
>>>> -            head = vmemmap_dup_page_from_list(page, walk->vmemmap_pages);
>>>> -            entry = mk_pte(head, PAGE_KERNEL);
>>>> +    if (likely(vmemmap_optimizable_pfn(page_to_pfn(page), folio_order(folio)))) {
>>>> +            dst = folio_vmemmap_shared_tail_page(folio);
>>>> +            if (!dst)
>>>> +                    return -ENOMEM;
>>>>     } else {
>>>> -            VM_WARN_ON_ONCE(!PageTail((const struct page *)addr));
>>>> -
>>>> -            /*
>>>> -             * Map the shared tail page writable. A lockless get that read
>>>> -             * a non-zero refcount before the remap can still do its
>>>> -             * cmpxchg after it. The refcount is zero, so the cmpxchg
>>>> -             * fails and leaves the page unchanged.
>>>> -             */
>>>> -            entry = mk_pte(walk->vmemmap_tail, PAGE_KERNEL);
>>>> +            dst = vmemmap_dup_page_from_list(src, walk->vmemmap_pages);
>>>>     }
>>> 
>>> [Severity: High]
>>> Can calling page_folio(page), folio_order(folio), and
>>> folio_vmemmap_shared_tail_page(folio) on every PTE in vmemmap_optimize_pte()
>>> read uninitialized tail struct pages for bootmem gigantic hugepages?
>>> 
>>> During early boot in alloc_bootmem_huge_page(), gigantic hugepages mark all
>>> tail pages [pfn + 1, pfn + nr_pages) with memblock_reserved_mark_noinit(), so
>>> memmap_init_reserved_pages() skips initializing their struct pages:
>>> 
>>> mm/hugetlb.c:alloc_bootmem_huge_page() {
>>>   ...
>>>      if (hugetlb_vmemmap_optimizable(h))
>>>              hugetlb_vmemmap_optimize_bootmem_page(pfn, huge_page_order(h));
>>>   ...
>>>      memblock_reserved_mark_noinit(PFN_PHYS(pfn + 1),
>>>                                    (nr_pages - 1) * PAGE_SIZE);
>>> }
>>> 
>>> When a bootmem gigantic hugepage is not aligned to PAGES_PER_SECTION (for
>>> example, when nr_pages < PAGES_PER_SECTION, such as on 64-bit LoongArch with
>>> 16KB base pages where SECTION_SIZE_BITS is 29 and PMD_SHIFT is 25 with
>>> CONFIG_ARCH_FORCE_MAX_ORDER=10), section_set_compound_order_range() returns
>> 
>> This issue does not occur with the default LoongArch configuration. For
>> 16KB base pages, LoongArch defaults ARCH_FORCE_MAX_ORDER to 11, which
>> matches HUGETLB_PAGE_ORDER.
>> 
>> The reported case requires manually overriding CONFIG_ARCH_FORCE_MAX_ORDER
>> to 10 while keeping HugeTLB enabled. This creates an **invalid configuration**
>> that Kconfig currently permits:
>> 
>> HUGETLB_PAGE_ORDER      = 11
>> MAX_FOLIO_ORDER         = 10
>> 
>> LoongArch does not select ARCH_HAS_GIGANTIC_PAGE, so the generic MM code
>> assumes that folios cannot exceed MAX_PAGE_ORDER. The forced configuration
>> violates that assumption and is already diagnosed by:
>> 
>>        WARN_ON(order > MAX_FOLIO_ORDER);
>> 
>> in hugetlb_add_hstate().
>> 
>> Therefore, this is not a regression in a supported LoongArch configuration,
>> and I don't think this series needs to handle it. The proper fix is a separate
>> LoongArch Kconfig change preventing ARCH_FORCE_MAX_ORDER from being set below
>> the HugeTLB order when HugeTLB is enabled.
> Yes, this isn't a regression, the problem existed before this series.
> 
> I suggest change BUILD_BUG_ON_INVALID(HUGETLB_PAGE_ORDER >
> MAX_FOLIO_ORDER) to MAYBE_BUILD_BUG_ON(HUGETLB_PAGE_ORDER >
> MAX_FOLIO_ORDER) in mm/hugetlb.c, which cause a build error
> immediately for invalid ARCH_FORCE_MAX_ORDER configurations.

Thanks for your suggestion. I've sent a fix here [1].

[1] https://lore.kernel.org/all/20261103023831.1805473-1-songmuchun@bytedance.com/

> 
> Huacai
> 
>> 
>> Cc LoongArch maintainers.
>> 
>> Thanks,
>> Muchun
>> 
>>> early without setting section->compound_page_order:
>>> 
>>> include/linux/vmemmap-optimization.h:section_set_compound_order_range() {
>>>      unsigned long section_nr = pfn_to_section_nr(pfn);
>>> 
>>>      if (!IS_ALIGNED(pfn | nr_pages, PAGES_PER_SECTION))
>>>              return;
>>>   ...
>>> }
>>> 
>>> In that case, sparse_init() allocates regular vmemmap pages via
>>> memmap_alloc(), leaving them uninitialized (or filled with 0xff under
>>> CONFIG_DEBUG_VM).
>>> 
>>> Later, gather_bootmem_prealloc_node() initializes only the first
>>> VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES struct pages (covering only the first
>>> vmemmap PTE), and prep_and_add_bootmem_folios() defers initializing the
>>> remaining tail struct pages until after hugetlb_vmemmap_optimize_folios()
>>> runs and fails:
>>> 
>>> mm/hugetlb.c:gather_bootmem_prealloc_node() {
>>>   ...
>>>              hugetlb_folio_init_vmemmap(folio, h,
>>>                                         VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES);
>>>              init_new_hugetlb_folio(folio);
>>> 
>>>              if (vmemmap_optimizable_order(pfn_to_section_compound_order(pfn)))
>>>                      folio_set_hugetlb_vmemmap_optimized(folio);
>>>   ...
>>> }
>>> 
>>> mm/hugetlb.c:prep_and_add_bootmem_folios() {
>>>      /* Send list for bulk vmemmap optimization processing */
>>>      hugetlb_vmemmap_optimize_folios(h, folio_list);
>>> 
>>>      list_for_each_entry_safe(folio, tmp_f, folio_list, lru) {
>>>              if (!folio_test_hugetlb_vmemmap_optimized(folio)) {
>>>                      hugetlb_folio_init_tail_vmemmap(folio, h,
>>>                                      VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES,
>>>                                      pages_per_huge_page(h));
>>>              }
>>>   ...
>>> }
>>> 
>>> Previously, __hugetlb_vmemmap_optimize_folio() derived vmemmap_tail upfront
>>> from the initialized head folio, and vmemmap_optimize_pte() checked
>>> walk->nr_walked == 0 without inspecting tail struct pages.
>>> 
>>> With this change, on the second and subsequent PTEs (where addr >=
>>> vmemmap_start + VMEMMAP_OPTIMIZATION_SIZE), page->compound_info in the tail
>>> struct page at addr has not been initialized by prep_compound_tail() yet:
>>> 
>>> 1. If the raw vmemmap backing pages are zeroed, page_folio(page) returns page
>>> itself rather than the head folio, folio_order(folio) returns 0, and
>>> vmemmap_optimizable_pfn() returns false. vmemmap_optimize_pte() then falls
>>> back to vmemmap_dup_page_from_list(src, walk->vmemmap_pages) for every tail
>>> PTE, rotating the displaced vmemmap pages across the tail PTEs instead of
>>> mapping the shared tail page, while leaving HPG_vmemmap_optimized set so
>>> prep_and_add_bootmem_folios() skips hugetlb_folio_init_tail_vmemmap().
>>> 
>>> 2. If CONFIG_DEBUG_VM is enabled, page_init_poison() fills the uninitialized
>>> tail struct pages with 0xff. _compound_head(page) then evaluates to the
>>> poisoned tail page itself, and folio_order(folio) reads 0xff, leading to an
>>> out-of-bounds shift in vmemmap_optimizable_pfn() and an out-of-bounds
>>> node_zones access in folio_zone(folio).
>>> 
>>> --
>>> Sashiko AI review · https://sashiko.dev/#/patchset/20261102032525.3597743-1-songmuchun@bytedance.com?part=7




  reply	other threads:[~2026-11-03  3:26 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02  3:25 [PATCH v2 00/12] mm: Decouple folio-based vmemmap optimization from HugeTLB Muchun Song
2026-11-02  3:25 ` [PATCH v2 01/12] mm: hugetlb_vmemmap: remove HugeTLB-specific vmemmap optimization macros Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 02/12] mm: hugetlb_vmemmap: unify VMEMMAP_{SPLIT,REMAP}_NO_TLB_FLUSH Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02 11:30     ` Muchun Song
2026-11-02 12:28     ` Muchun Song
2026-11-02  3:25 ` [PATCH v2 03/12] mm: hugetlb_vmemmap: simplify vmemmap head page handling Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 04/12] mm: hugetlb_vmemmap: generalize vmemmap page allocation Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02 12:08     ` Muchun Song
2026-11-02 12:34     ` Muchun Song
2026-11-02 17:57       ` Mike Rapoport
2026-11-03  3:21         ` Muchun Song
2026-11-02  3:25 ` [PATCH v2 05/12] mm: hugetlb_vmemmap: unify writable vmemmap page preparation Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 06/12] mm: hugetlb_vmemmap: rename vmemmap_remap_pte() to vmemmap_optimize_pte() Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 07/12] mm: hugetlb_vmemmap: simplify shared tail page handling Muchun Song
2026-11-02  3:52   ` sashiko-bot
2026-11-02  6:12     ` Muchun Song
2026-11-02 13:48       ` Huacai Chen
2026-11-03  3:26         ` Muchun Song [this message]
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 08/12] mm: hugetlb_vmemmap: move alloc_vmemmap_page_list() next to its free helper Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 09/12] mm: hugetlb_vmemmap: make vmemmap optimization folio-oriented Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 10/12] mm: hugetlb_vmemmap: make vmemmap restoration folio-oriented Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 11/12] mm: hugetlb_vmemmap: unify vmemmap range restoration Muchun Song
2026-11-02 10:18   ` Mike Rapoport
2026-11-02  3:25 ` [PATCH v2 12/12] mm: hugetlb_vmemmap: make vmemmap PTE conversion folio-oriented Muchun Song
2026-11-02 10:18   ` Mike Rapoport

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=06A12352-99CF-4611-96FB-9943A2E6BD50@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=chenhuacai@kernel.org \
    --cc=david@kernel.org \
    --cc=jthoughton@google.com \
    --cc=kernel@xen0n.name \
    --cc=lance.yang@linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=loongarch@lists.linux.dev \
    --cc=osalvador@suse.de \
    --cc=qi.zheng@linux.dev \
    --cc=rppt@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox