Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Yosry Ahmed]

On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@xxxxxxxxx> wrote:
>
> From: Kaitao Cheng <chengkaitao@xxxxxxxxxx>
>
> Successful writeback calls zswap_entry_free(), which removes the entry
> from the LRU and decrements both the sublist and node item counts.
> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> to decrement the node count again.

I wonder if we have a valid use case for adding LRU_REMOVED_COUNTED or
LRU_DELETED instead to simply avoid the decrement.

Adding shrinker maintainers.

>
> The affected call path before this change is:
>
>   __list_lru_walk_one()
>     shrink_memcg_cb()
>       zswap_writeback_entry()
>         zswap_entry_free()
>           zswap_lru_del()
>             list_lru_del()
>               __list_lru_del()
>                 atomic_long_dec()  /* first node count decrement */
>       return LRU_REMOVED_RETRY
>     atomic_long_dec()              /* second node count decrement */
>
> Add zswap_lru_del_nocnt() to remove the entry and update only the sublist
> count, leaving node accounting to the walker. Split data cleanup into
> __zswap_entry_free() so writeback can free the entry without performing
> another LRU deletion. Other free paths retain their existing accounting.
>
> Fixes: 5878303c5353 ("mm/zswap: fix race between lru writeback and swapoff")
> Link: https://sashiko.dev/#/bug/linux-68d2ca49-23be-4504-9a75-b8526ec92388
> Signed-off-by: Kaitao Cheng <chengkaitao@xxxxxxxxxx>
> ---
>  mm/zswap.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index ad01c691a263..d0c98daddf2e 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -719,6 +719,22 @@ static void zswap_lru_del(struct zswap_entry *entry)
>         rcu_read_unlock();
>  }
>
> +/* Like zswap_lru_del(), but does not decrement the node item count. */
> +static void zswap_lru_del_nocnt(struct zswap_entry *entry)
> +{
> +       int nid = entry_to_nid(entry);
> +       struct mem_cgroup *memcg;
> +       struct list_lru_one *l;
> +
> +       rcu_read_lock();
> +       memcg = obj_cgroup_memcg(entry->objcg);
> +       l = list_lru_lock(&zswap_list_lru, nid, &memcg);
> +       if (!list_empty(&entry->lru))
> +               list_lru_isolate(l, &entry->lru);
> +       list_lru_unlock(l);
> +       rcu_read_unlock();
> +}
> +
>  void zswap_lruvec_state_init(struct lruvec *lruvec)
>  {
>         atomic_long_set(&lruvec->zswap_lruvec_state.nr_disk_swapins, 0);
> @@ -781,11 +797,10 @@ static void zswap_entry_cache_free(struct zswap_entry *entry)
>   * Carries out the common pattern of freeing an entry's zsmalloc allocation,
>   * freeing the entry itself, and decrementing the number of stored pages.
>   */
> -static void zswap_entry_free(struct zswap_entry *entry)
> +static void __zswap_entry_free(struct zswap_entry *entry)
>  {
>         struct zswap_pool *pool = zswap_entry_pool(entry);
>
> -       zswap_lru_del(entry);
>         if (!WARN_ON_ONCE(!pool)) {
>                 zs_free(pool->zs_pool, entry->handle);
>                 zswap_pool_put(pool);
> @@ -800,6 +815,12 @@ static void zswap_entry_free(struct zswap_entry *entry)
>         atomic_long_dec(&zswap_stored_pages);
>  }
>
> +static void zswap_entry_free(struct zswap_entry *entry)
> +{
> +       zswap_lru_del(entry);
> +       __zswap_entry_free(entry);
> +}
> +
>  /*********************************
>  * compressed storage functions
>  **********************************/
> @@ -1119,7 +1140,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>         if (entry->objcg)
>                 count_objcg_events(entry->objcg, ZSWPWB, 1);
>
> -       zswap_entry_free(entry);
> +       zswap_lru_del_nocnt(entry);
> +       __zswap_entry_free(entry);
>
>         /* folio is up to date */
>         folio_mark_uptodate(folio);
> --
> 2.53.0
>