From f01dc08b734809d760ced014af7414c280b8391a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 16 Aug 2024 14:44:07 +0000 Subject: [PATCH] mm/memcontrol: respect zswap.writeback setting from parent cg too 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=3D1 still performing writeback. The inconsistency became more noticeable after I introduced the MemoryZSwapWriteback=3D 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/20240816144344.18135-1-me@yhndnzj.com Signed-off-by: Mike Yuan Reviewed-by: Nhat Pham Cc: Yosry Ahmed Cc: Johannes Weiner Cc: Michal Hocko Cc: Muchun Song Cc: Roman Gushchin Cc: Shakeel Butt Cc: Yosry Ahmed Signed-off-by: Andrew Morton --- Documentation/admin-guide/cgroup-v2.rst | 5 ++++- mm/memcontrol.c | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index d3344218010c..b90219c0210d 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -1744,7 +1744,10 @@ The following nested keys are defined. 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. + created, it inherits the current value of its parent. 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 diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 001803c13204..42c826eec124 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5355,7 +5355,14 @@ void obj_cgroup_uncharge_zswap(struct obj_cgroup *objcg, size_t size) 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, -- 2.49.0