Re: [RESEND v7 11/29] mm: split PMD swap entries into PTE swap entries

[Usama Arif]

On 23/09/2026 12:20, David Hildenbrand (Arm) wrote:
> On 9/14/26 14:28, Usama Arif wrote:
>> Once a PMD can hold a swap entry, everything that splits a PMD - mprotect()
>> or munmap() over part of the range, MADV_FREE, a pagewalk with no PMD
>> handler - has to be able to split that entry too, or the callers that rely
>> on split_huge_pmd() to hand them a PTE table would find the PMD unchanged.
>>
>> No reference counting is needed: a swap entry pins no folio, and swap_map
>> is already one per slot, so the PTEs simply take over what the PMD held.
>>
>> The migration-only entry point cannot reach the new branch, because
>> page_vma_mapped_walk() never hands back a swap PMD for the folio being
>> migrated. Warn if that ever changes, and force the regular split anyway,
>> since the branch leaves folio and page uninitialised.
>>
>> Test the pre-split old_pmd rather than re-reading *pmd in the trailing
>> folio_remove_rmap_pmd() gate, so every entry-type test in the function
>> interrogates the same snapshot. That part is cosmetic: pmdp_invalidate()
>> leaves the PMD present as far as software is concerned.
>>
>> Signed-off-by: Usama Arif <usama.arif@xxxxxxxxx>
>> ---
>>  mm/huge_memory.c | 36 +++++++++++++++++++++++++++++++++++-
>>  1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 873887aed0bc2..0e347a545588c 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3304,6 +3304,21 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>>  			folio_add_anon_rmap_ptes(folio, page, HPAGE_PMD_NR,
>>  						 vma, haddr, rmap_flags);
>>  		}
>> +	} else if (pmd_is_swap_entry(*pmd)) {
>> +		/*
>> +		 * A PMD swap entry has no page, so it cannot be turned into
>> +		 * PTE migration entries.  page_vma_mapped_walk() never hands
>> +		 * one back for the folio being migrated, so this should not
>> +		 * happen; warn, but also force the regular split so that a
>> +		 * broken invariant cannot make the code below dereference the
>> +		 * uninitialised folio and page.
> 
> I disagree with the force (and the comment). We cannot make each and every
> assertion that never happens (unless someone messes up real bad and would find
> this during early testing) have recovery code.

Yeah that makes sense, I will fix it for next revision.> 
> The real bug would be calling split_pmd_to_migration_entries() with something
> unexpected. See my reply to #10 where we bail out earlier
> 
> 
>> +		 */
>> +		VM_WARN_ON_ONCE(use_migration_entries);
>> +		use_migration_entries = false;
> 
> Can we just have on the beginning of the function a check that
> use_migration_entries is only ever set on present PMDs or device-private entries.

I have moved it into the previous patch (#10) as

  VM_WARN_ON_ONCE(to_migration_entries && !pmd_present(*pmd) &&
                  !pmd_is_device_private_entry(*pmd));
> 
>> +		old_pmd = *pmd;
>> +		soft_dirty = pmd_swp_soft_dirty(old_pmd);
>> +		uffd_wp = pmd_swp_uffd(old_pmd);
>> +		anon_exclusive = pmd_swp_exclusive(old_pmd);
>>  	} else {
>>  		/*
>>  		 * Up to this point the pmd is present and huge and userland has
>> @@ -3440,6 +3455,25 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>>  			VM_WARN_ON(!pte_none(ptep_get(pte + i)));
>>  			set_pte_at(mm, addr, pte + i, entry);
>>  		}
>> +	} else if (pmd_is_swap_entry(old_pmd)) {
>> +		const softleaf_t old_entry = softleaf_from_pmd(old_pmd);
>> +		pte_t pte_swp_entry;
>> +		swp_entry_t entry;
>> +
>> +		for (i = 0, addr = haddr; i < HPAGE_PMD_NR;
>> +		     i++, addr += PAGE_SIZE) {
> 
> Just squeeze it into one line like the other instances.

Done for next revision.> 
>> +			entry = swp_entry(swp_type(old_entry),
>> +					  swp_offset(old_entry) + i);
> 
> Didn't we have a helper to advance by a delta? Ah, yes, pte_move_swp_offset().
> 
> I guess one could construct the initial pte and then advance one by one through
> pte_move_swp_offset(). Won't remove a lot of code, though, so just a thought.
> 


Done, it reads better than I expected, because the three bit tests
hoist out of the loop rather than running HPAGE_PMD_NR times:

  } else if (pmd_is_swap_entry(old_pmd)) {
          pte_t entry = softleaf_to_pte(softleaf_from_pmd(old_pmd));

          if (soft_dirty)
                  entry = pte_swp_mksoft_dirty(entry);
          if (uffd_wp)
                  entry = pte_swp_mkuffd(entry);
          if (anon_exclusive)
                  entry = pte_swp_mkexclusive(entry);

          for (i = 0, addr = haddr; i < HPAGE_PMD_NR;
               i++, addr += PAGE_SIZE) {
                  VM_WARN_ON(!pte_none(ptep_get(pte + i)));
                  set_pte_at(mm, addr, pte + i, entry);
                  entry = pte_next_swp_offset(entry);
          }


> Apart from that LGTM.
> 

Thanks for the reviews!!