Re: [RFC PATCH 4/4] mm/zswap: move reclaim-driven writeback to a kworker

[Nhat Pham]

On Mon, Sep 28, 2026 at 9:23 AM Alexandre Ghiti <alex@xxxxxxxx> wrote:

I understand the problem this patch is trying to solve, but this
solution seems more involved than the changelog suggests :)

>
> zswap_shrinker_scan() writes back in whatever context reclaim called it
> from: an application thread inside a page fault, or kswapd reclaiming
> for the whole node. Now that the cgroup IO controllers throttle zswap
> writeback, that context sleeps uninterruptibly until the owning cgroup's
> IO budget allows the write. Such stalls were observed on a large
> production workload.
>
> So defer the writeback to a kworker: each lruvec has its own work item
> on shrink_wq, which writes back what reclaim asked for, SWAP_CLUSTER_MAX

Ahhh, so it's one work-item (added to the shared shrink_wq)
per-lruvec, not one kworker or thread per-lruvec.

Hmm, supposed multiple tasks in a cgroup enter direct reclaim. In the
past, the amount of writeback submission will scale with the number of
tasks entering reclaim. But now, it's just a single work item
per-lruvec - IIUC, the submission of his work item is idempotent, so
it wouldn't scale like that anymore, right? I haven't wrapped quite
wrapped my head around this, but this sounds like a non-trivial
effect...

Also, do we need to defer writeback submission for proactive
reclaimer? Or kswapd? Only direct reclaimers have concerns with the
latency blowup from the throttling right?

There are a couple more behavioural changes, without proper
justifcation or mentioning in the changelog. More details inlined
below:

> entries at a time, while holding a reference on the memcg. The shrinker
> does not queue more work while the worker has yet to claim the previous

Why not?

> one, and reports nothing freed, as nothing is until the worker runs.
>
> Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>

[...]

> +
> +static bool zswap_defer_writeback(struct shrink_control *sc)
> +{
> +       struct lruvec *lruvec = mem_cgroup_lruvec(sc->memcg, NODE_DATA(sc->nid));
> +       struct zswap_lruvec_state *zls = &lruvec->zswap_lruvec_state;
> +       long old = 0;
> +
> +       /* Do not accumulate work: back off if the worker is lagging behind. */

Why not? This also has a non-trivial effect on the writeback amount...

> +       if (!atomic_long_try_cmpxchg(&zls->nr_deferred_writeback, &old,
> +                                    sc->nr_to_scan))
> +               return false;
> +
> +       if (!mem_cgroup_tryget_online(sc->memcg))
> +               return false;
>
> -       shrink_ret = list_lru_shrink_walk(&zswap_list_lru, sc, &shrink_memcg_cb,
> -               &flags);
> +       if (!queue_work(shrink_wq, &zls->deferred_writeback_work))
> +               mem_cgroup_put(sc->memcg);
> +
> +       return true;
> +}
>
> -       if (flags & ZSWAP_SHRINK_SWAPCACHE)
> +static unsigned long zswap_shrinker_scan(struct shrinker *shrinker,
> +                                        struct shrink_control *sc)
> +{
> +       if (!zswap_shrinker_enabled ||
> +           !mem_cgroup_zswap_writeback_enabled(sc->memcg)) {
> +               sc->nr_scanned = 0;
>                 return SHRINK_STOP;
> +       }
>
> -       return shrink_ret ? shrink_ret : SHRINK_STOP;
> +       /* Nothing is freed until the worker runs, so report nothing. */
> +       return zswap_defer_writeback(sc) ? 0 : SHRINK_STOP;

This will have effect on the shrinker control loop too, right?
Shrinker API accumulates the number of writebacks overtime based on
shrinker_count()'s output, and we serve that number of writebacks
throgh shrinker_scan(). With this basically that accumulation just
grows over time, correct? Check do_shrink_slab() in mm/shrinker.c...

Feels like this completely breaks that control loop.

>  }
>
>  static unsigned long zswap_shrinker_count(struct shrinker *shrinker,
> @@ -1807,7 +1897,7 @@ static int zswap_setup(void)
>                 goto hp_fail;
>
>         shrink_wq = alloc_workqueue("zswap-shrink",
> -                       WQ_UNBOUND|WQ_MEM_RECLAIM, 1);
> +                       WQ_UNBOUND | WQ_MEM_RECLAIM, 0);

This turns a single-threaded workqueue into multi-threaded workqueue, IIUC.

Worth at least an callout in the changelog?

>         if (!shrink_wq)
>                 goto shrink_wq_fail;
>
> --
> 2.53.0-Meta
>