Re: [PATCH v3 08/18] mm/huge_memory: move unmap and remap into the split helpers

[David Hildenbrand (Arm)]

On 8/20/26 20:55, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@xxxxxxxxxxx>
> 
> To prepare for further cleanup, move the unmap/remap handling from
> __folio_split() into the split helpers.  Only anon folios need to
> be remapped, so remap_page() is now only called for anon splits and
> the anon check in remap_page() is redundant and can be removed.

I can understand why we would want to move the remap_page().

But why the unmap_folio()?

> 
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
>  mm/huge_memory.c | 58 ++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 01c8cf428595..af9c2edd1fba 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3589,9 +3589,6 @@ static void remap_page(struct folio *folio, unsigned long nr, int flags)
>  {
>  	int i = 0;
>  
> -	/* If unmap_folio() uses try_to_migrate() on file, remove this check */
> -	if (!folio_test_anon(folio))
> -		return;
>  	for (;;) {
>  		remove_migration_ptes(folio, folio, TTU_RMAP_LOCKED | flags);
>  		i += folio_nr_pages(folio);

I think we should rename that function now to unmap_folio, but likely better
unmap_anon_folio().

> @@ -3933,19 +3930,23 @@ static unsigned int folio_cache_ref_count(const struct folio *folio)
>  	return folio_nr_pages(folio);
>  }
>  
> -static int __folio_freeze_split_unmapped_anon(struct folio *folio, unsigned int new_order,
> -					      struct page *split_at, bool do_lru,
> -					      struct list_head *list, enum split_type split_type)
> +static int __folio_split_unmap_and_freeze_anon(struct folio *folio, unsigned int new_order,
> +					       struct page *split_at, bool do_lru, bool unmap,
> +					       struct list_head *list, enum split_type split_type)
>  {
>  	struct folio *end_folio = folio_next(folio);
>  	struct swap_cluster_info *ci = NULL;
>  	struct folio *new_folio, *next;
>  	int old_order = folio_order(folio);
> +	enum ttu_flags ttu_flags = 0;
>  	struct list_lru_one *lru;
>  	struct lruvec *lruvec;
>  	bool dequeue_deferred;
>  	int ret = 0;
>  
> +	if (unmap)
> +		unmap_folio(folio);

I really hate that "bool unmap" parameter.

folio_split_unmapped() has this VM_WARN_ON_ONCE_FOLIO(folio_mapped(folio), folio);

So you can really just do

if (folio_mapped())
	unmap_folio(folio);

Or have an early exit and hide it in unmap_folio().



-- 
Cheers,

David