Re: [PATCH v6 04/12] mm, swap: support physical swap as a vswap backend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: try_to_unuse() dirties unmapped vswap folios rejected by pageout()
- [High] mm: try_to_unuse() leaks vswap folios and slots when swap count is zero
- [High] mm: large vswap folios declined by zswap fail swapout without splitting
--

commit f1735375be4432e3cb8facdcc6b588e080aeb34c
Author: Nhat Pham <nphamcs@gmail.com>

mm, swap: support physical swap as a vswap backend

This commit adds physical swapfile support as a backend for virtual swap
entries when zswap declines a folio during swap_writeout(), tracking the
reverse mapping via vswap-tagged swap_table entries on the physical cluster.
During swapoff, try_to_unuse() reads vswap-backed physical slots into the
swap cache and releases their physical backing slots before walking process
and shmem mappings.

> diff --git a/mm/page_io.c b/mm/page_io.c
> index 61bb5c6e97595..ed8c037e4aa08 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -209,6 +209,7 @@ static void swap_zeromap_folio_clear(struct folio *folio)
>   */
>  int swap_writeout(struct swap_io_ctx *ctx, struct folio *folio)
>  {
> +	swp_entry_t phys = folio->swap;
>  	int ret = 0;
>  
>  	if (folio_free_swap(folio))

[ ... ]

> @@ -257,13 +264,21 @@ int swap_writeout(struct swap_io_ctx *ctx, struct folio *folio)
>  	}
>  	rcu_read_unlock();
>  
> -	/* A vswap folio has no physical slot to write to. */
>  	if (is_vswap_entry(folio->swap)) {
> +		phys = folio_realloc_swap(folio);
> +		if (!phys.val) {
> +			folio_mark_dirty(folio);
> +			return AOP_WRITEPAGE_ACTIVATE;
> +		}
> +	}

[Severity: High]
What happens here when a large folio (order > 0) with a virtual swap entry
is declined by zswap_store() and the underlying physical swap cannot satisfy
a high-order allocation?

During reclaim in shrink_folio_list() or shmem_writeout(), folio_alloc_swap()
allocates from vswap_si via vswap_alloc() -> cluster_alloc_swap_entry():

mm/swapfile.c:cluster_alloc_swap_entry() {
	if (order && !(si->flags & SWP_BLKDEV) && !swap_is_vswap(si))
		return 0;
}

Because swap_is_vswap(vswap_si) is true, vswap_alloc() succeeds for order > 0
folios even when the physical swap devices are regular swapfiles without
SWP_BLKDEV or have fragmented high-order clusters. As a result,
folio_alloc_swap() returns 0 instead of -E2BIG, bypassing the
split_folio_to_list() fallback in both shrink_folio_list() and
shmem_writeout().

Later in swap_writeout(), if zswap_store() declines the large folio,
folio_realloc_swap() -> folio_alloc_phys_swap() attempts to allocate a
contiguous physical swap range of folio_order(folio), which always fails on
non-SWP_BLKDEV swapfiles and fails when high-order physical clusters are
unavailable. swap_writeout() then marks the folio dirty and returns
AOP_WRITEPAGE_ACTIVATE without splitting it.

On subsequent reclaim passes, an anonymous large folio is already in the swap
cache with PTEs referencing its vswap slots, so shrink_folio_list() skips the
!folio_test_swapcache(folio) block where split_folio_to_list() is called:

