[v5,0/6] workload-specific and memory pressure-driven zswap writeback

  Changelog:
v5:
   * Replace reference getting with an rcu_read_lock() section for
     zswap lru modifications (suggested by Yosry)
   * Add a new prep patch that allows mem_cgroup_iter() to return
     online cgroup.
   * Add a callback that updates pool->next_shrink when the cgroup is
     offlined (suggested by Yosry Ahmed, Johannes Weiner)
v4:
   * Rename list_lru_add to list_lru_add_obj and __list_lru_add to
     list_lru_add (patch 1) (suggested by Johannes Weiner and
	 Yosry Ahmed)
   * Some cleanups on the memcg aware LRU patch (patch 2)
     (suggested by Yosry Ahmed)
   * Use event interface for the new per-cgroup writeback counters.
     (patch 3) (suggested by Yosry Ahmed)
   * Abstract zswap's lruvec states and handling into 
     zswap_lruvec_state (patch 5) (suggested by Yosry Ahmed)
v3:
   * Add a patch to export per-cgroup zswap writeback counters
   * Add a patch to update zswap's kselftest
   * Separate the new list_lru functions into its own prep patch
   * Do not start from the top of the hierarchy when encounter a memcg
     that is not online for the global limit zswap writeback (patch 2)
     (suggested by Yosry Ahmed)
   * Do not remove the swap entry from list_lru in
     __read_swapcache_async() (patch 2) (suggested by Yosry Ahmed)
   * Removed a redundant zswap pool getting (patch 2)
     (reported by Ryan Roberts)
   * Use atomic for the nr_zswap_protected (instead of lruvec's lock)
     (patch 5) (suggested by Yosry Ahmed)
   * Remove the per-cgroup zswap shrinker knob (patch 5)
     (suggested by Yosry Ahmed)
v2:
   * Fix loongarch compiler errors
   * Use pool stats instead of memcg stats when !CONFIG_MEMCG_KEM

There are currently several issues with zswap writeback:

1. There is only a single global LRU for zswap, making it impossible to
   perform worload-specific shrinking - an memcg under memory pressure
   cannot determine which pages in the pool it owns, and often ends up
   writing pages from other memcgs. This issue has been previously
   observed in practice and mitigated by simply disabling
   memcg-initiated shrinking:

   https://lore.kernel.org/all/20230530232435.3097106-1-nphamcs@gmail.com/T/#u

   But this solution leaves a lot to be desired, as we still do not
   have an avenue for an memcg to free up its own memory locked up in
   the zswap pool.

2. We only shrink the zswap pool when the user-defined limit is hit.
   This means that if we set the limit too high, cold data that are
   unlikely to be used again will reside in the pool, wasting precious
   memory. It is hard to predict how much zswap space will be needed
   ahead of time, as this depends on the workload (specifically, on
   factors such as memory access patterns and compressibility of the
   memory pages).

This patch series solves these issues by separating the global zswap
LRU into per-memcg and per-NUMA LRUs, and performs workload-specific
(i.e memcg- and NUMA-aware) zswap writeback under memory pressure. The
new shrinker does not have any parameter that must be tuned by the
user, and can be opted in or out on a per-memcg basis.

As a proof of concept, we ran the following synthetic benchmark:
build the linux kernel in a memory-limited cgroup, and allocate some
cold data in tmpfs to see if the shrinker could write them out and
improved the overall performance. Depending on the amount of cold data
generated, we observe from 14% to 35% reduction in kernel CPU time used
in the kernel builds.

Domenico Cerasuolo (3):
  zswap: make shrinking memcg-aware
  mm: memcg: add per-memcg zswap writeback stat
  selftests: cgroup: update per-memcg zswap writeback selftest

Nhat Pham (3):
  list_lru: allows explicit memcg and NUMA node selection
  memcontrol: allows mem_cgroup_iter() to check for onlineness
  zswap: shrinks zswap pool based on memory pressure

 Documentation/admin-guide/mm/zswap.rst      |   7 +
 drivers/android/binder_alloc.c              |   5 +-
 fs/dcache.c                                 |   8 +-
 fs/gfs2/quota.c                             |   6 +-
 fs/inode.c                                  |   4 +-
 fs/nfs/nfs42xattr.c                         |   8 +-
 fs/nfsd/filecache.c                         |   4 +-
 fs/xfs/xfs_buf.c                            |   6 +-
 fs/xfs/xfs_dquot.c                          |   2 +-
 fs/xfs/xfs_qm.c                             |   2 +-
 include/linux/list_lru.h                    |  46 ++-
 include/linux/memcontrol.h                  |   9 +-
 include/linux/mmzone.h                      |   2 +
 include/linux/vm_event_item.h               |   1 +
 include/linux/zswap.h                       |  27 +-
 mm/list_lru.c                               |  48 ++-
 mm/memcontrol.c                             |  20 +-
 mm/mmzone.c                                 |   1 +
 mm/shrinker.c                               |   4 +-
 mm/swap.h                                   |   3 +-
 mm/swap_state.c                             |  26 +-
 mm/vmscan.c                                 |  26 +-
 mm/vmstat.c                                 |   1 +
 mm/workingset.c                             |   4 +-
 mm/zswap.c                                  | 430 +++++++++++++++++---
 tools/testing/selftests/cgroup/test_zswap.c |  74 ++--
 26 files changed, 625 insertions(+), 149 deletions(-)
  

Hi Nhat,

Sorry for being late to the party. I want to take a look at your patches series.
However I wasn't able to "git am" your patches series cleanly on current
mm-stable, mm-unstable or linux tip.

$ git am patches/v5_20231106_nphamcs_workload_specific_and_memory_pressure_driven_zswap_writeback.mbx
Applying: list_lru: allows explicit memcg and NUMA node selection
Applying: memcontrol: allows mem_cgroup_iter() to check for onlineness
Applying: zswap: make shrinking memcg-aware (fix)
error: patch failed: mm/zswap.c:174
error: mm/zswap.c: patch does not apply
Patch failed at 0003 zswap: make shrinking memcg-aware (fix)

What is the base of your patches? A git hash or a branch I can pull
from would be
nice.

Thanks

Chris

On Mon, Nov 6, 2023 at 10:32 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> Changelog:
> v5:
>    * Replace reference getting with an rcu_read_lock() section for
>      zswap lru modifications (suggested by Yosry)
>    * Add a new prep patch that allows mem_cgroup_iter() to return
>      online cgroup.
>    * Add a callback that updates pool->next_shrink when the cgroup is
>      offlined (suggested by Yosry Ahmed, Johannes Weiner)
> v4:
>    * Rename list_lru_add to list_lru_add_obj and __list_lru_add to
>      list_lru_add (patch 1) (suggested by Johannes Weiner and
>          Yosry Ahmed)
>    * Some cleanups on the memcg aware LRU patch (patch 2)
>      (suggested by Yosry Ahmed)
>    * Use event interface for the new per-cgroup writeback counters.
>      (patch 3) (suggested by Yosry Ahmed)
>    * Abstract zswap's lruvec states and handling into
>      zswap_lruvec_state (patch 5) (suggested by Yosry Ahmed)
> v3:
>    * Add a patch to export per-cgroup zswap writeback counters
>    * Add a patch to update zswap's kselftest
>    * Separate the new list_lru functions into its own prep patch
>    * Do not start from the top of the hierarchy when encounter a memcg
>      that is not online for the global limit zswap writeback (patch 2)
>      (suggested by Yosry Ahmed)
>    * Do not remove the swap entry from list_lru in
>      __read_swapcache_async() (patch 2) (suggested by Yosry Ahmed)
>    * Removed a redundant zswap pool getting (patch 2)
>      (reported by Ryan Roberts)
>    * Use atomic for the nr_zswap_protected (instead of lruvec's lock)
>      (patch 5) (suggested by Yosry Ahmed)
>    * Remove the per-cgroup zswap shrinker knob (patch 5)
>      (suggested by Yosry Ahmed)
> v2:
>    * Fix loongarch compiler errors
>    * Use pool stats instead of memcg stats when !CONFIG_MEMCG_KEM
>
> There are currently several issues with zswap writeback:
>
> 1. There is only a single global LRU for zswap, making it impossible to
>    perform worload-specific shrinking - an memcg under memory pressure
>    cannot determine which pages in the pool it owns, and often ends up
>    writing pages from other memcgs. This issue has been previously
>    observed in practice and mitigated by simply disabling
>    memcg-initiated shrinking:
>
>    https://lore.kernel.org/all/20230530232435.3097106-1-nphamcs@gmail.com/T/#u
>
>    But this solution leaves a lot to be desired, as we still do not
>    have an avenue for an memcg to free up its own memory locked up in
>    the zswap pool.
>
> 2. We only shrink the zswap pool when the user-defined limit is hit.
>    This means that if we set the limit too high, cold data that are
>    unlikely to be used again will reside in the pool, wasting precious
>    memory. It is hard to predict how much zswap space will be needed
>    ahead of time, as this depends on the workload (specifically, on
>    factors such as memory access patterns and compressibility of the
>    memory pages).
>
> This patch series solves these issues by separating the global zswap
> LRU into per-memcg and per-NUMA LRUs, and performs workload-specific
> (i.e memcg- and NUMA-aware) zswap writeback under memory pressure. The
> new shrinker does not have any parameter that must be tuned by the
> user, and can be opted in or out on a per-memcg basis.
>
> As a proof of concept, we ran the following synthetic benchmark:
> build the linux kernel in a memory-limited cgroup, and allocate some
> cold data in tmpfs to see if the shrinker could write them out and
> improved the overall performance. Depending on the amount of cold data
> generated, we observe from 14% to 35% reduction in kernel CPU time used
> in the kernel builds.
>
> Domenico Cerasuolo (3):
>   zswap: make shrinking memcg-aware
>   mm: memcg: add per-memcg zswap writeback stat
>   selftests: cgroup: update per-memcg zswap writeback selftest
>
> Nhat Pham (3):
>   list_lru: allows explicit memcg and NUMA node selection
>   memcontrol: allows mem_cgroup_iter() to check for onlineness
>   zswap: shrinks zswap pool based on memory pressure
>
>  Documentation/admin-guide/mm/zswap.rst      |   7 +
>  drivers/android/binder_alloc.c              |   5 +-
>  fs/dcache.c                                 |   8 +-
>  fs/gfs2/quota.c                             |   6 +-
>  fs/inode.c                                  |   4 +-
>  fs/nfs/nfs42xattr.c                         |   8 +-
>  fs/nfsd/filecache.c                         |   4 +-
>  fs/xfs/xfs_buf.c                            |   6 +-
>  fs/xfs/xfs_dquot.c                          |   2 +-
>  fs/xfs/xfs_qm.c                             |   2 +-
>  include/linux/list_lru.h                    |  46 ++-
>  include/linux/memcontrol.h                  |   9 +-
>  include/linux/mmzone.h                      |   2 +
>  include/linux/vm_event_item.h               |   1 +
>  include/linux/zswap.h                       |  27 +-
>  mm/list_lru.c                               |  48 ++-
>  mm/memcontrol.c                             |  20 +-
>  mm/mmzone.c                                 |   1 +
>  mm/shrinker.c                               |   4 +-
>  mm/swap.h                                   |   3 +-
>  mm/swap_state.c                             |  26 +-
>  mm/vmscan.c                                 |  26 +-
>  mm/vmstat.c                                 |   1 +
>  mm/workingset.c                             |   4 +-
>  mm/zswap.c                                  | 430 +++++++++++++++++---
>  tools/testing/selftests/cgroup/test_zswap.c |  74 ++--
>  26 files changed, 625 insertions(+), 149 deletions(-)
>
> --
> 2.34.1
>
  

On Wed, Nov 8, 2023 at 11:46 AM Chris Li <chrisl@kernel.org> wrote:
>
> Hi Nhat,
>
> Sorry for being late to the party. I want to take a look at your patches series.
> However I wasn't able to "git am" your patches series cleanly on current
> mm-stable, mm-unstable or linux tip.
>
> $ git am patches/v5_20231106_nphamcs_workload_specific_and_memory_pressure_driven_zswap_writeback.mbx
> Applying: list_lru: allows explicit memcg and NUMA node selection
> Applying: memcontrol: allows mem_cgroup_iter() to check for onlineness
> Applying: zswap: make shrinking memcg-aware (fix)
> error: patch failed: mm/zswap.c:174
> error: mm/zswap.c: patch does not apply
> Patch failed at 0003 zswap: make shrinking memcg-aware (fix)

Ah that was meant to be a fixlet - so that on top of the original
"zswap: make shrinking memcg-aware" patch. The intention was
to eventually squash it...

But this is getting a bit annoyingly confusing, I admit. I just rebased to
mm-unstable + squashed it all again, then sent one single replacement
patch:

[PATCH v5 3/6 REPLACE] zswap: make shrinking memcg-aware

Let me know if this still fails to apply. If not, I'll send the whole thing
again as v6! My sincerest apologies for the troubles and confusion :(


>
> What is the base of your patches? A git hash or a branch I can pull
> from would be
> nice.
>
> Thanks
>
> Chris
>
> On Mon, Nov 6, 2023 at 10:32 AM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > Changelog:
> > v5:
> >    * Replace reference getting with an rcu_read_lock() section for
> >      zswap lru modifications (suggested by Yosry)
> >    * Add a new prep patch that allows mem_cgroup_iter() to return
> >      online cgroup.
> >    * Add a callback that updates pool->next_shrink when the cgroup is
> >      offlined (suggested by Yosry Ahmed, Johannes Weiner)
> > v4:
> >    * Rename list_lru_add to list_lru_add_obj and __list_lru_add to
> >      list_lru_add (patch 1) (suggested by Johannes Weiner and
> >          Yosry Ahmed)
> >    * Some cleanups on the memcg aware LRU patch (patch 2)
> >      (suggested by Yosry Ahmed)
> >    * Use event interface for the new per-cgroup writeback counters.
> >      (patch 3) (suggested by Yosry Ahmed)
> >    * Abstract zswap's lruvec states and handling into
> >      zswap_lruvec_state (patch 5) (suggested by Yosry Ahmed)
> > v3:
> >    * Add a patch to export per-cgroup zswap writeback counters
> >    * Add a patch to update zswap's kselftest
> >    * Separate the new list_lru functions into its own prep patch
> >    * Do not start from the top of the hierarchy when encounter a memcg
> >      that is not online for the global limit zswap writeback (patch 2)
> >      (suggested by Yosry Ahmed)
> >    * Do not remove the swap entry from list_lru in
> >      __read_swapcache_async() (patch 2) (suggested by Yosry Ahmed)
> >    * Removed a redundant zswap pool getting (patch 2)
> >      (reported by Ryan Roberts)
> >    * Use atomic for the nr_zswap_protected (instead of lruvec's lock)
> >      (patch 5) (suggested by Yosry Ahmed)
> >    * Remove the per-cgroup zswap shrinker knob (patch 5)
> >      (suggested by Yosry Ahmed)
> > v2:
> >    * Fix loongarch compiler errors
> >    * Use pool stats instead of memcg stats when !CONFIG_MEMCG_KEM
> >
> > There are currently several issues with zswap writeback:
> >
> > 1. There is only a single global LRU for zswap, making it impossible to
> >    perform worload-specific shrinking - an memcg under memory pressure
> >    cannot determine which pages in the pool it owns, and often ends up
> >    writing pages from other memcgs. This issue has been previously
> >    observed in practice and mitigated by simply disabling
> >    memcg-initiated shrinking:
> >
> >    https://lore.kernel.org/all/20230530232435.3097106-1-nphamcs@gmail.com/T/#u
> >
> >    But this solution leaves a lot to be desired, as we still do not
> >    have an avenue for an memcg to free up its own memory locked up in
> >    the zswap pool.
> >
> > 2. We only shrink the zswap pool when the user-defined limit is hit.
> >    This means that if we set the limit too high, cold data that are
> >    unlikely to be used again will reside in the pool, wasting precious
> >    memory. It is hard to predict how much zswap space will be needed
> >    ahead of time, as this depends on the workload (specifically, on
> >    factors such as memory access patterns and compressibility of the
> >    memory pages).
> >
> > This patch series solves these issues by separating the global zswap
> > LRU into per-memcg and per-NUMA LRUs, and performs workload-specific
> > (i.e memcg- and NUMA-aware) zswap writeback under memory pressure. The
> > new shrinker does not have any parameter that must be tuned by the
> > user, and can be opted in or out on a per-memcg basis.
> >
> > As a proof of concept, we ran the following synthetic benchmark:
> > build the linux kernel in a memory-limited cgroup, and allocate some
> > cold data in tmpfs to see if the shrinker could write them out and
> > improved the overall performance. Depending on the amount of cold data
> > generated, we observe from 14% to 35% reduction in kernel CPU time used
> > in the kernel builds.
> >
> > Domenico Cerasuolo (3):
> >   zswap: make shrinking memcg-aware
> >   mm: memcg: add per-memcg zswap writeback stat
> >   selftests: cgroup: update per-memcg zswap writeback selftest
> >
> > Nhat Pham (3):
> >   list_lru: allows explicit memcg and NUMA node selection
> >   memcontrol: allows mem_cgroup_iter() to check for onlineness
> >   zswap: shrinks zswap pool based on memory pressure
> >
> >  Documentation/admin-guide/mm/zswap.rst      |   7 +
> >  drivers/android/binder_alloc.c              |   5 +-
> >  fs/dcache.c                                 |   8 +-
> >  fs/gfs2/quota.c                             |   6 +-
> >  fs/inode.c                                  |   4 +-
> >  fs/nfs/nfs42xattr.c                         |   8 +-
> >  fs/nfsd/filecache.c                         |   4 +-
> >  fs/xfs/xfs_buf.c                            |   6 +-
> >  fs/xfs/xfs_dquot.c                          |   2 +-
> >  fs/xfs/xfs_qm.c                             |   2 +-
> >  include/linux/list_lru.h                    |  46 ++-
> >  include/linux/memcontrol.h                  |   9 +-
> >  include/linux/mmzone.h                      |   2 +
> >  include/linux/vm_event_item.h               |   1 +
> >  include/linux/zswap.h                       |  27 +-
> >  mm/list_lru.c                               |  48 ++-
> >  mm/memcontrol.c                             |  20 +-
> >  mm/mmzone.c                                 |   1 +
> >  mm/shrinker.c                               |   4 +-
> >  mm/swap.h                                   |   3 +-
> >  mm/swap_state.c                             |  26 +-
> >  mm/vmscan.c                                 |  26 +-
> >  mm/vmstat.c                                 |   1 +
> >  mm/workingset.c                             |   4 +-
> >  mm/zswap.c                                  | 430 +++++++++++++++++---
> >  tools/testing/selftests/cgroup/test_zswap.c |  74 ++--
> >  26 files changed, 625 insertions(+), 149 deletions(-)
> >
> > --
> > 2.34.1
> >
  

Hi Nhat,

On Wed, Nov 8, 2023 at 1:15 PM Nhat Pham <nphamcs@gmail.com> wrote:
>
> Ah that was meant to be a fixlet - so that on top of the original
> "zswap: make shrinking memcg-aware" patch. The intention was
> to eventually squash it...
>
> But this is getting a bit annoyingly confusing, I admit. I just rebased to
> mm-unstable + squashed it all again, then sent one single replacement
> patch:
>
> [PATCH v5 3/6 REPLACE] zswap: make shrinking memcg-aware

Thank you for the quick response.

Yes, I am able to download your replacement version of patch 3.
Just FYI, I am using "git mailsplit" to split up the mbox into 6
separate patch files.
On mm-unstable, I am able to apply your replacement patch 3 cleanly.
I also need some help on the patch 0005, it does not apply cleanly either.

$ git mailsplit -ozswap-pool-lru
v5_20231106_nphamcs_workload_specific_and_memory_pressure_driven_zswap_writeback.mbx
$ git am patches/zswap-pool-lru/0001
Applying: list_lru: allows explicit memcg and NUMA node selection
$ git am patches/zswap-pool-lru/0002
Applying: memcontrol: allows mem_cgroup_iter() to check for onlineness
$ git am patches/zswap-pool-lru/3.replace
Applying: zswap: make shrinking memcg-aware
$ git am patches/zswap-pool-lru/0004
Applying: mm: memcg: add per-memcg zswap writeback stat
$ git am patches/zswap-pool-lru/0005
Applying: selftests: cgroup: update per-memcg zswap writeback selftest
error: patch failed: tools/testing/selftests/cgroup/test_zswap.c:50
error: tools/testing/selftests/cgroup/test_zswap.c: patch does not apply
Patch failed at 0001 selftests: cgroup: update per-memcg zswap
writeback selftest
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

>
> Let me know if this still fails to apply. If not, I'll send the whole thing
> again as v6! My sincerest apologies for the troubles and confusion :(

No problem at all. Thanks for your help on patch 3.

Chris
  

On Wed, Nov 8, 2023 at 3:12 PM Chris Li <chrisl@kernel.org> wrote:
>
> Hi Nhat,
>
> On Wed, Nov 8, 2023 at 1:15 PM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > Ah that was meant to be a fixlet - so that on top of the original
> > "zswap: make shrinking memcg-aware" patch. The intention was
> > to eventually squash it...
> >
> > But this is getting a bit annoyingly confusing, I admit. I just rebased to
> > mm-unstable + squashed it all again, then sent one single replacement
> > patch:
> >
> > [PATCH v5 3/6 REPLACE] zswap: make shrinking memcg-aware
>
> Thank you for the quick response.
>
> Yes, I am able to download your replacement version of patch 3.
> Just FYI, I am using "git mailsplit" to split up the mbox into 6
> separate patch files.
> On mm-unstable, I am able to apply your replacement patch 3 cleanly.
> I also need some help on the patch 0005, it does not apply cleanly either.
>
> $ git mailsplit -ozswap-pool-lru
> v5_20231106_nphamcs_workload_specific_and_memory_pressure_driven_zswap_writeback.mbx
> $ git am patches/zswap-pool-lru/0001
> Applying: list_lru: allows explicit memcg and NUMA node selection
> $ git am patches/zswap-pool-lru/0002
> Applying: memcontrol: allows mem_cgroup_iter() to check for onlineness
> $ git am patches/zswap-pool-lru/3.replace
> Applying: zswap: make shrinking memcg-aware
> $ git am patches/zswap-pool-lru/0004
> Applying: mm: memcg: add per-memcg zswap writeback stat
> $ git am patches/zswap-pool-lru/0005
> Applying: selftests: cgroup: update per-memcg zswap writeback selftest
> error: patch failed: tools/testing/selftests/cgroup/test_zswap.c:50
> error: tools/testing/selftests/cgroup/test_zswap.c: patch does not apply
> Patch failed at 0001 selftests: cgroup: update per-memcg zswap
> writeback selftest
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> >
> > Let me know if this still fails to apply. If not, I'll send the whole thing
> > again as v6! My sincerest apologies for the troubles and confusion :(
>
> No problem at all. Thanks for your help on patch 3.
>
> Chris

Hmm my guess is that I probably sent this out based on an outdated
mm-unstable. There has since been a new zswap selftest merged
to mm-unstable (written by no other than myself - oh the irony), so
maybe it does not apply cleanly anymore with git am.

I was, however, able to apply the patch with the -3 argument, i.e:

git am -3 ../mbox/0005

This will fall back to the 3-way merge if direct application fails.
And, FWIW, the kselftest still seems to build.

I think you'll have to do the same with the 6th patch as well. My
guess is that on my latest rebase attempt, this was done silent and
automatically, so I did not notice this.

Let me know if this works. Worst case scenario, I can still rebase 'n
resend the patch series :)
  

On Wed, Nov 8, 2023 at 4:28 PM Nhat Pham <nphamcs@gmail.com> wrote:
>
> Hmm my guess is that I probably sent this out based on an outdated
> mm-unstable. There has since been a new zswap selftest merged
> to mm-unstable (written by no other than myself - oh the irony), so
> maybe it does not apply cleanly anymore with git am.

$ git am -3 patches/zswap-pool-lru/0005
Applying: selftests: cgroup: update per-memcg zswap writeback selftest
Using index info to reconstruct a base tree...
M       tools/testing/selftests/cgroup/test_zswap.c
Falling back to patching base and 3-way merge...
Auto-merging tools/testing/selftests/cgroup/test_zswap.c
$ git am -3 patches/zswap-pool-lru/0006
Applying: zswap: shrinks zswap pool based on memory pressure
error: sha1 information is lacking or useless (mm/zswap.c).
error: could not build fake ancestor
Patch failed at 0001 zswap: shrinks zswap pool based on memory pressure
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

I was able to resolve the conflict on patch 6 by hand though. So I am good now.

Thanks

Chris
  

Hi Nhat,

I want want to share the high level feedback we discussed here in the
mailing list as well.

It is my observation that each memcg LRU list can't compare the page
time order with other memcg.
It works great when the leaf level memcg hits the memory limit and you
want to reclaim from that memcg.
It works less well on the global memory pressure you need to reclaim
from all memcg. You kind of have to
scan each all child memcg to find out the best page to shrink from. It
is less effective to get to the most desirable page quickly.

This can benefit from a design similar to MGLRU. This idea is
suggested by Yu Zhao, credit goes to him not me.
In other words, the current patch is similar to the memcg page list
pre MGLRU world. We can have a MRLRU
like per memcg zswap shrink list.


Chris

On Wed, Nov 8, 2023 at 6:10 PM Chris Li <chrisl@kernel.org> wrote:
>
> On Wed, Nov 8, 2023 at 4:28 PM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > Hmm my guess is that I probably sent this out based on an outdated
> > mm-unstable. There has since been a new zswap selftest merged
> > to mm-unstable (written by no other than myself - oh the irony), so
> > maybe it does not apply cleanly anymore with git am.
>
> $ git am -3 patches/zswap-pool-lru/0005
> Applying: selftests: cgroup: update per-memcg zswap writeback selftest
> Using index info to reconstruct a base tree...
> M       tools/testing/selftests/cgroup/test_zswap.c
> Falling back to patching base and 3-way merge...
> Auto-merging tools/testing/selftests/cgroup/test_zswap.c
> $ git am -3 patches/zswap-pool-lru/0006
> Applying: zswap: shrinks zswap pool based on memory pressure
> error: sha1 information is lacking or useless (mm/zswap.c).
> error: could not build fake ancestor
> Patch failed at 0001 zswap: shrinks zswap pool based on memory pressure
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> I was able to resolve the conflict on patch 6 by hand though. So I am good now.
>
> Thanks
>
> Chris
  

On Thu, Nov 16, 2023 at 4:57 PM Chris Li <chrisl@kernel.org> wrote:
>
> Hi Nhat,
>
> I want want to share the high level feedback we discussed here in the
> mailing list as well.
>
> It is my observation that each memcg LRU list can't compare the page
> time order with other memcg.
> It works great when the leaf level memcg hits the memory limit and you
> want to reclaim from that memcg.
> It works less well on the global memory pressure you need to reclaim
> from all memcg. You kind of have to
> scan each all child memcg to find out the best page to shrink from. It
> is less effective to get to the most desirable page quickly.
>
> This can benefit from a design similar to MGLRU. This idea is
> suggested by Yu Zhao, credit goes to him not me.
> In other words, the current patch is similar to the memcg page list
> pre MGLRU world. We can have a MRLRU
> like per memcg zswap shrink list.

I was gonna summarize the points myself :P But thanks for doing this.
It's your idea so you're more qualified to explain this anyway ;)

I absolutely agree that having a generation-aware cgroup-aware
NUMA-aware LRU is the future way to go. Currently, IIUC, the reclaim logic
selects cgroups in a round-robin-ish manner. It's "fair" in this perspective,
but I also think it's not ideal. As we have discussed, the current list_lru
infrastructure only take into account intra-cgroup relative recency, not
inter-cgroup relative recency. The recently proposed time-based zswap
reclaim mechanism will provide us with a source of information, but the
overhead of using this might be too high - and it's very zswap-specific.

Maybe after this, we should improve zswap reclaim (and perhaps all
list_lru users) by adding generations to list_lru then take generations
into account in the vmscan code. This patch series could be merged
as-is, and once we make list_lru generation-aware, zswap shrinker
will automagically be improved (along with all other list_lru/shrinker
users).

I don't know enough about the current design of MGLRU to comment
too much further, but let me know if this makes sense, and if you have
objections/other ideas.

And if you have other documentations for MGLRU than its code, could
you please let me know? I'm struggling to find more details about this.


>
>
> Chris
>
> On Wed, Nov 8, 2023 at 6:10 PM Chris Li <chrisl@kernel.org> wrote:
> >
> > On Wed, Nov 8, 2023 at 4:28 PM Nhat Pham <nphamcs@gmail.com> wrote:
> > >
> > > Hmm my guess is that I probably sent this out based on an outdated
> > > mm-unstable. There has since been a new zswap selftest merged
> > > to mm-unstable (written by no other than myself - oh the irony), so
> > > maybe it does not apply cleanly anymore with git am.
> >
> > $ git am -3 patches/zswap-pool-lru/0005
> > Applying: selftests: cgroup: update per-memcg zswap writeback selftest
> > Using index info to reconstruct a base tree...
> > M       tools/testing/selftests/cgroup/test_zswap.c
> > Falling back to patching base and 3-way merge...
> > Auto-merging tools/testing/selftests/cgroup/test_zswap.c
> > $ git am -3 patches/zswap-pool-lru/0006
> > Applying: zswap: shrinks zswap pool based on memory pressure
> > error: sha1 information is lacking or useless (mm/zswap.c).
> > error: could not build fake ancestor
> > Patch failed at 0001 zswap: shrinks zswap pool based on memory pressure
> > hint: Use 'git am --show-current-patch=diff' to see the failed patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> > I was able to resolve the conflict on patch 6 by hand though. So I am good now.
> >
> > Thanks
> >
> > Chris
  

On Fri, Nov 17, 2023 at 8:23 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Thu, Nov 16, 2023 at 4:57 PM Chris Li <chrisl@kernel.org> wrote:
> >
> > Hi Nhat,
> >
> > I want want to share the high level feedback we discussed here in the
> > mailing list as well.
> >
> > It is my observation that each memcg LRU list can't compare the page
> > time order with other memcg.
> > It works great when the leaf level memcg hits the memory limit and you
> > want to reclaim from that memcg.
> > It works less well on the global memory pressure you need to reclaim
> > from all memcg. You kind of have to
> > scan each all child memcg to find out the best page to shrink from. It
> > is less effective to get to the most desirable page quickly.
> >
> > This can benefit from a design similar to MGLRU. This idea is
> > suggested by Yu Zhao, credit goes to him not me.
> > In other words, the current patch is similar to the memcg page list
> > pre MGLRU world. We can have a MRLRU
> > like per memcg zswap shrink list.
>
> I was gonna summarize the points myself :P But thanks for doing this.
> It's your idea so you're more qualified to explain this anyway ;)
>
> I absolutely agree that having a generation-aware cgroup-aware
> NUMA-aware LRU is the future way to go. Currently, IIUC, the reclaim logic
> selects cgroups in a round-robin-ish manner. It's "fair" in this perspective,
> but I also think it's not ideal. As we have discussed, the current list_lru
> infrastructure only take into account intra-cgroup relative recency, not
> inter-cgroup relative recency. The recently proposed time-based zswap
> reclaim mechanism will provide us with a source of information, but the
> overhead of using this might be too high - and it's very zswap-specific.
>
> Maybe after this, we should improve zswap reclaim (and perhaps all
> list_lru users) by adding generations to list_lru then take generations
> into account in the vmscan code. This patch series could be merged
> as-is, and once we make list_lru generation-aware, zswap shrinker
> will automagically be improved (along with all other list_lru/shrinker
> users).
>
> I don't know enough about the current design of MGLRU to comment
> too much further, but let me know if this makes sense, and if you have
> objections/other ideas.
>
> And if you have other documentations for MGLRU than its code, could
> you please let me know? I'm struggling to find more details about this.
>

This could be a good place to start:
https://www.youtube.com/watch?v=9HvJfN21H9Y
  

On Fri, Nov 17, 2023 at 8:23 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Thu, Nov 16, 2023 at 4:57 PM Chris Li <chrisl@kernel.org> wrote:
> >
> > Hi Nhat,
> >
> > I want want to share the high level feedback we discussed here in the
> > mailing list as well.
> >
> > It is my observation that each memcg LRU list can't compare the page
> > time order with other memcg.
> > It works great when the leaf level memcg hits the memory limit and you
> > want to reclaim from that memcg.
> > It works less well on the global memory pressure you need to reclaim
> > from all memcg. You kind of have to
> > scan each all child memcg to find out the best page to shrink from. It
> > is less effective to get to the most desirable page quickly.
> >
> > This can benefit from a design similar to MGLRU. This idea is
> > suggested by Yu Zhao, credit goes to him not me.
> > In other words, the current patch is similar to the memcg page list
> > pre MGLRU world. We can have a MRLRU
> > like per memcg zswap shrink list.
>
> I was gonna summarize the points myself :P But thanks for doing this.
> It's your idea so you're more qualified to explain this anyway ;)

The MGLRU like shrinker was Zhao Yu's idea. I just observe the problem.

>
> I absolutely agree that having a generation-aware cgroup-aware
> NUMA-aware LRU is the future way to go. Currently, IIUC, the reclaim logic
> selects cgroups in a round-robin-ish manner. It's "fair" in this perspective,
> but I also think it's not ideal. As we have discussed, the current list_lru
> infrastructure only take into account intra-cgroup relative recency, not
> inter-cgroup relative recency. The recently proposed time-based zswap
> reclaim mechanism will provide us with a source of information, but the
> overhead of using this might be too high - and it's very zswap-specific.

I don't mind it is zswap-specific, as long as it is effective.
The overhead has two folds:
1) memory overhead on storing timestamps on per compressed page.
2) cpu overhead for reading timestamps.
Using MGLRU likely have advantage over time stamps on both memory and
cpu. The generation can use fewer bits and doesn't require reading
time on every page.

> Maybe after this, we should improve zswap reclaim (and perhaps all
> list_lru users) by adding generations to list_lru then take generations
> into account in the vmscan code. This patch series could be merged

One high level idea is that we can get the page generation in the
MGLRU before it gets into zswap. Just retain the generation into the
zpool LRU somehow.

> as-is, and once we make list_lru generation-aware, zswap shrinker
> will automagically be improved (along with all other list_lru/shrinker
> users).

I don't think it will automatically improve, you will need to rewrite
a lot of code in the shrinker as well to best use MGLRU zpool.

>
> I don't know enough about the current design of MGLRU to comment
> too much further, but let me know if this makes sense, and if you have
> objections/other ideas.

Taking the step by step approach is fine by me as long as we are
making steady progress towards the better end goal.

>
> And if you have other documentations for MGLRU than its code, could
> you please let me know? I'm struggling to find more details about this.

I would need to learn MGLRU myself. We can share and compare notes
when we get to it.

Chris