Re: [PATCH v4 13/19] mm, swap: remove workaround for unsynchronized swap map cache state

Baoquan He
2025-12-18T03:37:43Z

On 12/05/25 at 03:29am, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
>
> Remove the "skip if exists" check from commit a65b0e7607ccb ("zswap:
> make shrinking memcg-aware"). It was needed because there is a tiny time
> window between setting the SWAP_HAS_CACHE bit and actually adding the
> folio to the swap cache. If a user is trying to add the folio into the
> swap cache but another user was interrupted after setting SWAP_HAS_CACHE
> but hasn't added the folio to the swap cache yet, it might lead to a
> deadlock.
>
> We have moved the bit setting to the same critical section as adding the
> folio, so this is no longer needed. Remove it and clean it up.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
> mm/swap.h | 2 +-
> mm/swap_state.c | 27 ++++++++++-----------------
> mm/zswap.c | 2 +-
> 3 files changed, 12 insertions(+), 19 deletions(-)

Reviewed-by: Baoquan He <bhe@redhat.com>

>
> diff --git a/mm/swap.h b/mm/swap.h
> index b5075a1aee04..6777b2ab9d92 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -260,7 +260,7 @@ int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> void swap_cache_del_folio(struct folio *folio);
> struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_flags,
> struct mempolicy *mpol, pgoff_t ilx,
> - bool *alloced, bool skip_if_exists);
> + bool *alloced);
> /* Below helpers require the caller to lock and pass in the swap cluster. */
> void __swap_cache_del_folio(struct swap_cluster_info *ci,
> struct folio *folio, swp_entry_t entry, void *shadow);
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index df7df8b75e52..1a69ba3be87f 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -445,8 +445,6 @@ void swap_update_readahead(struct folio *folio, struct vm_area_struct *vma,
> * @folio: folio to be added.
> * @gfp: memory allocation flags for charge, can be 0 if @charged if true.
> * @charged: if the folio is already charged.
> - * @skip_if_exists: if the slot is in a cached state, return NULL.
> - * This is an old workaround that will be removed shortly.
> *
> * Update the swap_map and add folio as swap cache, typically before swapin.
> * All swap slots covered by the folio must have a non-zero swap count.
> @@ -457,8 +455,7 @@ void swap_update_readahead(struct folio *folio, struct vm_area_struct *vma,
> */
> static struct folio *__swap_cache_prepare_and_add(swp_entry_t entry,
> struct folio *folio,
> - gfp_t gfp, bool charged,
> - bool skip_if_exists)
> + gfp_t gfp, bool charged)
> {
> struct folio *swapcache = NULL;
> void *shadow;
> @@ -478,7 +475,7 @@ static struct folio *__swap_cache_prepare_and_add(swp_entry_t entry,
> * might return a folio that is irrelevant to the faulting
> * entry because @entry is aligned down. Just return NULL.
> */
> - if (ret != -EEXIST || skip_if_exists || folio_test_large(folio))
> + if (ret != -EEXIST || folio_test_large(folio))
> goto failed;
>
> swapcache = swap_cache_get_folio(entry);
> @@ -511,8 +508,6 @@ static struct folio *__swap_cache_prepare_and_add(swp_entry_t entry,
> * @mpol: NUMA memory allocation policy to be applied
> * @ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
> * @new_page_allocated: sets true if allocation happened, false otherwise
> - * @skip_if_exists: if the slot is a partially cached state, return NULL.
> - * This is a workaround that would be removed shortly.
> *
> * Allocate a folio in the swap cache for one swap slot, typically before
> * doing IO (e.g. swap in or zswap writeback). The swap slot indicated by
> @@ -525,8 +520,7 @@ static struct folio *__swap_cache_prepare_and_add(swp_entry_t entry,
> */
> struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_mask,
> struct mempolicy *mpol, pgoff_t ilx,
> - bool *new_page_allocated,
> - bool skip_if_exists)
> + bool *new_page_allocated)
> {
> struct swap_info_struct *si = __swap_entry_to_info(entry);
> struct folio *folio;
> @@ -547,8 +541,7 @@ struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_mask,
> if (!folio)
> return NULL;
> /* Try add the new folio, returns existing folio or NULL on failure. */
> - result = __swap_cache_prepare_and_add(entry, folio, gfp_mask,
> - false, skip_if_exists);
> + result = __swap_cache_prepare_and_add(entry, folio, gfp_mask, false);
> if (result == folio)
> *new_page_allocated = true;
> else
> @@ -577,7 +570,7 @@ struct folio *swapin_folio(swp_entry_t entry, struct folio *folio)
> unsigned long nr_pages = folio_nr_pages(folio);
>
> entry = swp_entry(swp_type(entry), round_down(offset, nr_pages));
> - swapcache = __swap_cache_prepare_and_add(entry, folio, 0, true, false);
> + swapcache = __swap_cache_prepare_and_add(entry, folio, 0, true);
> if (swapcache == folio)
> swap_read_folio(folio, NULL);
> return swapcache;
> @@ -605,7 +598,7 @@ struct folio *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>
> mpol = get_vma_policy(vma, addr, 0, &ilx);
> folio = swap_cache_alloc_folio(entry, gfp_mask, mpol, ilx,
> - &page_allocated, false);
> + &page_allocated);
> mpol_cond_put(mpol);
>
> if (page_allocated)
> @@ -724,7 +717,7 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
> /* Ok, do the async read-ahead now */
> folio = swap_cache_alloc_folio(
> swp_entry(swp_type(entry), offset), gfp_mask, mpol, ilx,
> - &page_allocated, false);
> + &page_allocated);
> if (!folio)
> continue;
> if (page_allocated) {
> @@ -742,7 +735,7 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
> skip:
> /* The page was likely read above, so no need for plugging here */
> folio = swap_cache_alloc_folio(entry, gfp_mask, mpol, ilx,
> - &page_allocated, false);
> + &page_allocated);
> if (unlikely(page_allocated))
> swap_read_folio(folio, NULL);
> return folio;
> @@ -847,7 +840,7 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
> continue;
> }
> folio = swap_cache_alloc_folio(entry, gfp_mask, mpol, ilx,
> - &page_allocated, false);
> + &page_allocated);
> if (si)
> put_swap_device(si);
> if (!folio)
> @@ -869,7 +862,7 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
> skip:
> /* The folio was likely read above, so no need for plugging here */
> folio = swap_cache_alloc_folio(targ_entry, gfp_mask, mpol, targ_ilx,
> - &page_allocated, false);
> + &page_allocated);
> if (unlikely(page_allocated))
> swap_read_folio(folio, NULL);
> return folio;
> diff --git a/mm/zswap.c b/mm/zswap.c
> index a7a2443912f4..d8a33db9d3cc 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1015,7 +1015,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>
> mpol = get_task_policy(current);
> folio = swap_cache_alloc_folio(swpentry, GFP_KERNEL, mpol,
> - NO_INTERLEAVE_INDEX, &folio_was_allocated, true);
> + NO_INTERLEAVE_INDEX, &folio_was_allocated);
> put_swap_device(si);
> if (!folio)
> return -ENOMEM;
>
> --
> 2.52.0
>