Re: [PATCH] mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd()

[Konstantin Khorenko <khorenko@virtuozzo.com>]

On 10/14/26 15:19, Lance Yang wrote:
> 
> 
> On 2026/10/14 19:57, Konstantin Khorenko wrote:
> [...]
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 1e5d68acf62a5..fcea98f0d8865 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -1965,13 +1965,22 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>>   		src_ptl = pmd_lockptr(src_mm, src_pmd);
>>   		spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
>>   		/*
>> -		 * No need to recheck the pmd, it can't change with write
>> -		 * mmap lock held here.
>> -		 *
>> -		 * Meanwhile, making sure it's not a CoW VMA with writable
>> -		 * mapping, otherwise it means either the anon page wrongly
>> -		 * applied special bit, or we made the PRIVATE mapping be
>> -		 * able to wrongly write to the backend MMIO.
>> +		 * The mmap and VMA write locks of the source keep faults
>> +		 * out, but unmap_mapping_range() does not take the mmap
>> +		 * lock and may have cleared the pmd since the lockless
>> +		 * read. Let copy_pmd_range() skip it then.
>> +		 */
>> +		pmd = pmdp_get(src_pmd);
>> +		if (unlikely(!pmd_present(pmd) || !pmd_special(pmd))) {
> 
> Why not just check pmd_none(pmd) here? We're holding the source mmap/VMA
> write locks, so the PMD either stays special or gets cleared, right?

Right: with the source mmap and VMA write locks held, only a zap can
change it, and both ways a zap takes leave the PMD none:
zap_huge_pmd() clears it, and a partial zap splits it in
__split_huge_pmd_locked(), which for a special VMA only clears it and
returns.

I tested the same condition as the lockless check that chose this
branch, so that the branch never copies anything it did not select,
but taking into account the above paragraph we can easily use pmd_none()
here, it even better matches the comment :), so I will switch to it in v3.

v2 is already out, with a second patch for the missing page table
deposit on ppc64 hash:
https://lore.kernel.org/r/20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v2-0-e1548488fa68@virtuozzo.com

The pmd_none() change will go into v3.

Cheers, Konstantin


>> +			spin_unlock(src_ptl);
>> +			spin_unlock(dst_ptl);
>> +			return -EAGAIN;
>> +		}
>> +		/*
>> +		 * Make sure it's not a CoW VMA with writable mapping,
>> +		 * otherwise it means either the anon page wrongly applied
>> +		 * special bit, or we made the PRIVATE mapping be able to
>> +		 * wrongly write to the backend MMIO.
>>   		 */
>>   		VM_WARN_ON_ONCE(vma_is_cow_mapping(src_vma) && pmd_write(pmd));
>>   		goto set_pmd;
>>
>> ---
>> base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
>> change-id: 20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-bdf7efb612c2
>>
>> Best regards,
>> --
>> Konstantin Khorenko <khorenko@virtuozzo.com>
>>
>