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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux