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

[Alexandre Ghiti]

Hi Nhat,

On Tue, Sep 29, 2026 at 3:43 PM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
>
> >
> 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...

It would not scale indeed, the worker does not accumulate in this
version of the patch. That needs to be tested and discussed.

>
> 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?

Today the writeback bio is issued as root, which incurs debt to the
cgroup. The submitting task repays it on return to user or on any
other IO of the cgroup until the debt is paid off.

With this series the bio is throttled at submission instead, so
whoever submits it sleeps until the cgroup has budget:
- for direct reclaimers, that's the application latency we observed
- for kswapd reclaim, it would stall on a single cgroup's IO budget

So we need to defer for everyone.

>
> 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?

In the large production workload I have worked on, when the shrinker
is enabled, the zswap pool size decreases by ~97%. So I did not want
to accumulate "too much" work to prevent zswap from draining "hot"
pages, it acts like an additional throttling mechanism. But that needs
discussion, hence the RFC.

And you're right, In the next version, I'll clearly point out all the
behavioral changes I made with their justifications.

>
> > 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...

Explained above

>
> > +       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.

Yes, this is another behavioural change that I will explain in the next version.

Indeed the work piles up in the shrinker control loop if writeback
cannot catch up but note that it is bounded, which is what we want
right?

>
> >  }
> >
> >  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?

You're right, and the explanation is "we cannot starve other memcgs if
one worker stalls because it exhausted its memcg budget".

Thanks Nhat!

Alex




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