On Sun, Sep 1, 2024 at 5:59 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > > The quilt patch titled > Subject: mm/memcontrol: respect zswap.writeback setting from parent cg too > has been removed from the -mm tree. Its filename was > mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too.patch > > This patch was dropped because it was merged into the mm-hotfixes-stable branch > of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > > ------------------------------------------------------ > From: Mike Yuan <me@xxxxxxxxxxx> > Subject: mm/memcontrol: respect zswap.writeback setting from parent cg too > Date: Fri, 23 Aug 2024 16:27:06 +0000 > > Currently, the behavior of zswap.writeback wrt. the cgroup hierarchy > seems a bit odd. Unlike zswap.max, it doesn't honor the value from parent > cgroups. This surfaced when people tried to globally disable zswap > writeback, i.e. reserve physical swap space only for hibernation [1] - > disabling zswap.writeback only for the root cgroup results in subcgroups > with zswap.writeback=1 still performing writeback. > > The inconsistency became more noticeable after I introduced the > MemoryZSwapWriteback= systemd unit setting [2] for controlling the knob. > The patch assumed that the kernel would enforce the value of parent > cgroups. It could probably be workarounded from systemd's side, by going > up the slice unit tree and inheriting the value. Yet I think it's more > sensible to make it behave consistently with zswap.max and friends. > > [1] https://wiki.archlinux.org/title/Power_management/Suspend_and_hibernate#Disable_zswap_writeback_to_use_the_swap_space_only_for_hibernation > [2] https://github.com/systemd/systemd/pull/31734 > > Link: https://lkml.kernel.org/r/20240823162506.12117-1-me@xxxxxxxxxxx > Fixes: 501a06fe8e4c ("zswap: memcontrol: implement zswap writeback disabling") > Signed-off-by: Mike Yuan <me@xxxxxxxxxxx> > Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx> > Acked-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> We wanted to CC stable here, it's too late at this point, right? > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > Cc: Michal Hocko <mhocko@xxxxxxxxxx> > Cc: Michal Koutný <mkoutny@xxxxxxxx> > Cc: Muchun Song <muchun.song@xxxxxxxxx> > Cc: Roman Gushchin <roman.gushchin@xxxxxxxxx> > Cc: Shakeel Butt <shakeel.butt@xxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > --- > > Documentation/admin-guide/cgroup-v2.rst | 7 ++++--- > mm/memcontrol.c | 12 +++++++++--- > 2 files changed, 13 insertions(+), 6 deletions(-) > > --- a/Documentation/admin-guide/cgroup-v2.rst~mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too > +++ a/Documentation/admin-guide/cgroup-v2.rst > @@ -1717,9 +1717,10 @@ The following nested keys are defined. > entries fault back in or are written out to disk. > > memory.zswap.writeback > - A read-write single value file. The default value is "1". The > - initial value of the root cgroup is 1, and when a new cgroup is > - created, it inherits the current value of its parent. > + A read-write single value file. The default value is "1". > + Note that this setting is hierarchical, i.e. the writeback would be > + implicitly disabled for child cgroups if the upper hierarchy > + does so. > > When this is set to 0, all swapping attempts to swapping devices > are disabled. This included both zswap writebacks, and swapping due > --- a/mm/memcontrol.c~mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too > +++ a/mm/memcontrol.c > @@ -3613,8 +3613,7 @@ mem_cgroup_css_alloc(struct cgroup_subsy > memcg1_soft_limit_reset(memcg); > #ifdef CONFIG_ZSWAP > memcg->zswap_max = PAGE_COUNTER_MAX; > - WRITE_ONCE(memcg->zswap_writeback, > - !parent || READ_ONCE(parent->zswap_writeback)); > + WRITE_ONCE(memcg->zswap_writeback, true); > #endif > page_counter_set_high(&memcg->swap, PAGE_COUNTER_MAX); > if (parent) { > @@ -5320,7 +5319,14 @@ void obj_cgroup_uncharge_zswap(struct ob > bool mem_cgroup_zswap_writeback_enabled(struct mem_cgroup *memcg) > { > /* if zswap is disabled, do not block pages going to the swapping device */ > - return !zswap_is_enabled() || !memcg || READ_ONCE(memcg->zswap_writeback); > + if (!zswap_is_enabled()) > + return true; > + > + for (; memcg; memcg = parent_mem_cgroup(memcg)) > + if (!READ_ONCE(memcg->zswap_writeback)) > + return false; > + > + return true; > } > > static u64 zswap_current_read(struct cgroup_subsys_state *css, > _ > > Patches currently in -mm which might be from me@xxxxxxxxxxx are > > documentation-cgroup-v2-clarify-that-zswapwriteback-is-ignored-if-zswap-is-disabled.patch > selftests-test_zswap-add-test-for-hierarchical-zswapwriteback.patch >
