Re: [PATCH v6 03/12] mm: add PMD swap entry splitting support

"David Hildenbrand (Arm)" <[email protected]>
Newsgroups org.kvack.linux-mm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 8/18/26 15:09, Usama Arif wrote:
> Add a swap branch in __split_huge_pmd_locked() that splits a PMD swap
> entry into 512 PTE swap entries. No folio reference is needed because
> swap entries point to swap slots rather than pages. Each PTE inherits
> the correct sub-slot offset and preserves soft_dirty, uffd_wp, and
> exclusive flags.
> 
> The folio_remove_rmap_pmd() gate at the end must inspect old_pmd
> rather than *pmd: for a present THP split, *pmd has already been
> cleared by pmdp_invalidate(), and that invalidated bit pattern can
> decode as a plausible swap entry.
> 
> This branch is reached from the explicit __split_huge_pmd() callers
> that hit a non-present PMD: partial-range mprotect / munmap, the
> wp_huge_pmd() PMD-COW fallback, and the swap-in / swapoff fallbacks
> added in later patches when the cached folio is no longer PMD-sized.
> page_vma_mapped_walk() does not iterate PMD swap entries, so
> try_to_unmap_one() and try_to_migrate_one() do not reach this branch
> and freeze=true cannot occur in this branch today.  page and folio
> are therefore left uninitialized in the swap branch; a
> VM_WARN_ON_ONCE(freeze) catches any future caller that breaks this
> invariant before the freeze path dereferences page_to_pfn(page + i)
> or put_page(page).
> 
> Signed-off-by: Usama Arif <[email protected]>
> ---
>  mm/huge_memory.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 1b6b0aa2baa3b..a473e85d30f51 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3252,6 +3252,14 @@ 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)) {
> +		VM_WARN_ON_ONCE(freeze);
> +		/* Swap entries have no page for the migration freeze path. */
> +		freeze = false;

It's odd to VM_WARN_ON_ONCE() and then set freeze=false;

I'd just add the comment above the VM_WARN_ON_ONCE() and drop the =false.

freeze=true really only applies during page migration, where swap entries don't
apply.

I think it's time to clean that up ... that is

a) Expose a helper called split_pmd_to_migration_entries() that is only used by
code that installs migration entries.

b) Hide that "freeze" flag from all other file-external functions

c) Rename the boolean to "use_migration_entries"

Then it's rather clear in this code that this should never happen.



> +		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
> @@ -3388,6 +3396,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)) {
> +		softleaf_t sl_entry = softleaf_from_pmd(old_pmd);

No existing code uses "sl_entry". Maybe just call it "pmd_swp_entry"/"swp_entry"
and below "pte_swp_entry".



Apart from that nothing jumped at me :)

-- 
Cheers,

David
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.