Re: [PATCH 3/4] mm/sparse-vmemmap: drop vmemmap_can_optimize()

[Muchun Song <muchun.song@linux.dev>]

> On Oct 31, 2026, at 08:54, sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] mm/sparse-vmemmap: dropping vmemmap_can_optimize() bypasses DAX arch config
> - [Medium] mm/sparse-vmemmap: section_activate() stores non-optimizable compound order
> - [Low] mm/sparse-vmemmap: missing header include breaks powerpc init_64.c build
> --
> 
> commit 4171e4b9fc2abb84bf0e245a60563f33f94a263d
> Author: Muchun Song <muchun.song@linux.dev>
> 
> mm/sparse-vmemmap: drop vmemmap_can_optimize()
> 
> This commit removes the DAX-specific vmemmap_can_optimize() helper from
> include/linux/mm.h and relies on section_vmemmap_optimizable() to determine
> whether a section uses vmemmap optimization.
> 
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index e875f73b0c6ff..4fb48fbe712d2 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -38,7 +38,6 @@
>> #include <linux/bitops.h>
>> #include <linux/iommu-debug-pagealloc.h>
>> #include <linux/kcsan-checks.h>
>> -#include <linux/vmemmap-optimization.h>
> 
> [Severity: Low]
> Does removing this include break the 64-bit PowerPC build when
> CONFIG_SPARSEMEM_VMEMMAP=y is enabled?
> 
> In arch/powerpc/mm/init_64.c, vmemmap_populate() calls
> section_set_compound_order():
> 
>    section_set_compound_order(__pfn_to_section(page_to_pfn((struct page *)start)), 0);
> 
> Previously, arch/powerpc/mm/init_64.c got the declaration of
> section_set_compound_order() transitively via include/linux/mm.h. Without
> including linux/vmemmap-optimization.h in arch/powerpc/mm/init_64.c,
> compiling it will fail with an implicit function declaration error.

I should include this header in the patch 2 to fix this, because I don't like
this implicit way of header file inclusion.

> 
>> struct mempolicy;
>> struct anon_vma;
>> @@ -5395,42 +5394,6 @@ static inline void vmem_altmap_free(struct vmem_altmap *altmap,
>> }
>> #endif
>> 
>> -#ifdef CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP
>> -static inline bool __vmemmap_can_optimize(struct vmem_altmap *altmap,
>> -					  struct dev_pagemap *pgmap)
>> -{
>> -	unsigned long nr_pages;
>> -	unsigned long nr_vmemmap_pages;
>> -
>> -	if (!IS_ENABLED(CONFIG_VMEMMAP_OPTIMIZATION))
>> -		return false;
>> -
>> -	if (!pgmap || !is_power_of_2(sizeof(struct page)))
>> -		return false;
>> -
>> -	nr_pages = pgmap_vmemmap_nr(pgmap);
>> -	nr_vmemmap_pages = ((nr_pages * sizeof(struct page)) >> PAGE_SHIFT);
>> -	/*
>> -	 * For vmemmap optimization with DAX we need minimum 2 vmemmap
>> -	 * pages. See layout diagram in Documentation/mm/vmemmap_dedup.rst
>> -	 */
>> -	return !altmap && (nr_vmemmap_pages > VMEMMAP_OPTIMIZATION_PAGES);
>> -}
>> -/*
>> - * If we don't have an architecture override, use the generic rule
>> - */
>> -#ifndef vmemmap_can_optimize
>> -#define vmemmap_can_optimize __vmemmap_can_optimize
>> -#endif
>> -
>> -#else
>> -static inline bool vmemmap_can_optimize(struct vmem_altmap *altmap,
>> -					   struct dev_pagemap *pgmap)
>> -{
>> -	return false;
>> -}
>> -#endif
> 
> [Severity: High]
> With vmemmap_can_optimize() removed, nothing in C code checks
> CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP anymore. Could this unintendedly
> enable DAX vmemmap optimization on architectures that only select
> ARCH_WANT_OPTIMIZE_HUGETLB_VMEMMAP, such as 64-bit LoongArch?

You are right. This patch will enable vmemmap-optimization for device DAX
on LoongArch.

Because I plan to remove ARCH_WANT_OPTIMIZE_HUGETLB_VMEMMAP completely in
the future, so I'll add a new commit before this one to disable HVO for
device DAX on LoongArch. It'll be easy to do this, just like patch 2 does
for PowerPC.

