On Wed, Nov 29, 2023 at 1:18 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Tue 28-11-23 08:53:56, Nhat Pham wrote: > > On Tue, Nov 28, 2023 at 1:38 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > On Mon 27-11-23 11:36:59, Nhat Pham wrote: > > > > The new zswap writeback scheme requires an online-only memcg hierarchy > > > > traversal. Add a new parameter to mem_cgroup_iter() to check for > > > > onlineness before returning. > > > > > > Why is this needed? > > > > For context, in patch 3 of this series, Domenico and I are adding > > cgroup-aware LRU to zswap, so that we can perform workload-specific > > zswap writeback. When the reclaim happens due to the global zswap > > limit being hit, a cgroup is selected by the mem_cgroup_iter(), and > > the last one selected is saved in the zswap pool (so that the > > iteration can follow from there next time the limit is hit). > > > > However, one problem with this scheme is we will be pinning the > > reference to that saved memcg until the next global reclaim attempt, > > which could prevent it from being killed for quite some time after it > > has been offlined. Johannes, Yosry, and I discussed a couple of > > approaches for a while, and decided to add a callback that would > > release the reference held by the zswap pool when the memcg is > > offlined, and the zswap pool will obtain the reference to the next > > online memcg in the traversal (or at least one that has not had the > > zswap-memcg-release-callback run on it yet). > > This should be a part of the changelog along with an explanation why > this cannot be handled on the caller level? You have a pin on the memcg, > you can check it is online and scratch it if not, right? Why do we need > to make a rather convoluted iterator interface more complex when most > users simply do not require that? Ah that's a good point. Hmm then I'll just do an extra online check in the zswap reclaim callsite - cleaner and less invasive. Thanks for the suggestion! > > -- > Michal Hocko > SUSE Labs
