Re: [PATCH v5 09/17] mm/huge_memory: rename remap_page() to remap_anon_folio()

[David Hildenbrand (Arm)]

On 9/14/26 19:14, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@xxxxxxxxxxx>
> 
> remap_page() now only has one caller, __folio_freeze_split_anon(),
> and is only ever called for anon folios: unmap_folio() currently
> leaves file folios unmapped after the split, so they need no
> remapping.
> 
> Rename it to remap_anon_folio() to make that explicit, and add a
> VM_WARN_ON_FOLIO() documenting it.
> 
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
>  mm/huge_memory.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 1749905ade6a..77bf68c9b9af 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3552,7 +3552,7 @@ static void unmap_folio(struct folio *folio)
>  	/*
>  	 * Anon pages need migration entries to preserve them, but file
>  	 * pages can simply be left unmapped, then faulted back on demand.
> -	 * If that is ever changed (perhaps for mlock), update remap_page().
> +	 * If that is ever changed (perhaps for mlock), update remap_anon_folio().

See my comment below, delete that comment about "what if X" entirely. Also, we
wouldn't want to update remap_anon_folio().

>  	 */
>  	if (folio_test_anon(folio))
>  		try_to_migrate(folio, ttu_flags);
> @@ -3637,10 +3637,17 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>  	return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio);
>  }
>  
> -static void remap_page(struct folio *folio, unsigned long nr, int flags)
> +static void remap_anon_folio(struct folio *folio, unsigned long nr, int flags)
>  {
>  	int i = 0;
>  
> +	/*
> +	 * unmap_folio() installs migration entries only for anon folios,
> +	 * so currently only anon folios need to be remapped. File folios
> +	 * stay unmapped after the split and are faulted back on demand.
> +	 */
> +	VM_WARN_ON_FOLIO(!folio_test_anon(folio), folio);
> +
>  	for (;;) {
>  		remove_migration_ptes(folio, folio, TTU_RMAP_LOCKED | flags);
>  		i += folio_nr_pages(folio);
> @@ -3724,7 +3731,7 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
>  		 *
>  		 * Note that for mapped sub-pages of an anonymous THP,
>  		 * PG_anon_exclusive has been cleared in unmap_folio() and is stored in
> -		 * the migration entry instead from where remap_page() will restore it.
> +		 * the migration entry instead from where remap_anon_folio() will restore it.
>  		 * We can still have PG_anon_exclusive set on effectively unmapped and
>  		 * unreferenced sub-pages of an anonymous THP: we can simply drop
>  		 * PG_anon_exclusive (-> PG_mappedtodisk) for these here.
> @@ -4084,7 +4091,7 @@ static int __folio_freeze_split_anon(struct folio *folio,
>  	if (need_remap) {
>  		if (!ret && !folio_is_device_private(folio))
>  			ttu_flags = TTU_USE_SHARED_ZEROPAGE;
> -		remap_page(folio, 1 << old_order, ttu_flags);
> +		remap_anon_folio(folio, 1 << old_order, ttu_flags);
>  	}
>  
>  	return ret;
> @@ -4206,7 +4213,7 @@ static int __folio_freeze_split_file(struct folio *folio,
>  fail:
>  	/*
>  	 * If we want to use try_to_migrate() on file in unmap_folio,
> -	 * remember to add remap_page() and adapt it.
> +	 * remember to add remap_anon_folio() and adapt it.
>  	 */

Add remap_anon_folio for file folios? Just delete this entire code comment.
Whoever wants to implement that can look into the pieces that are actually needed.


With the comments sorted

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David