Re: [PATCH hotfix] mm: fix pmd_special() fallback to observe huge_zero

[Hugh Dickins <hughd@google.com>]

On Thu, 30 Apr 2026, David Hildenbrand (Arm) wrote:
> 
> Okay, I'd say we do the following:
> 
> From fd9ead548f102f7c257980ccc7b96cce7e42a570 Mon Sep 17 00:00:00 2001
> From: Hugh Dickins <hughd@google.com>
> Date: Thu, 30 Apr 2026 07:35:31 +0200
> Subject: [PATCH] mm: fix pmd_special() fallback to observe huge_zero

No to those lines, this patch of yours is quite different.

> 
> On x86 32-bit with THP enabled, zap_huge_pmd() is seen to generate a
> "WARNING: mm/memory.c:735 at __vm_normal_page+0x6a/0x7d", from the
> VM_WARN_ON_ONCE(is_zero_pfn(pfn) || is_huge_zero_pfn(pfn)); followed
> by "BUG: Bad rss-counter state"s, then later "BUG: Bad page state"s
> when reclaim gets to call shrink_huge_zero_folio_scan().
> 
> It's as if the _PAGE_SPECIAL bit never got set in the huge_zero pmd:
> and indeed, whereas pte_special() and pte_mkspecial() are subject to a
> dedicated CONFIG_ARCH_HAS_PTE_SPECIAL, pmd_special() and pmd_mkspecial()
> are subject to CONFIG_ARCH_SUPPORTS_PMD_PFNMAP, which is never enabled
> on any 32-bit architecture.
> 
> While the problem was exposed through d80a9cb1a64a ("mm/huge_memory: add
> and use normal_or_softleaf_folio_pmd()"), it was an oversight in
> af38538801c6 ("mm/memory: factor out common code from vm_normal_page_*()")
> and would result in other problems:
> * huge zero folio accounted in smaps, pagemap (PAGE_IS_FILE) and
>   numamaps as file-backed THP
> * folio_walk_start() returning the folio even without FW_ZEROPAGE set.
>   Callers seem to tolerate that, though.

Yes, I hadn't thought to check other uses when I posted yesterday;
but later took a look through, and was coming to exactly the conclusion
you reach in that paragraph (well, I gave up before following through
far enough on damon: perhaps it could also have been affected).

> 
> ... and triggering the VM_WARN_ON_ONE(), although never reported so far.
> 
> To fix it, teach vm_normal_page_pmd()/vm_normal_page_pud() whether
> pmd_special/pud_special is actually implemented.
> 
> Fixes: af38538801c6 ("mm/memory: factor out common code from vm_normal_page_*()")

Agreed.  My Fixes tag (where my bisection arrived) was correct for the
common zap_huge_pmd() symptom I was seeing (Lorenzo's commit removed
an independent is_huge_zero_pmd() check from it, so it now relies on
vm_normal_folio_pmd() to give the right answer).  But you've chased
up other usages, and realized it goes back further.  You could just as
well blame the other commit mentioned in this thread, the d82d09e48219
("mm/huge_memory: mark PMD mappings of the huge zero folio special"),
because in come configs it is not doing what it expects to be doing.
But af38538801c6 is where effects start appearing, so fine to blame
it (and both come from the same 6.18 series, do it doesn't matter).

> Signed-off-by: Hugh Dickins <hughd@google.com>>
> Co-developed-by: David Hildenbrand (Arm) <david@kernel.org>

That's generous, but the patch is not mine at all, and I'll
happily let you grab my two paragraphs above.  Please, just
Reported-by: Hugh Dickins <hughd@google.com>

> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
>  mm/memory.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 7322a40e73b9..a60bc07b48b2 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -612,6 +612,21 @@ static void print_bad_page_map(struct vm_area_struct *vma,
>  	dump_stack();
>  	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
>  }
> +
> +static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
> +{
> +	switch (level) {
> +	case PGTABLE_LEVEL_PTE:
> +		return IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL);
> +	case PGTABLE_LEVEL_PMD:
> +		return IS_ENABLED(CONFIG_ARCH_SUPPORTS_PMD_PFNMAP);
> +	case PGTABLE_LEVEL_PUD:
> +		return IS_ENABLED(CONFIG_ARCH_SUPPORTS_PUD_PFNMAP);
> +	default:
> +		return false;
> +	}
> +}
> +
>  #define print_bad_pte(vma, addr, pte, page) \
>  	print_bad_page_map(vma, addr, pte_val(pte), page, PGTABLE_LEVEL_PTE)
>  
> @@ -684,7 +699,7 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
>  		unsigned long addr, unsigned long pfn, bool special,
>  		unsigned long long entry, enum pgtable_level level)
>  {
> -	if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) {
> +	if (pgtable_level_has_pxx_special(level)) {
>  		if (unlikely(special)) {
>  #ifdef CONFIG_FIND_NORMAL_PAGE
>  			if (vma->vm_ops && vma->vm_ops->find_normal_page)

That block ends with a comment on CONFIG_ARCH_HAS_PTE_SPECIAL:
perhaps better reworded now - but I don't know what to suggest!

This patch seems okay, but TBH I have no enthusiasm for it -
it forces me to think too hard, and I prefer my own one-liner
(iwhich Lance found odd: odd if you're thinking pmd_special(pmd)
means pmd has the _PAGE_SPECIAL bit set, yes, but not odd if you
think it means that pmd is of a special folio).

But whatever, you and Lance prefer this one: thanks for the fix!
Hugh