On LoongArch, HugeTLB HVO is supported without gigantic HugeTLB pages. So
there is no users of section-based vmemmap optimization. So just disabling
vmemmap optimization in architectural vmemmap_populate() is enough.

> 
> When CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP=y and CONFIG_ZONE_DEVICE=y are
> both enabled on 64-bit LoongArch, CONFIG_VMEMMAP_OPTIMIZATION is selected.
> In section_activate(), order is now set from pgmap->vmemmap_shift, and
> section_vmemmap_optimizable() -> vmemmap_optimizable_order() only checks
> IS_ENABLED(CONFIG_VMEMMAP_OPTIMIZATION).
> 
> As a result, activating a DAX section with pgmap->vmemmap_shift >=
> VMEMMAP_OPTIMIZATION_MIN_ORDER on LoongArch will map read-only shared tail
> PTEs and grab references to vmemmap_shared_tail_page via vmemmap_alloc_pte().
> However, LoongArch's vmemmap_free() in arch/loongarch/mm/init.c is an empty
> no-op stub:
> 
>    void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *altmap)
>    {
>    }
> 
> On section_deactivate(), those shared tail page references are leaked and
> the read-only PTEs remain mapped, causing subsequent section activations to
> either trigger WARN_ON_ONCE(vmemmap_optimizable_pfn(pfn)) and fail with
> -ENOMEM in vmemmap_pte_populate(), or fault in kernel mode when writing to
> struct pages through the stale read-only PTEs.
> 
> [ ... ]
> 
>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
>> index 3e9d6617c4bb2..43559593107c9 100644
>> --- a/mm/sparse-vmemmap.c
>> +++ b/mm/sparse-vmemmap.c
> 
> [ ... ]
> 
>> @@ -699,10 +707,9 @@ static struct page * __meminit section_activate(int nid, unsigned long pfn,
>> 	struct mem_section *ms = __pfn_to_section(pfn);
>> 	struct mem_section_usage *usage = NULL;
>> 	struct page *memmap;
>> -	unsigned int order;
>> +	unsigned int order = pgmap ? pgmap->vmemmap_shift : 0;
>> 	int rc;
>> 
>> -	order = vmemmap_can_optimize(altmap, pgmap) ? pgmap->vmemmap_shift : 0;
>> 	if (nr_pages < PAGES_PER_SECTION && section_compound_order(ms))
>> 		return ERR_PTR(-EOPNOTSUPP);
> 
> [Severity: Medium]
> What happens in section_activate() when pgmap->vmemmap_shift is non-zero,
> but is not actually optimizable (for example, when 0 < pgmap->vmemmap_shift
> < VMEMMAP_OPTIMIZATION_MIN_ORDER, or when !is_power_of_2(sizeof(struct page)))?

Even though this issue is unreachable, I'm still going to fix it since it is easy.
Something like:

diff --git a/include/linux/vmemmap-optimization.h b/include/linux/vmemmap-optimization.h
index fa9e9abd6656..104f7275a49f 100644
--- a/include/linux/vmemmap-optimization.h
+++ b/include/linux/vmemmap-optimization.h
@@ -42,7 +42,9 @@ static inline void section_set_compound_order(struct mem_section *section,
 {
        VM_WARN_ON(section_compound_order(section) && order &&
                   section_compound_order(section) != order);
-       section->compound_page_order = order;
+
+       if (vmemmap_optimizable_order(order))
+               section->compound_page_order = order;
 }


Thanks.

> 
> Previously, order was forced to 0 when vmemmap_can_optimize() returned
> false. Now, section_activate() passes the raw pgmap->vmemmap_shift to
> section_set_compound_order_range(pfn, nr_pages, order).
> 
> Because __populate_section_memmap() only resets the order when both altmap
> and section_vmemmap_optimizable(ms) are true:
> 
>    if (altmap && section_vmemmap_optimizable(ms))
>            section_set_compound_order(ms, 0);
> 
> ms->compound_page_order remains non-zero for unoptimizable non-zero orders.
> Since section_activate() checks section_compound_order(ms) rather than
> section_vmemmap_optimizable(ms), wouldn't a subsequent partial-section
> activation (nr_pages < PAGES_PER_SECTION) in that section be erroneously
> rejected with -EOPNOTSUPP even though vmemmap optimization was never used?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261030140720.1256062-1-songmuchun@bytedance.com?part=3