Re: [PATCH v2 3/3] mm/zswap: preserve the workingset shadow across writeback

[David Hildenbrand (Arm)]

On 8/21/26 11:35, Alexandre Ghiti wrote:
> When zswap writes an entry back, allocating the buffer folio in the swap
> cache overwrites the swap slot's workingset shadow. It is lost and later
> re-minted inaccurately, which corrupts anon refault/workingset accounting
> for the written-back data.
> 
> Preserve it within zswap itself: at writeback, park the shadow in the
> zswap tree in place of the freed entry, so it outlives the writeback
> buffer folio. The buffer then meets one of three fates, and the parked
> shadow is handled at whichever happens first:
> 
>  - swap-in: do_swap_page()/shmem_swapin_folio() find the buffer in the swap
>    cache, take the shadow and feed it to workingset_refault() to account
>    the refault of the now-resident folio, then drop it.
>  - reclaim: __remove_mapping() skips minting a fresh shadow for the buffer
>    (minting would double-count the eviction) and
>    __swap_cache_do_del_folio() restores the parked shadow into the slot, so
>    a later disk swap-in still refaults against the original eviction.
>  - slot free (e.g. process exit): the shadow is not needed and is cleared.
> 
> The last two both go through __swap_cache_do_del_folio(), the single point
> every swap-cache removal passes through, which is why the shadow is read
> and cleared there.
> 
> Fixes: aae466b0052e ("mm/swap: implement workingset detection for anonymous LRU")
> Signed-off-by: Nhat Pham <nphamcs@xxxxxxxxx>
> Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> ---
>  include/linux/swap.h  |  2 +-
>  include/linux/zswap.h | 12 ++++++
>  mm/filemap.c          |  2 +-
>  mm/memory.c           |  5 +++
>  mm/shmem.c            |  5 +++
>  mm/swap_state.c       | 14 ++++++-
>  mm/vmscan.c           |  4 +-
>  mm/workingset.c       | 16 ++++++--
>  mm/zswap.c            | 91 ++++++++++++++++++++++++++++++++++++++++++-
>  9 files changed, 142 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 6d72778e6cc3..ef9b1612ac12 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -298,7 +298,7 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset,
>  				bool flush);
>  void workingset_age_nonresident(struct lruvec *lruvec, unsigned long nr_pages);
>  void *workingset_eviction(struct folio *folio, struct mem_cgroup *target_memcg);
> -void workingset_refault(struct folio *folio, void *shadow);
> +void workingset_refault(struct folio *folio, void *shadow, bool lru_managed);

Passing booleans makes calling code hard to read. It's preferable to either use

(a) Dedicated flags; only makes sense if there are really more flags expected

(b) Having helpfer functions that spell it out for calling code

workingset_refault_lru_managed
workingset_refault

workingset_refault_lru
workingset_refault_nonlru

Or something like that, which makes calling code easier to get.

-- 
Cheers,

David