[merged mm-hotfixes-stable] mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too.patch removed from -mm tree

[merged mm-hotfixes-stable] mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too.patch removed from -mm tree

[Andrew Morton]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4445 bytes --]


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@yhndnzj.com>
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@yhndnzj.com
Fixes: 501a06fe8e4c ("zswap: memcontrol: implement zswap writeback disabling")
Signed-off-by: Mike Yuan <me@yhndnzj.com>
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Acked-by: Yosry Ahmed <yosryahmed@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Michal Koutný <mkoutny@suse.com>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 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@yhndnzj.com are

documentation-cgroup-v2-clarify-that-zswapwriteback-is-ignored-if-zswap-is-disabled.patch
selftests-test_zswap-add-test-for-hierarchical-zswapwriteback.patch

Re: [merged mm-hotfixes-stable] mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too.patch removed from -mm tree

[Yosry Ahmed]

On Sun, Sep 1, 2024 at 5:59 PM Andrew Morton <akpm@linux-foundation.org> 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@yhndnzj.com>
> 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@yhndnzj.com
> Fixes: 501a06fe8e4c ("zswap: memcontrol: implement zswap writeback disabling")
> Signed-off-by: Mike Yuan <me@yhndnzj.com>
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> Acked-by: Yosry Ahmed <yosryahmed@google.com>

We wanted to CC stable here, it's too late at this point, right?

> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Michal Koutný <mkoutny@suse.com>
> Cc: Muchun Song <muchun.song@linux.dev>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Cc: Shakeel Butt <shakeel.butt@linux.dev>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  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@yhndnzj.com are
>
> documentation-cgroup-v2-clarify-that-zswapwriteback-is-ignored-if-zswap-is-disabled.patch
> selftests-test_zswap-add-test-for-hierarchical-zswapwriteback.patch
>

Re: [merged mm-hotfixes-stable] mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too.patch removed from -mm tree

[Andrew Morton]

On Tue, 3 Sep 2024 10:53:21 -0700 Yosry Ahmed <yosryahmed@google.com> wrote:

> > 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@yhndnzj.com
> > Fixes: 501a06fe8e4c ("zswap: memcontrol: implement zswap writeback disabling")
> > Signed-off-by: Mike Yuan <me@yhndnzj.com>
> > Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> > Acked-by: Yosry Ahmed <yosryahmed@google.com>
> 
> We wanted to CC stable here, it's too late at this point, right?

It has cc:stable?

> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Michal Hocko <mhocko@kernel.org>
> > Cc: Michal Koutný <mkoutny@suse.com>
> > Cc: Muchun Song <muchun.song@linux.dev>
> > Cc: Roman Gushchin <roman.gushchin@linux.dev>
> > Cc: Shakeel Butt <shakeel.butt@linux.dev>
> > Cc: <stable@vger.kernel.org>

^^ here

Re: [merged mm-hotfixes-stable] mm-memcontrol-respect-zswapwriteback-setting-from-parent-cg-too.patch removed from -mm tree

[Yosry Ahmed]

On Tue, Sep 3, 2024 at 12:53 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 3 Sep 2024 10:53:21 -0700 Yosry Ahmed <yosryahmed@google.com> wrote:
>
> > > 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@yhndnzj.com
> > > Fixes: 501a06fe8e4c ("zswap: memcontrol: implement zswap writeback disabling")
> > > Signed-off-by: Mike Yuan <me@yhndnzj.com>
> > > Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> > > Acked-by: Yosry Ahmed <yosryahmed@google.com>
> >
> > We wanted to CC stable here, it's too late at this point, right?
>
> It has cc:stable?
>
> > > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > > Cc: Michal Hocko <mhocko@kernel.org>
> > > Cc: Michal Koutný <mkoutny@suse.com>
> > > Cc: Muchun Song <muchun.song@linux.dev>
> > > Cc: Roman Gushchin <roman.gushchin@linux.dev>
> > > Cc: Shakeel Butt <shakeel.butt@linux.dev>
> > > Cc: <stable@vger.kernel.org>
>
> ^^ here

Sorry for the noise, I'll go find an eye doctor.