Re: [PATCH v3 2/3] mm/swap: refault on swap-in, not in the swap cache allocator

[Usama Arif]

On Tue, 25 Aug 2026 19:24:52 +0200 Alexandre Ghiti <alex@xxxxxxxx> wrote:

> The swap cache allocator evaluates the refault of every folio it
> allocates.  zswap writeback also allocates through it: the shrinker puts
> a buffer folio in the swap cache to write the compressed data out, and
> that allocation is then counted as an anon refault (and, if the eviction
> looks recent, as an activation) even though nothing faulted the page
> back in.  On a workload that writes back continuously this inflates
> workingset_refault_anon and workingset_activate_anon substantially.
> 
> Move the refault evaluation out of the allocator and into the two
> swap-in callers, which read the slot's shadow before the allocation
> overwrites it.  zswap writeback keeps allocating the buffer, but no
> longer reports a refault for it.
> 
> The refault is evaluated before folio_add_lru(), as it was before this
> patch, so workingset_refault() still sets PG_workingset/PG_active while
> the folio is off the LRU: folio_add_lru() consumes both when it picks
> the LRU list, and under MGLRU when it picks the generation.
> 
> Fixes: aae466b0052e ("mm/swap: implement workingset detection for anonymous LRU")
> Signed-off-by: Nhat Pham <nphamcs@xxxxxxxxx>
> Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> ---
>  mm/swap_state.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index bf8ff2d2dbf1..8046fea015c9 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -483,8 +483,6 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
>  
>  	/* memsw uncharges swap when folio is added to swap cache */
>  	memcg1_swapin(folio);
> -	if (shadow)
> -		workingset_refault(folio, shadow);
>  
>  	node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
>  	lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
> @@ -646,17 +644,26 @@ static struct folio *swap_cache_read_folio(struct swap_io_ctx *ctx,
>  		pgoff_t ilx, bool readahead)
>  {
>  	struct folio *folio;
> +	void *shadow = NULL;
>  
>  	do {
>  		folio = swap_cache_get_folio(entry);
>  		if (folio)
>  			return folio;
> +		/*
> +		 * Capture the slot's shadow before the allocation overwrites it,
> +		 * so a fresh swap-in can be evaluated as a refault below.
> +		 */
> +		shadow = swap_cache_get_shadow(entry);
>  		folio = __swap_cache_alloc_folio(entry, gfp, BIT(0), NULL, mpol, ilx);

Can this refault against a shadow from a different cache generation?
Another swap-in can replace this value, reclaim its folio, and install
a newer shadow while this caller is allocating.  This caller can then
win the locked insertion but still use the older snapshot.

What I think should be done here is have an optional shadow output to
__swap_cache_alloc_folio() and return the value captured by the successful
__swap_cache_add_check() under ci->lock and se that output in the swap-in
path?

>  	} while (PTR_ERR(folio) == -EEXIST);
>  
>  	if (IS_ERR_OR_NULL(folio))
>  		return NULL;
>  
> +	if (shadow)
> +		workingset_refault(folio, shadow);
> +
>  	folio_add_lru(folio);
>  	swap_read_folio(ctx, folio);
>  	if (readahead) {
> @@ -688,17 +695,26 @@ struct folio *swapin_sync(swp_entry_t entry, gfp_t gfp, unsigned long orders,
>  {
>  	struct swap_io_ctx ctx = {};
>  	struct folio *folio;
> +	void *shadow = NULL;
>  
>  	do {
>  		folio = swap_cache_get_folio(entry);
>  		if (folio)
>  			return folio;
> +		/*
> +		 * Capture the slot's shadow before the allocation overwrites it,
> +		 * so a fresh swap-in can be evaluated as a refault below.
> +		 */
> +		shadow = swap_cache_get_shadow(entry);
>  		folio = __swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx);
>  	} while (PTR_ERR(folio) == -EEXIST);
>  
>  	if (IS_ERR(folio))
>  		return folio;
>  
> +	if (shadow)
> +		workingset_refault(folio, shadow);
> +
>  	folio_add_lru(folio);
>  	swap_read_folio(&ctx, folio);
>  	swap_read_submit(&ctx);
> -- 
> 2.53.0-Meta
> 
>