mm/vmscan.c:shrink_folio_list() {
	if (folio_test_anon(folio) && folio_test_swapbacked(folio) &&
			!folio_test_swapcache(folio)) {
		...
		ret = folio_alloc_swap(folio);
		if (ret) {
			...
			if (split_folio_to_list(folio, folio_list))
				goto activate_locked;
}

For a large shmem folio, shmem_writeout() removes the unsplit folio from the
swap cache on AOP_WRITEPAGE_ACTIVATE without reaching its -E2BIG split path,
repeating the same vswap_alloc() success and folio_realloc_swap() failure on
every pass.

Can this cause large vswap folios declined by zswap to never split or swap
out?

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 69fac2d905a4f..be66ac79313ce 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c

[ ... ]

> @@ -3087,19 +3287,95 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
>  
>  static int try_to_unuse(unsigned int type)
>  {
> +	struct mempolicy *mpol = get_task_policy(current);
>  	struct mm_struct *prev_mm;
>  	struct mm_struct *mm;
>  	struct list_head *p;
>  	int retval = 0;
>  	struct swap_info_struct *si = swap_info[type];
>  	struct folio *folio;
> -	swp_entry_t entry;
> -	unsigned int i;
> +	struct swap_io_ctx ctx;
> +	swp_entry_t entry, vswap_entry, phys;
> +	unsigned long swp_tb;
> +	unsigned int i, j;
>  
>  	if (!swap_usage_in_pages(si))
>  		goto success;
>  
>  retry:
> +	/*
> +	 * Free vswap-backing slots (Vswap-tagged) first. Walk physical
> +	 * clusters, read the vswap entry from the rmap, ensure the data
> +	 * is in the swap cache, and transition PHYS to FOLIO. Freeing the
> +	 * physical backing is enough, so no page table walk is needed.
> +	 */
> +	i = 0;
> +	while (vswap_is_enabled() &&
> +	       swap_usage_in_pages(si) &&
> +	       !signal_pending(current) &&
> +	       (i = find_next_to_unuse(si, i)) != 0) {
> +		swp_tb = swap_table_lookup(swp_entry(si->type, i));
> +		if (!swp_tb_is_vswap(swp_tb))
> +			continue;
> +
> +		vswap_entry = swp_tb_vswap_to_swp_entry(swp_tb);
> +
> +		folio = swap_cache_get_folio(vswap_entry);
> +		if (!folio) {
> +			folio = __swap_cache_alloc_folio(vswap_entry,
> +							 GFP_HIGHUSER_MOVABLE,
> +							 BIT(0), NULL, mpol,
> +							 NO_INTERLEAVE_INDEX);
> +			if (IS_ERR(folio)) {
> +				if (PTR_ERR(folio) == -ENOMEM)
> +					return -ENOMEM;
> +				continue;
> +			}
> +			folio_add_lru(folio);
> +			ctx = (struct swap_io_ctx){};
> +			swap_read_folio(&ctx, folio);
> +			swap_read_submit(&ctx);
> +		}
> +		folio_lock(folio);
> +
> +		if (!folio_matches_swap_entry(folio, vswap_entry)) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			continue;
> +		}
> +
> +		/*
> +		 * Re-validate under folio lock: rmap holds folio->swap + j
> +		 * for some j in [0, nr_pages). Check folio->swap still maps
> +		 * to the contiguous physical run that includes our slot i.
> +		 */
> +		j = vswap_entry.val - folio->swap.val;
> +		phys = vswap_to_phys(folio->swap);
> +		if (!phys.val || swp_type(phys) != type ||
> +		    swp_offset(phys) + j != i) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			continue;
> +		}
> +
> +		folio_wait_writeback(folio);
> +		folio_release_vswap_backing(folio);
> +		/*
> +		 * Drop a folio whose read failed rather than dirtying
> +		 * uninitialised memory; the next fault finds no backing and
> +		 * gets SIGBUS.
> +		 */
> +		if (unlikely(!folio_test_uptodate(folio)))
> +			swap_cache_del_folio(folio);
> +		else
> +			folio_mark_dirty(folio);

[Severity: High]
Can pageout() ever write out the unmapped swap-cache folios allocated and
dirtied by this pre-pass?

When __swap_cache_alloc_folio() -> __swap_cache_alloc() allocates a fresh
swap-cache folio here, it sets PG_swapbacked and PG_swapcache, but leaves
folio->mapping as NULL because try_to_unuse() does not map the folio into a
VMA via folio_add_new_anon_rmap(). Shmem folios in the swap cache also have
folio->mapping set to NULL. As a result, folio_test_anon(folio) is false.

When shrink_folio_list() later scans one of these dirty swap-cache folios on
 the LRU, folio_mapping(folio) returns &swap_space and shrink_folio_list()
calls pageout():

mm/vmscan.c:pageout() {
	if (folio_ref_count(folio) != 1 + folio_nr_pages(folio) || !mapping)
		return PAGE_KEEP;
	if (!shmem_mapping(mapping) && !folio_test_anon(folio))
		return PAGE_ACTIVATE;
	if (!folio_clear_dirty_for_io(folio))
		return PAGE_CLEAN;
}

Because mapping is &swap_space (!shmem_mapping(mapping)) and
folio_test_anon(folio) is false, pageout() unconditionally returns
PAGE_ACTIVATE before reaching swap_writeout(). Wouldn't this prevent reclaim
from ever writing these dirtied vswap folios out to zswap or another swap
device until they are faulted in?

> +		folio_unlock(folio);
> +		folio_put(folio);
> +	}

[Severity: High]
Does this pre-pass leak the folio, its vswap slot, and its memcg swap charge
if the vswap entry's swap count is 0 (or drops to 0 concurrently)?

There are two ways the vswap entry can have swap_count == 0 when reaching
folio_release_vswap_backing(folio):

1. While try_to_unuse() holds the folio lock (during swap_read_folio() IO or
   before folio_unlock()), the owning process exits or zaps the swap PTE via
   zap_nonpresent_ptes() -> free_swap_and_cache_nr() ->
   swap_put_entries_cluster(). That path decrements the vswap slot's swap
   count to 0 and calls __try_to_reclaim_swap(), which fails folio_trylock()
   and bails out expecting the folio lock holder to clean up.

2. A prior read fault in do_swap_page() mapped the folio, decremented the
   vswap entry's swap count to 0 via folio_put_swap(), and left the folio in
   the swap cache because should_try_to_free_swap() saw
   folio_phys_swap_backed(folio) == true.

After folio_release_vswap_backing(folio) frees the physical slot,
folio_phys_swap_backed(folio) becomes false, and try_to_unuse() marks the
folio dirty without calling folio_free_swap(folio) (unlike the post-pass at
the end of try_to_unuse()).

In the first case, the unlocked folio is unmapped, has folio->mapping == NULL
(!folio_test_anon(folio)), has swap_count == 0, and is no longer physically
backed. pageout() rejects it with PAGE_ACTIVATE before reaching
swap_writeout() -> folio_free_swap(), and the activate_locked path in
shrink_folio_list() also skips folio_free_swap() because
folio_phys_swap_backed(folio) is now false:

mm/vmscan.c:shrink_folio_list() {
activate_locked:
	/* Not a candidate for swapping, so reclaim swap space. */
	if (folio_test_swapcache(folio) &&
	    ((mem_cgroup_swap_full(folio) && folio_phys_swap_backed(folio)) ||
	     folio_test_mlocked(folio)))
		folio_free_swap(folio);
}

Should try_to_unuse() check or call folio_free_swap(folio) when releasing the
vswap backing here?

>  
> +	if (!swap_usage_in_pages(si))
> +		goto success;
> +
>  	retval = shmem_unuse(type);

[ ... ]

完美 (Wait: oops, don't write any extra text after the report! Let's make sure the final output has only the report and ends with a blank line.)
Let's review the whole text and output it cleanly.commit f1735375be4432e3cb8facdcc6b588e080aeb34c
Author: Nhat Pham <nphamcs@gmail.com>

mm, swap: support physical swap as a vswap backend

This commit adds physical swapfile support as a backend for virtual swap
entries when zswap declines a folio during swap_writeout(), tracking the
reverse mapping via vswap-tagged swap_table entries on the physical cluster.
During swapoff, try_to_unuse() reads vswap-backed physical slots into the
swap cache and releases their physical backing slots before walking process
and shmem mappings.

> diff --git a/mm/page_io.c b/mm/page_io.c
> index 61bb5c6e97595..ed8c037e4aa08 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -209,6 +209,7 @@ static void swap_zeromap_folio_clear(struct folio *folio)
>   */
>  int swap_writeout(struct swap_io_ctx *ctx, struct folio *folio)
>  {
> +	swp_entry_t phys = folio->swap;
>  	int ret = 0;
>  
>  	if (folio_free_swap(folio))

[ ... ]

> @@ -257,13 +264,21 @@ int swap_writeout(struct swap_io_ctx *ctx, struct folio *folio)
>  	}
>  	rcu_read_unlock();
>  
> -	/* A vswap folio has no physical slot to write to. */
>  	if (is_vswap_entry(folio->swap)) {
> +		phys = folio_realloc_swap(folio);
> +		if (!phys.val) {
> +			folio_mark_dirty(folio);
> +			return AOP_WRITEPAGE_ACTIVATE;
> +		}
> +	}

[Severity: High]
What happens here when a large folio (order > 0) with a virtual swap entry
is declined by zswap_store() and the underlying physical swap cannot satisfy
a high-order allocation?

During reclaim in shrink_folio_list() or shmem_writeout(), folio_alloc_swap()
allocates from vswap_si via vswap_alloc() -> cluster_alloc_swap_entry():

mm/swapfile.c:cluster_alloc_swap_entry() {
	if (order && !(si->flags & SWP_BLKDEV) && !swap_is_vswap(si))
		return 0;
}

Because swap_is_vswap(vswap_si) is true, vswap_alloc() succeeds for order > 0
folios even when the physical swap devices are regular swapfiles without
SWP_BLKDEV or have fragmented high-order clusters. As a result,
folio_alloc_swap() returns 0 instead of -E2BIG, bypassing the
split_folio_to_list() fallback in both shrink_folio_list() and
shmem_writeout().

Later in swap_writeout(), if zswap_store() declines the large folio,
folio_realloc_swap() -> folio_alloc_phys_swap() attempts to allocate a
contiguous physical swap range of folio_order(folio), which always fails on
non-SWP_BLKDEV swapfiles and fails when high-order physical clusters are
unavailable. swap_writeout() then marks the folio dirty and returns
AOP_WRITEPAGE_ACTIVATE without splitting it.

On subsequent reclaim passes, an anonymous large folio is already in the swap
cache with PTEs referencing its vswap slots, so shrink_folio_list() skips the
!folio_test_swapcache(folio) block where split_folio_to_list() is called:

mm/vmscan.c:shrink_folio_list() {
	if (folio_test_anon(folio) && folio_test_swapbacked(folio) &&
			!folio_test_swapcache(folio)) {
		...
		ret = folio_alloc_swap(folio);
		if (ret) {
			...
			if (split_folio_to_list(folio, folio_list))
				goto activate_locked;
}

For a large shmem folio, shmem_writeout() removes the unsplit folio from the
swap cache on AOP_WRITEPAGE_ACTIVATE without reaching its -E2BIG split path,
repeating the same vswap_alloc() success and folio_realloc_swap() failure on
every pass.

Can this cause large vswap folios declined by zswap to never split or swap
out?

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 69fac2d905a4f..be66ac79313ce 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c

[ ... ]

> @@ -3087,19 +3287,95 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
>  
>  static int try_to_unuse(unsigned int type)
>  {
> +	struct mempolicy *mpol = get_task_policy(current);
>  	struct mm_struct *prev_mm;
>  	struct mm_struct *mm;
>  	struct list_head *p;
>  	int retval = 0;
>  	struct swap_info_struct *si = swap_info[type];
>  	struct folio *folio;
> -	swp_entry_t entry;
> -	unsigned int i;
> +	struct swap_io_ctx ctx;
> +	swp_entry_t entry, vswap_entry, phys;
> +	unsigned long swp_tb;
> +	unsigned int i, j;
>  
>  	if (!swap_usage_in_pages(si))
>  		goto success;
>  
>  retry:
> +	/*
> +	 * Free vswap-backing slots (Vswap-tagged) first. Walk physical
> +	 * clusters, read the vswap entry from the rmap, ensure the data
> +	 * is in the swap cache, and transition PHYS to FOLIO. Freeing the
> +	 * physical backing is enough, so no page table walk is needed.
> +	 */
> +	i = 0;
> +	while (vswap_is_enabled() &&
> +	       swap_usage_in_pages(si) &&
> +	       !signal_pending(current) &&
> +	       (i = find_next_to_unuse(si, i)) != 0) {
> +		swp_tb = swap_table_lookup(swp_entry(si->type, i));
> +		if (!swp_tb_is_vswap(swp_tb))
> +			continue;
> +
> +		vswap_entry = swp_tb_vswap_to_swp_entry(swp_tb);
> +
> +		folio = swap_cache_get_folio(vswap_entry);
> +		if (!folio) {
> +			folio = __swap_cache_alloc_folio(vswap_entry,
> +							 GFP_HIGHUSER_MOVABLE,
> +							 BIT(0), NULL, mpol,
> +							 NO_INTERLEAVE_INDEX);
> +			if (IS_ERR(folio)) {
> +				if (PTR_ERR(folio) == -ENOMEM)
> +					return -ENOMEM;
> +				continue;
> +			}
> +			folio_add_lru(folio);
> +			ctx = (struct swap_io_ctx){};
> +			swap_read_folio(&ctx, folio);
> +			swap_read_submit(&ctx);
> +		}
> +		folio_lock(folio);
> +
> +		if (!folio_matches_swap_entry(folio, vswap_entry)) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			continue;
> +		}
> +
> +		/*
> +		 * Re-validate under folio lock: rmap holds folio->swap + j
> +		 * for some j in [0, nr_pages). Check folio->swap still maps
> +		 * to the contiguous physical run that includes our slot i.
> +		 */
> +		j = vswap_entry.val - folio->swap.val;
> +		phys = vswap_to_phys(folio->swap);
> +		if (!phys.val || swp_type(phys) != type ||
> +		    swp_offset(phys) + j != i) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			continue;
> +		}
> +
> +		folio_wait_writeback(folio);
> +		folio_release_vswap_backing(folio);
> +		/*
> +		 * Drop a folio whose read failed rather than dirtying
> +		 * uninitialised memory; the next fault finds no backing and
> +		 * gets SIGBUS.
> +		 */
> +		if (unlikely(!folio_test_uptodate(folio)))
> +			swap_cache_del_folio(folio);
> +		else
> +			folio_mark_dirty(folio);

[Severity: High]
Can pageout() ever write out the unmapped swap-cache folios allocated and
dirtied by this pre-pass?

When __swap_cache_alloc_folio() -> __swap_cache_alloc() allocates a fresh
swap-cache folio here, it sets PG_swapbacked and PG_swapcache, but leaves
folio->mapping as NULL because try_to_unuse() does not map the folio into a
VMA via folio_add_new_anon_rmap(). Shmem folios in the swap cache also have
folio->mapping set to NULL. As a result, folio_test_anon(folio) is false.

When shrink_folio_list() later scans one of these dirty swap-cache folios on
the LRU, folio_mapping(folio) returns &swap_space and shrink_folio_list()
calls pageout():

mm/vmscan.c:pageout() {
	if (folio_ref_count(folio) != 1 + folio_nr_pages(folio) || !mapping)
		return PAGE_KEEP;
	if (!shmem_mapping(mapping) && !folio_test_anon(folio))
		return PAGE_ACTIVATE;
	if (!folio_clear_dirty_for_io(folio))
		return PAGE_CLEAN;
}

Because mapping is &swap_space (!shmem_mapping(mapping)) and
folio_test_anon(folio) is false, pageout() unconditionally returns
PAGE_ACTIVATE before reaching swap_writeout(). Wouldn't this prevent reclaim
from ever writing these dirtied vswap folios out to zswap or another swap
device until they are faulted in?

> +		folio_unlock(folio);
> +		folio_put(folio);
> +	}

[Severity: High]
Does this pre-pass leak the folio, its vswap slot, and its memcg swap charge
if the vswap entry's swap count is 0 (or drops to 0 concurrently)?

There are two ways the vswap entry can have swap_count == 0 when reaching
folio_release_vswap_backing(folio):

1. While try_to_unuse() holds the folio lock (during swap_read_folio() IO or
   before folio_unlock()), the owning process exits or zaps the swap PTE via
   zap_nonpresent_ptes() -> free_swap_and_cache_nr() ->
   swap_put_entries_cluster(). That path decrements the vswap slot's swap
   count to 0 and calls __try_to_reclaim_swap(), which fails folio_trylock()
   and bails out expecting the folio lock holder to clean up.

2. A prior read fault in do_swap_page() mapped the folio, decremented the
   vswap entry's swap count to 0 via folio_put_swap(), and left the folio in
   the swap cache because should_try_to_free_swap() saw
   folio_phys_swap_backed(folio) == true.

After folio_release_vswap_backing(folio) frees the physical slot,
folio_phys_swap_backed(folio) becomes false, and try_to_unuse() marks the
folio dirty without calling folio_free_swap(folio) (unlike the post-pass at
the end of try_to_unuse()).

In the first case, the unlocked folio is unmapped, has folio->mapping == NULL
(!folio_test_anon(folio)), has swap_count == 0, and is no longer physically
backed. pageout() rejects it with PAGE_ACTIVATE before reaching
swap_writeout() -> folio_free_swap(), and the activate_locked path in
shrink_folio_list() also skips folio_free_swap() because
folio_phys_swap_backed(folio) is now false:

mm/vmscan.c:shrink_folio_list() {
activate_locked:
	/* Not a candidate for swapping, so reclaim swap space. */
	if (folio_test_swapcache(folio) &&
	    ((mem_cgroup_swap_full(folio) && folio_phys_swap_backed(folio)) ||
	     folio_test_mlocked(folio)))
		folio_free_swap(folio);
}

Should try_to_unuse() check or call folio_free_swap(folio) when releasing the
vswap backing here?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021060323.553433-1-nphamcs@gmail.com?part=4