The swap cache allocator adds the new folio to the LRU itself, which forces every caller to live with that ordering. The next patch needs to evaluate the refault of a swapped-in folio *before* it becomes visible to reclaim, because folio_add_lru() consumes PG_workingset/PG_active when it picks the LRU list (and, under MGLRU, the generation). So defer the LRU addition to the callers of __swap_cache_alloc_folio(), no functional change intended. Suggested-by: Kairui Song Signed-off-by: Alexandre Ghiti --- mm/swap.h | 6 +++--- mm/swap_state.c | 20 ++++++++++++-------- mm/zswap.c | 5 +++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/mm/swap.h b/mm/swap.h index 77d2d14eda42..fc44daae1de1 100644 --- a/mm/swap.h +++ b/mm/swap.h @@ -304,9 +304,9 @@ bool swap_cache_has_folio(swp_entry_t entry); struct folio *swap_cache_get_folio(swp_entry_t entry); void *swap_cache_get_shadow(swp_entry_t entry); void swap_cache_del_folio(struct folio *folio); -struct folio *swap_cache_alloc_folio(swp_entry_t target_entry, gfp_t gfp_mask, - unsigned long orders, struct vm_fault *vmf, - struct mempolicy *mpol, pgoff_t ilx); +struct folio *__swap_cache_alloc_folio(swp_entry_t target_entry, gfp_t gfp_mask, + unsigned long orders, struct vm_fault *vmf, + struct mempolicy *mpol, pgoff_t ilx); /* Below helpers require the caller to lock and pass in the swap cluster. */ void __swap_cache_add_folio(struct swap_cluster_info *ci, struct folio *folio, swp_entry_t entry); diff --git a/mm/swap_state.c b/mm/swap_state.c index 9c3a5cf99778..90638a8d7232 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -483,13 +483,11 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci, node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages); lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages); - /* Caller will initiate read into locked new_folio */ - folio_add_lru(folio); return folio; } /** - * swap_cache_alloc_folio - Allocate folio for swapped out slot in swap cache. + * __swap_cache_alloc_folio - Allocate folio for swapped out slot in swap cache. * @targ_entry: swap entry indicating the target slot * @gfp: memory allocation flags * @orders: allocation orders, must be non zero @@ -501,13 +499,17 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci, * doing IO (e.g. swap in or zswap writeback). The swap slot indicated by * @targ_entry must have a non-zero swap count (swapped out). * + * The returned folio is locked and is NOT on the LRU. The caller must either + * add it to the LRU with folio_add_lru() so page reclaim can find it, or free + * it directly once done; a folio left off the LRU is unreclaimable and leaks. + * * Context: Caller must protect the swap device with reference count or locks. * Return: Returns the folio if allocation succeeded and folio is in the swap * cache. Returns error code if failed due to race, OOM or invalid arguments. */ -struct folio *swap_cache_alloc_folio(swp_entry_t targ_entry, gfp_t gfp, - unsigned long orders, struct vm_fault *vmf, - struct mempolicy *mpol, pgoff_t ilx) +struct folio *__swap_cache_alloc_folio(swp_entry_t targ_entry, gfp_t gfp, + unsigned long orders, struct vm_fault *vmf, + struct mempolicy *mpol, pgoff_t ilx) { int order, err; struct folio *ret; @@ -643,12 +645,13 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp, folio = swap_cache_get_folio(entry); if (folio) return folio; - folio = swap_cache_alloc_folio(entry, gfp, BIT(0), NULL, mpol, ilx); + folio = __swap_cache_alloc_folio(entry, gfp, BIT(0), NULL, mpol, ilx); } while (PTR_ERR(folio) == -EEXIST); if (IS_ERR_OR_NULL(folio)) return NULL; + folio_add_lru(folio); swap_read_folio(folio, plug); if (readahead) { folio_set_readahead(folio); @@ -683,12 +686,13 @@ struct folio *swapin_sync(swp_entry_t entry, gfp_t gfp, unsigned long orders, folio = swap_cache_get_folio(entry); if (folio) return folio; - folio = swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx); + folio = __swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx); } while (PTR_ERR(folio) == -EEXIST); if (IS_ERR(folio)) return folio; + folio_add_lru(folio); swap_read_folio(folio, NULL); return folio; } diff --git a/mm/zswap.c b/mm/zswap.c index 761cd699e0a3..8163e6c5f76c 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1000,8 +1000,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry, return -EEXIST; mpol = get_task_policy(current); - folio = swap_cache_alloc_folio(swpentry, GFP_KERNEL, BIT(0), NULL, mpol, - NO_INTERLEAVE_INDEX); + folio = __swap_cache_alloc_folio(swpentry, GFP_KERNEL, BIT(0), NULL, mpol, + NO_INTERLEAVE_INDEX); put_swap_device(si); /* @@ -1013,6 +1013,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry, */ if (IS_ERR(folio)) return PTR_ERR(folio); + folio_add_lru(folio); /* * folio is locked, and the swapcache is now secured against -- 2.53.0-Meta 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 Signed-off-by: Alexandre Ghiti --- 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 90638a8d7232..4e15d50aece6 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -477,8 +477,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); @@ -640,17 +638,26 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp, struct swap_iocb **plug, 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); } 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(folio, plug); if (readahead) { @@ -681,17 +688,26 @@ struct folio *swapin_sync(swp_entry_t entry, gfp_t gfp, unsigned long orders, struct vm_fault *vmf, struct mempolicy *mpol, pgoff_t ilx) { 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(folio, NULL); return folio; -- 2.53.0-Meta 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 Signed-off-by: Alexandre Ghiti --- 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); void workingset_activation(struct folio *folio); /* linux/mm/page_alloc.c */ diff --git a/include/linux/zswap.h b/include/linux/zswap.h index 30c193a1207e..dfb7153236f5 100644 --- a/include/linux/zswap.h +++ b/include/linux/zswap.h @@ -35,6 +35,8 @@ void zswap_lruvec_state_init(struct lruvec *lruvec); void zswap_folio_swapin(struct folio *folio); bool zswap_is_enabled(void); bool zswap_never_enabled(void); +bool zswap_folio_is_writeback_buffer(struct folio *folio); +void *zswap_lookup_and_clear_shadows(struct folio *folio); #else struct zswap_lruvec_state {}; @@ -69,6 +71,16 @@ static inline bool zswap_never_enabled(void) return true; } +static inline bool zswap_folio_is_writeback_buffer(struct folio *folio) +{ + return false; +} + +static inline void *zswap_lookup_and_clear_shadows(struct folio *folio) +{ + return NULL; +} + #endif #endif /* _LINUX_ZSWAP_H */ diff --git a/mm/filemap.c b/mm/filemap.c index dc3a0e960b9f..c1c9cfc319e3 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -978,7 +978,7 @@ int filemap_add_folio(struct address_space *mapping, struct folio *folio, */ WARN_ON_ONCE(folio_test_active(folio)); if (!(gfp & __GFP_WRITE) && shadow) - workingset_refault(folio, shadow); + workingset_refault(folio, shadow, false); folio_add_lru(folio); if (kernel_file) mod_node_page_state(folio_pgdat(folio), diff --git a/mm/memory.c b/mm/memory.c index 56be920c56d7..67d1073365a9 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4759,6 +4759,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) unsigned long page_idx; unsigned long address; pte_t *ptep; + void *shadow; if (!pte_unmap_same(vmf)) goto out; @@ -4880,6 +4881,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) goto out_page; } + shadow = zswap_lookup_and_clear_shadows(folio); + if (shadow) + workingset_refault(folio, shadow, true); + /* * KSM sometimes has to copy on read faults, for example, if * folio->index of non-ksm folios would be nonlinear inside the diff --git a/mm/shmem.c b/mm/shmem.c index 56c23a7b15c7..ce4a8cf2298a 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -2242,6 +2242,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index, struct folio *folio = NULL; int error, nr_pages, order; pgoff_t offset; + void *shadow; VM_BUG_ON(!*foliop || !xa_is_value(*foliop)); index_entry = radix_to_swp_entry(*foliop); @@ -2353,6 +2354,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index, */ arch_swap_restore(folio_swap(swap, folio), folio); + shadow = zswap_lookup_and_clear_shadows(folio); + if (shadow) + workingset_refault(folio, shadow, true); + if (shmem_should_replace_folio(folio, gfp)) { error = shmem_replace_folio(&folio, gfp, info, index, vma); if (error) diff --git a/mm/swap_state.c b/mm/swap_state.c index 4e15d50aece6..66f7efb12d2b 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "internal.h" #include "swap_table.h" #include "swap.h" @@ -257,12 +258,21 @@ static void __swap_cache_do_del_folio(struct swap_cluster_info *ci, unsigned int ci_start, ci_off, ci_end; bool folio_swapped = false, need_free = false; unsigned long nr_pages = folio_nr_pages(folio); + void *shadow_parked; VM_WARN_ON_ONCE(__swap_entry_to_cluster(entry) != ci); VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio); VM_WARN_ON_ONCE_FOLIO(!folio_test_swapcache(folio), folio); VM_WARN_ON_ONCE_FOLIO(folio_test_writeback(folio), folio); + /* + * A zswap writeback buffer parked the slot's original shadow in the + * zswap tree: restore it into the slot for later swap-in. + */ + shadow_parked = zswap_lookup_and_clear_shadows(folio); + if (shadow_parked) + shadow = shadow_parked; + si = __swap_entry_to_info(entry); ci_start = swp_cluster_offset(entry); ci_end = ci_start + nr_pages; @@ -656,7 +666,7 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp, return NULL; if (shadow) - workingset_refault(folio, shadow); + workingset_refault(folio, shadow, false); folio_add_lru(folio); swap_read_folio(folio, plug); @@ -706,7 +716,7 @@ struct folio *swapin_sync(swp_entry_t entry, gfp_t gfp, unsigned long orders, return folio; if (shadow) - workingset_refault(folio, shadow); + workingset_refault(folio, shadow, false); folio_add_lru(folio); swap_read_folio(folio, NULL); diff --git a/mm/vmscan.c b/mm/vmscan.c index 3f3ff25e561a..2c57e88421b0 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -735,7 +736,8 @@ static int __remove_mapping(struct address_space *mapping, struct folio *folio, if (folio_test_swapcache(folio)) { swp_entry_t swap = folio->swap; - if (reclaimed && !mapping_exiting(mapping)) + if (reclaimed && !mapping_exiting(mapping) && + !zswap_folio_is_writeback_buffer(folio)) shadow = workingset_eviction(folio, target_memcg); __memcg1_swapout(folio, ci); __swap_cache_del_folio(ci, folio, swap, shadow); diff --git a/mm/workingset.c b/mm/workingset.c index f351798e723a..961c86862bf4 100644 --- a/mm/workingset.c +++ b/mm/workingset.c @@ -538,14 +538,15 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset, /** * workingset_refault - Evaluate the refault of a previously evicted folio. - * @folio: The freshly allocated replacement folio. + * @folio: The folio the eviction is refaulted into. * @shadow: Shadow entry of the evicted folio. + * @lru_managed: Whether @folio has already been added to the LRU. * * Calculates and evaluates the refault distance of the previously * evicted folio in the context of the node and the memcg whose memory * pressure caused the eviction. */ -void workingset_refault(struct folio *folio, void *shadow) +void workingset_refault(struct folio *folio, void *shadow, bool lru_managed) { bool file = folio_is_file_lru(folio); struct mem_cgroup *memcg; @@ -577,7 +578,16 @@ void workingset_refault(struct folio *folio, void *shadow) if (!workingset_test_recent(shadow, file, &workingset, true)) goto out; - folio_set_active(folio); + /* + * An LRU-managed folio may sit in a per-CPU batch, which cannot be + * determined here: setting the flag would race the drain and leave it + * disagreeing with the list. folio_activate() is safe, but misses the + * activation for such a folio. + */ + if (lru_managed) + folio_activate(folio); + else + folio_set_active(folio); workingset_age_nonresident(lruvec, nr); mod_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + file, nr); diff --git a/mm/zswap.c b/mm/zswap.c index 8163e6c5f76c..8559aec7cd4c 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -972,6 +972,79 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) /********************************* * writeback code **********************************/ + +#define ZSWAP_WRITEBACK_NO_SHADOW xa_mk_value(0) + +/* + * zswap_folio_is_writeback_buffer - is @folio a zswap writeback buffer? + * @folio: the folio being examined (typically a swap cache folio under reclaim) + * + * A folio is a zswap writeback buffer when every one of its swap offsets holds + * a parked writeback shadow (a real shadow or the ZSWAP_WRITEBACK_NO_SHADOW + * sentinel) in the zswap tree rather than a live zswap entry. + * + * Return: true if @folio is a writeback buffer, in which case the reclaim path + * must not mint a fresh workingset shadow for it. + */ +bool zswap_folio_is_writeback_buffer(struct folio *folio) +{ + swp_entry_t swp = folio->swap; + unsigned long nr_pages = folio_nr_pages(folio); + pgoff_t offset = swp_offset(swp); + unsigned long i; + + if (zswap_never_enabled()) + return false; + + for (i = 0; i < nr_pages; i++) { + swp_entry_t e = swp_entry(swp_type(swp), offset + i); + + if (!xa_is_value(xa_load(swap_zswap_tree(e), offset + i))) + return false; + } + + return true; +} + +/* + * zswap_lookup_and_clear_shadows - retrieve and clear @folio's parked shadow(s) + * @folio: the writeback buffer folio (or the swapin folio that consumed it) + * + * Remove any parked writeback shadows for @folio's swap offset(s) from the + * zswap tree. + * + * Return: the preserved workingset shadow, or NULL if the slot(s) had no shadow + * (sentinel only) or nothing parked. The caller either restores the returned + * shadow into the swap slot (buffer dropped) or feeds it to workingset_refault() + * (buffer consumed by a swapin); clearing here ensures the two paths never + * double-count. + */ +void *zswap_lookup_and_clear_shadows(struct folio *folio) +{ + swp_entry_t swp = folio->swap; + unsigned long nr_pages = folio_nr_pages(folio); + pgoff_t offset = swp_offset(swp); + void *shadow = NULL; + unsigned long i; + + if (zswap_never_enabled()) + return NULL; + + for (i = 0; i < nr_pages; i++) { + swp_entry_t e = swp_entry(swp_type(swp), offset + i); + struct xarray *tree = swap_zswap_tree(e); + void *parked = xa_load(tree, offset + i); + + if (!xa_is_value(parked)) + continue; + xa_erase(tree, offset + i); + if (parked != ZSWAP_WRITEBACK_NO_SHADOW) + shadow = parked; + } + + return shadow; +} + /* * Attempts to free an entry by adding a folio to the swap cache, * decompressing the entry data into the folio, and issuing a @@ -992,6 +1065,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry, struct folio *folio; struct mempolicy *mpol; struct swap_info_struct *si; + void *shadow; int ret = 0; /* try to allocate swap cache folio */ @@ -999,6 +1073,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry, if (!si) return -EEXIST; + shadow = swap_cache_get_shadow(swpentry); + mpol = get_task_policy(current); folio = __swap_cache_alloc_folio(swpentry, GFP_KERNEL, BIT(0), NULL, mpol, NO_INTERLEAVE_INDEX); @@ -1035,7 +1111,9 @@ static int zswap_writeback_entry(struct zswap_entry *entry, goto out; } - xa_erase(tree, offset); + if (!shadow) + shadow = ZSWAP_WRITEBACK_NO_SHADOW; + xa_store(tree, offset, shadow, GFP_KERNEL); count_vm_event(ZSWPWB); if (entry->objcg) @@ -1487,6 +1565,17 @@ bool zswap_store(struct folio *folio) VM_WARN_ON_ONCE(!folio_test_locked(folio)); VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); + /* + * A writeback buffer whose IO failed is redirtied and left in the + * swap cache, so reclaim writes it out again. Writeback already + * decided this data belongs on disk, so send it there instead of + * compressing it back into zswap. This also leaves the shadow + * parked for the buffer in place, so it is still restored into + * the slot once the folio leaves the swap cache. + */ + if (zswap_folio_is_writeback_buffer(folio)) + return false; + if (!zswap_enabled) goto check_old; -- 2.53.0-Meta