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

[Lance Yang <lance.yang@linux.dev>]

On 2026/4/29 15:14, David Hildenbrand (Arm) wrote:
> 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:

Nice, that works for me :)

> 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,
Lance