Re: [PATCH RFC 08/13] mm/huge_memory: move anon_vma and filemap management into split helpers

[Zi Yan]

On Fri Aug 7, 2026 at 5:17 PM EDT, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Only anon split needs vma info, and only file split needs the filemap
> handling. Move the related code into separate helpers so they are
> genuinely more self-contained.
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
>  mm/huge_memory.c | 177 +++++++++++++++++++++++++------------------------------
>  1 file changed, 79 insertions(+), 98 deletions(-)
>

<snip>

>  static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new_order,
> -					   struct page *split_at, struct xa_state *xas,
> -					   struct address_space *mapping, bool do_lru,
> +					   struct page *split_at, bool do_lru,
>  					   struct list_head *list, enum split_type split_type)
>  {
> +	struct address_space *mapping = folio->mapping;
> +	XA_STATE(xas, &mapping->i_pages, folio->index);

mapping should be always non-NULL now. mapping->i_pages no longer exists
for anon code path. I like it.

>  	struct folio *end_folio = folio_next(folio);
>  	struct folio *new_folio, *next;
>  	int nr_shmem_dropped = 0;
> +	unsigned int min_order;
>  	struct lruvec *lruvec;
>  	pgoff_t end = 0;
> -	int ret;
> +	gfp_t gfp;
> +	int ret = 0;
> +
> +	min_order = mapping_min_folio_order(mapping);
> +	if (new_order < min_order)
> +		return -EINVAL;
> +
> +	gfp = current_gfp_context(mapping_gfp_mask(mapping) & GFP_RECLAIM_MASK);
> +	if (!filemap_release_folio(folio, gfp))
> +		return -EBUSY;
> +
> +	mapping_set_update(&xas, mapping);
> +
> +	if (split_type == SPLIT_TYPE_UNIFORM) {
> +		int old_order = folio_order(folio);
> +
> +		xas_set_order(&xas, folio->index, new_order);
> +		xas_split_alloc(&xas, folio, old_order, gfp);
> +		if (xas_error(&xas)) {
> +			ret = xas_error(&xas);
> +			goto fail_free;
> +		}
> +	}
> +
> +	i_mmap_lock_read(mapping);
> +
> +	/* Racy check if we can split the page, before unmap_folio() */
> +	if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1) {
> +		ret = -EAGAIN;
> +		goto fail_mmap_unlock;
> +	}
>  
>  	/*
>  	 *__split_unmapped_folio() may need to trim off pages beyond
> @@ -4060,13 +4118,13 @@ static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new
>  
>  	unmap_folio(folio);
>  
> -	xas_lock_irq(xas);
> +	xas_lock_irq(&xas);
>  
>  	/*
>  	 * Check if the folio is present in page cache.
>  	 * We assume all tail are present too, if folio is there.
>  	 */
> -	if (xas_load(xas) != folio) {
> +	if (xas_load(&xas) != folio) {
>  		ret = -EAGAIN;
>  		goto fail;
>  	}
> @@ -4093,7 +4151,7 @@ static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new
>  	if (do_lru)
>  		lruvec = folio_lruvec_lock(folio);
>  
> -	ret = __split_unmapped_folio(folio, new_order, split_at, xas,
> +	ret = __split_unmapped_folio(folio, new_order, split_at, &xas,
>  				     mapping, split_type);
>  
>  	/*
> @@ -4145,9 +4203,19 @@ static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new
>  		lruvec_unlock(lruvec);
>  
>  fail:
> -	xas_unlock_irq(xas);
> +	xas_unlock_irq(&xas);
> +fail_mmap_unlock:
>  	if (nr_shmem_dropped)
>  		shmem_uncharge(mapping->host, nr_shmem_dropped);
> +	/*
> +	 * Drop the mapping while the inode is still pinned. @folio stays
> +	 * locked and present in the page cache, so eviction cannot free
> +	 * the inode yet, nothing past this point may touch the inode or
> +	 * the mapping.
> +	 */
> +	i_mmap_unlock_read(mapping);
> +fail_free:
> +	xas_destroy(&xas);
>  	return ret;
>  }
>  
> @@ -4176,12 +4244,9 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  		struct page *split_at, struct page *lock_at,
>  		struct list_head *list, enum split_type split_type)
>  {
> -	XA_STATE(xas, &folio->mapping->i_pages, folio->index);
>  	struct folio *end_folio = folio_next(folio);
>  	bool is_anon = folio_test_anon(folio);
>  	struct mem_cgroup *memcg, *old_memcg;
> -	struct address_space *mapping = NULL;
> -	struct anon_vma *anon_vma = NULL;
>  	int old_order = folio_order(folio);
>  	struct folio *new_folio, *next;
>  	int ret;
> @@ -4212,84 +4277,12 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	memcg = get_mem_cgroup_from_folio(folio);
>  	old_memcg = set_active_memcg(memcg);
>  
> -	if (is_anon) {
> -		/*
> -		 * The caller does not necessarily hold an mmap_lock that would
> -		 * prevent the anon_vma disappearing so we first we take a
> -		 * reference to it and then lock the anon_vma for write. This
> -		 * is similar to folio_lock_anon_vma_read except the write lock
> -		 * is taken to serialise against parallel split or collapse
> -		 * operations.
> -		 */
> -		anon_vma = folio_get_anon_vma(folio);
> -		if (!anon_vma) {
> -			ret = -EBUSY;
> -			goto out;
> -		}
> -		anon_vma_lock_write(anon_vma);
> -		mapping = NULL;
> -	} else {
> -		unsigned int min_order;
> -		gfp_t gfp;
> -
> -		mapping = folio->mapping;
> -		min_order = mapping_min_folio_order(mapping);
> -		if (new_order < min_order) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		gfp = current_gfp_context(mapping_gfp_mask(mapping) &
> -							GFP_RECLAIM_MASK);
> -
> -		if (!filemap_release_folio(folio, gfp)) {
> -			ret = -EBUSY;
> -			goto out;
> -		}
> -
> -		mapping_set_update(&xas, mapping);
> -
> -		if (split_type == SPLIT_TYPE_UNIFORM) {
> -			xas_set_order(&xas, folio->index, new_order);
> -			xas_split_alloc(&xas, folio, old_order, gfp);
> -			if (xas_error(&xas)) {
> -				ret = xas_error(&xas);
> -				goto out;
> -			}
> -		}
> -
> -		anon_vma = NULL;
> -		i_mmap_lock_read(mapping);
> -	}
> -
> -	/*
> -	 * Racy check if we can split the page, before unmap_folio() will
> -	 * split PMDs
> -	 */
> -	if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1) {
> -		ret = -EAGAIN;
> -		goto out_unlock;
> -	}
> -
> -	if (!is_anon) {
> -		ret = __folio_freeze_split_unmap_file(folio, new_order, split_at, &xas, mapping,
> -							 true, list, split_type);
> -	} else {
> +	if (is_anon)
>  		ret = __folio_freeze_split_unmap_anon(folio, new_order, split_at, true,
>  						      true, list, split_type);
> -	}
> -
> -	/*
> -	 * Drop the mapping while the inode is still pinned. @folio stays
> -	 * locked and present in the page cache until the loop below, so
> -	 * eviction cannot free the inode yet; @lock_at is not enough, it may
> -	 * be a tail beyond EOF that the split already dropped from the page
> -	 * cache. Nothing past this point may touch the inode or the mapping.
> -	 */
> -	if (mapping) {
> -		i_mmap_unlock_read(mapping);
> -		mapping = NULL;
> -	}
> +	else
> +		ret = __folio_freeze_split_unmap_file(folio, new_order, split_at,
> +						      true, list, split_type);
>  
>  	/*
>  	 * Unlock all after-split folios except the one containing
> @@ -4310,19 +4303,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  		free_folio_and_swap_cache(new_folio);
>  	}
>  
> -out_unlock:
> -	if (anon_vma) {
> -		anon_vma_unlock_write(anon_vma);
> -		put_anon_vma(anon_vma);
> -	}
> -	if (mapping)
> -		i_mmap_unlock_read(mapping);
> -out:
>  	/* restore to caller's old_memcg */
>  	set_active_memcg(old_memcg);
>  	mem_cgroup_put(memcg);
>  out_no_memcg:
> -	xas_destroy(&xas);
>  	if (is_pmd_order(old_order))
>  		count_vm_event(!ret ? THP_SPLIT_PAGE : THP_SPLIT_PAGE_FAILED);
>  	count_mthp_stat(old_order, !ret ? MTHP_STAT_SPLIT : MTHP_STAT_SPLIT_FAILED);
> @@ -4358,9 +4342,6 @@ int folio_split_unmapped(struct folio *folio, unsigned int new_order)
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_anon(folio), folio);
>  
> -	if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1)
> -		return -EAGAIN;
> -
>  	return __folio_freeze_split_unmap_anon(folio, new_order, &folio->page, false,
>  					       false, NULL, SPLIT_TYPE_UNIFORM);
>  }

__folio_split() looks much cleaner. Thanks.

Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>



-- 
Best Regards,
Yan, Zi