Re: [PATCH RFC 07/13] mm/huge_memory: move unmap and remap into the split helpers

On Fri Aug 7, 2026 at 5:17 PM EDT, Kairui Song via B4 Relay wrote:
> From: Kairui Song <[email protected]>
>
> 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.
>
> Signed-off-by: Kairui Song <[email protected]>
> ---
>  mm/huge_memory.c | 51 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 72f5d0d24127..c0115841d1a0 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 */

This comment is lost. Like it says, if file folios are unmappped using
migration entries, __folio_freeze_split_unmapped_file() will need to
call remap_page(). Can you move this comment to the end of
__folio_freeze_split_unmap_file(), where remap_page() could be called.

> -	if (!folio_test_anon(folio))
> -		return;
>  	for (;;) {
>  		remove_migration_ptes(folio, folio, TTU_RMAP_LOCKED | flags);
>  		i += folio_nr_pages(folio);
> @@ -3934,19 +3931,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_freeze_split_unmap_anon(struct folio *folio, unsigned int new_order,

__folio_split_unmap_and_freeze_anon() might be better?

> +					   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;
>  	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);

This unmap parameter is to accommodate folio_split_unmapped() and
counter-intuitive for __folio_freeze_split_unmap_anon() this function
name. __folio_split_may_unmap_and_freeze_anon() might match the code
better, but sounds ugly. :(

With the moved comment, feel free to add

Reviewed-by: Zi Yan <[email protected]>

The function name is secondary.


-- 
Best Regards,
Yan, Zi