Re: [PATCH v4 1/5] mm/zswap: Extend shrink_memcg() writeback capability

[Hao Jia]

On 2026/6/25 00:57, Yosry Ahmed wrote:
> > /*
> >    * Scan up to @nr_to_scan pages across the per-node zswap LRUs of @memcg
> >    * and write back the reclaimable ones.
> >    *
> >    * Since the second-chance algorithm rotates referenced entries to the
> >    * LRU tail, the per-node scan is capped at the current LRU length so
> >    * each entry is scanned at most once per call. It is up to the caller
> >    * to handle retries, deciding whether to scan the next memcg to complete
>
> Nit: "whether to scan another memcg to complete.."

Will fix in the next version.

> >    * the full iteration, or to rescan the current memcg to drain its zswap
> >    * entries.
> >    *
> >    * Return: The number of compressed bytes written back (>= 0), or -ENOENT
> >    * if @memcg has writeback disabled, is a zombie cgroup, or has empty
> >    * zswap LRUs.
> >    */
> > static long shrink_memcg(struct mem_cgroup *memcg, unsigned long nr_to_scan)
> > {
> >       struct zswap_shrink_walk_arg walk_arg = {
> >           .bytes_written = 0,
> >           .encountered_page_in_swapcache = false,
> >       };
> >       unsigned long nr_remaining = nr_to_scan;
> >       int nid;
> >
> >       if (!mem_cgroup_zswap_writeback_enabled(memcg))
> >           return -ENOENT;
> >
> >       /*
> >        * Skip zombies because their LRUs are reparented and we would be
> >        * reclaiming from the parent instead of the dead memcg.
> >        */
> >       if (memcg && !mem_cgroup_online(memcg))
> >           return -ENOENT;
> >
> >       for_each_node_state(nid, N_NORMAL_MEMORY) {
> >           unsigned long nr_to_walk;
> >
> >           /*
> >            * Cap the walk at the current LRU length to ensure each entry is
> >            * scanned at most once per call. Referenced entries are rotated
> >            * to the tail for a second chance, and this bound prevents them
> >            * from being revisited within a single call. Retries are left to
> >            * the caller, which can choose to rescan the current memcg or
> >            * move on to the next one.
> >            */
>
> Nit: Make this more concise since it's already explained above.

Will fix in the next version. Thanks a lot for the review!

Thanks,
Hao

> Otherwise this looks good to me, thank you!
>
> >           nr_to_walk = min(nr_remaining,
> >                    list_lru_count_one(&zswap_list_lru, nid, memcg));
> >           if (!nr_to_walk)
> >               continue;
> >
> >           nr_remaining -= nr_to_walk;
> >           list_lru_walk_one(&zswap_list_lru, nid, memcg, &shrink_memcg_cb,
> >                     &walk_arg, &nr_to_walk);
> >           /* Return the unused share of the budget to the pool. */
> >           nr_remaining += nr_to_walk;
> >
> >           if (!nr_remaining)
> >               break;
> >       }
> >
> >       /* Nothing was scanned: every LRU under @memcg was empty. */
> >       if (nr_remaining == nr_to_scan)
> >           return -ENOENT;
> >
> >       return walk_arg.bytes_written;
> > }