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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 4/29/26 08:57, Lance Yang wrote:
> 
> On Wed, Apr 29, 2026 at 08:12:55AM +0200, David Hildenbrand (Arm) wrote:
>> On 4/29/26 07:54, Lance Yang wrote:
>>>
>>>
>>> Good catch!
>>>
>>
>> Likely it should be
>>
>> 	Fixes: d82d09e48219 ("mm/huge_memory: mark PMD mappings of the huge zero folio special")
>>
>> Because vm_normal_page_pmd() would return the wrong thing.
> 
> Right.
> 
>> But I am surprised that we didn't run into the
>>
>> 	VM_WARN_ON_ONCE(is_zero_pfn(pfn) || is_huge_zero_pfn(pfn));
>>
>> earlier. 	
> 
> The history seems to be:
> 
> 	2025-09-13 d82d09e48219 ("mm/huge_memory: mark PMD mappings of the huge zero folio special")
> 	2025-09-13 af38538801c6 ("mm/memory: factor out common code from vm_normal_page_*()")
> 
> After d82d09e48219, vm_normal_page_pmd() still had the explicit huge
> zero check before returning the page:
> 
> --8<---
> struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
> 				pmd_t pmd)
> {
> 	unsigned long pfn = pmd_pfn(pmd);
> 
> 	if (unlikely(pmd_special(pmd)))
> 		return NULL;
> 
> 	if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) {
> 		if (vma->vm_flags & VM_MIXEDMAP) {
> 			if (!pfn_valid(pfn))
> 				return NULL;
> 			goto out;
> 		} else {
> 			unsigned long off;
> 			off = (addr - vma->vm_start) >> PAGE_SHIFT;
> 			if (pfn == vma->vm_pgoff + off)
> 				return NULL;
> 			if (!is_cow_mapping(vma->vm_flags))
> 				return NULL;
> 		}
> 	}
> 
> 	if (is_huge_zero_pfn(pfn))
> 		return NULL;
> 	if (unlikely(pfn > highest_memmap_pfn))
> 		return NULL;
> 
> 	/*
> 	 * NOTE! We still have PageReserved() pages in the page tables.
> 	 * eg. VDSO mappings can cause them to exist.
> 	 */
> out:
> 	return pfn_to_page(pfn);
> }
> ---
> 
> So even if pmd_mkspecial() was a no-op and pmd_special() stayed false,
> we would still return NULL there.
> 
> Then af38538801c6 moved the PMD path into __vm_normal_page():
> 
> ---8<---
> struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
> 				pmd_t pmd)
> {
> 	return __vm_normal_page(vma, addr, pmd_pfn(pmd), pmd_special(pmd),
> 				pmd_val(pmd), PGTABLE_LEVEL_PMD);
> }
> ---
> 
> For CONFIG_ARCH_HAS_PTE_SPECIAL=y, __vm_normal_page() only returns NULL
> for the huge zero PFN if special == true. On x86 32-bit, pmd_special()
> stays false, so this can now fall through to VM_WARN_ON_ONCE():
> 
> ---8<---
> 	if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) {
> 		if (unlikely(special)) {
> 			if (is_zero_pfn(pfn) || is_huge_zero_pfn(pfn))
> 				return NULL;
> ...
> 		}
> ...
> 	} else {
> ...
> 		if (is_zero_pfn(pfn) || is_huge_zero_pfn(pfn))
> 			return NULL;
> 	}
> 
> ...
> 	VM_WARN_ON_ONCE(is_zero_pfn(pfn) || is_huge_zero_pfn(pfn));
> ...
> ---
> 
> So my guess is that the warning above became possible after
> af38538801c6, IIUC.

Yes, I think you are right about af38538801c6.

What about the following then as a temporary solution:

diff --git a/mm/memory.c b/mm/memory.c
index 199214f8de08..bf447c8b2f57 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -684,7 +684,9 @@ 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 ((IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL) && level == PGTABLE_LEVEL_PTE) ||
+           (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PMD_PFNMAP) && level == PGTABLE_LEVEL_PMD) ||
+           (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PUD_PFNMAP) && level == PGTABLE_LEVEL_PUD)) {
                if (unlikely(special)) {
 #ifdef CONFIG_FIND_NORMAL_PAGE
                        if (vma->vm_ops && vma->vm_ops->find_normal_page)


We could wrap the check in a fancy helper.

-- 
Cheers,

David