Re: [PATCH v4 12/17] mm/huge_memory: move anon_vma handling into the anon split helper

[Kiryl Shutsemau]

On Tue, Sep 08, 2026 at 02:12:16AM +0800, Kairui Song via B4 Relay wrote:
> +	/*
> +	 * Unmap/remap needs the anon_vma. The caller does not necessarily
> +	 * hold an mmap_lock that would prevent the anon_vma from
> +	 * disappearing, so we first take a reference and lock it.
> +	 *
> +	 * An unmapped folio needs none of this: folio_get_anon_vma() and
> +	 * folio_lock_anon_vma_read() both bail out on !folio_mapped()
> +	 * before taking the lock, and folio_ref_freeze() below still
> +	 * rejects a folio that picked up a reference meanwhile. Note
> +	 * a swapped-out THP counts as unmapped here as swap PTEs do
> +	 * not contribute mapcount, and they are splittable.
> +	 */
>  	if (folio_mapped(folio)) {
> -		need_remap = true;
> +		anon_vma = folio_get_anon_vma(folio);
> +		if (!anon_vma)
> +			return -EBUSY;
> +		anon_vma_lock_write(anon_vma);
>  		ret = unmap_folio(folio);
>  		if (ret)
> -			return ret;
> +			goto out_unlock;
>  	}

Hm. Nothing serializes folio_mapped() here. I believe it is fine, but the
reasoning deserves a comment, since it is what the whole change rests on.
Something along the lines of:

       * folio_mapped() is not stable here, but it can only change in
       * one direction while the folio is locked. The mapcount can drop
       * to zero at any time, zap_pte_range() takes no folio lock. It
       * cannot go up: swapin, migration and uffd move all lock the folio
       * before mapping it, and fork only copies PTEs that already exist.
       *
       * So if we see the folio mapped, the worst case is an empty rmap
       * walk. If we see it unmapped, it stays unmapped. Anything else
       * needs a reference first and folio_ref_freeze() below catches it.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov