[v2,0/5] mm: memcg: subtree stats flushing and thresholds

Message ID 20231010032117.1577496-1-yosryahmed@google.com
Headers
Series mm: memcg: subtree stats flushing and thresholds |

Message

Yosry Ahmed Oct. 10, 2023, 3:21 a.m. UTC
  This series attempts to address shortages in today's approach for memcg
stats flushing, namely occasionally stale or expensive stat reads. The
series does so by changing the threshold that we use to decide whether
to trigger a flush to be per memcg instead of global (patch 3), and then
changing flushing to be per memcg (i.e. subtree flushes) instead of
global (patch 5).

Patch 3 & 5 are the core of the series, and they include more details
and testing results. The rest are either cleanups or prep work.

This series replaces the "memcg: more sophisticated stats flushing"
series [1], which also replaces another series, in a long list of
attempts to improve memcg stats flushing. It is not a new version of
the same patchset as it is a completely different approach. This is
based on collected feedback from discussions on lkml in all previous
attempts. Hopefully, this is the final attempt.

[1]https://lore.kernel.org/lkml/20230913073846.1528938-1-yosryahmed@google.com/

v1 -> v2:
- Fixed compilation error reported by the kernel robot in patch 4, also
  added a missing rcu_read_unlock().
- More testing results in the commit message of patch 3.

Yosry Ahmed (5):
  mm: memcg: change flush_next_time to flush_last_time
  mm: memcg: move vmstats structs definition above flushing code
  mm: memcg: make stats flushing threshold per-memcg
  mm: workingset: move the stats flush into workingset_test_recent()
  mm: memcg: restore subtree stats flushing

 include/linux/memcontrol.h |   8 +-
 mm/memcontrol.c            | 269 +++++++++++++++++++++----------------
 mm/vmscan.c                |   2 +-
 mm/workingset.c            |  42 ++++--
 4 files changed, 185 insertions(+), 136 deletions(-)
  

Comments

Domenico Cerasuolo Oct. 10, 2023, 4:48 p.m. UTC | #1
Il giorno mar 10 ott 2023 alle ore 05:21 Yosry Ahmed
<yosryahmed@google.com> ha scritto:
>
> This series attempts to address shortages in today's approach for memcg
> stats flushing, namely occasionally stale or expensive stat reads. The
> series does so by changing the threshold that we use to decide whether
> to trigger a flush to be per memcg instead of global (patch 3), and then
> changing flushing to be per memcg (i.e. subtree flushes) instead of
> global (patch 5).
>
> Patch 3 & 5 are the core of the series, and they include more details
> and testing results. The rest are either cleanups or prep work.
>
> This series replaces the "memcg: more sophisticated stats flushing"
> series [1], which also replaces another series, in a long list of
> attempts to improve memcg stats flushing. It is not a new version of
> the same patchset as it is a completely different approach. This is
> based on collected feedback from discussions on lkml in all previous
> attempts. Hopefully, this is the final attempt.
>
> [1]https://lore.kernel.org/lkml/20230913073846.1528938-1-yosryahmed@google.com/
>
> v1 -> v2:
> - Fixed compilation error reported by the kernel robot in patch 4, also
>   added a missing rcu_read_unlock().
> - More testing results in the commit message of patch 3.
>
> Yosry Ahmed (5):
>   mm: memcg: change flush_next_time to flush_last_time
>   mm: memcg: move vmstats structs definition above flushing code
>   mm: memcg: make stats flushing threshold per-memcg
>   mm: workingset: move the stats flush into workingset_test_recent()
>   mm: memcg: restore subtree stats flushing
>
>  include/linux/memcontrol.h |   8 +-
>  mm/memcontrol.c            | 269 +++++++++++++++++++++----------------
>  mm/vmscan.c                |   2 +-
>  mm/workingset.c            |  42 ++++--
>  4 files changed, 185 insertions(+), 136 deletions(-)
>
> --
> 2.42.0.609.gbb76f46606-goog
>
>

Hi Yosry,

thanks for this series! We backported it on a 5.19-based kernel and ran it on a
machine for almost a week now. The goal was to fix a CPU utilization regression
caused by memory stats readings, it seems that this series was the last bit
needed to completely fix it and bring CPU utilization to 5.12 levels.

FWIW,

Tested-by: Domenico Cerasuolo <cerasuolodomenico@gmail.com>
  
Yosry Ahmed Oct. 10, 2023, 7:01 p.m. UTC | #2
On Tue, Oct 10, 2023 at 9:48 AM domenico cerasuolo
<cerasuolodomenico@gmail.com> wrote:
>
> Il giorno mar 10 ott 2023 alle ore 05:21 Yosry Ahmed
> <yosryahmed@google.com> ha scritto:
> >
> > This series attempts to address shortages in today's approach for memcg
> > stats flushing, namely occasionally stale or expensive stat reads. The
> > series does so by changing the threshold that we use to decide whether
> > to trigger a flush to be per memcg instead of global (patch 3), and then
> > changing flushing to be per memcg (i.e. subtree flushes) instead of
> > global (patch 5).
> >
> > Patch 3 & 5 are the core of the series, and they include more details
> > and testing results. The rest are either cleanups or prep work.
> >
> > This series replaces the "memcg: more sophisticated stats flushing"
> > series [1], which also replaces another series, in a long list of
> > attempts to improve memcg stats flushing. It is not a new version of
> > the same patchset as it is a completely different approach. This is
> > based on collected feedback from discussions on lkml in all previous
> > attempts. Hopefully, this is the final attempt.
> >
> > [1]https://lore.kernel.org/lkml/20230913073846.1528938-1-yosryahmed@google.com/
> >
> > v1 -> v2:
> > - Fixed compilation error reported by the kernel robot in patch 4, also
> >   added a missing rcu_read_unlock().
> > - More testing results in the commit message of patch 3.
> >
> > Yosry Ahmed (5):
> >   mm: memcg: change flush_next_time to flush_last_time
> >   mm: memcg: move vmstats structs definition above flushing code
> >   mm: memcg: make stats flushing threshold per-memcg
> >   mm: workingset: move the stats flush into workingset_test_recent()
> >   mm: memcg: restore subtree stats flushing
> >
> >  include/linux/memcontrol.h |   8 +-
> >  mm/memcontrol.c            | 269 +++++++++++++++++++++----------------
> >  mm/vmscan.c                |   2 +-
> >  mm/workingset.c            |  42 ++++--
> >  4 files changed, 185 insertions(+), 136 deletions(-)
> >
> > --
> > 2.42.0.609.gbb76f46606-goog
> >
> >
>
> Hi Yosry,
>
> thanks for this series! We backported it on a 5.19-based kernel and ran it on a
> machine for almost a week now. The goal was to fix a CPU utilization regression
> caused by memory stats readings, it seems that this series was the last bit
> needed to completely fix it and bring CPU utilization to 5.12 levels.
>
> FWIW,
>
> Tested-by: Domenico Cerasuolo <cerasuolodomenico@gmail.com>

That's awesome. Thanks for the testing!
  
Andrew Morton Oct. 18, 2023, 9:12 p.m. UTC | #3
On Tue, 10 Oct 2023 03:21:11 +0000 Yosry Ahmed <yosryahmed@google.com> wrote:

> This series attempts to address shortages in today's approach for memcg
> stats flushing, namely occasionally stale or expensive stat reads. The
> series does so by changing the threshold that we use to decide whether
> to trigger a flush to be per memcg instead of global (patch 3), and then
> changing flushing to be per memcg (i.e. subtree flushes) instead of
> global (patch 5).
> 
> Patch 3 & 5 are the core of the series, and they include more details
> and testing results. The rest are either cleanups or prep work.
> 
> This series replaces the "memcg: more sophisticated stats flushing"
> series [1], which also replaces another series, in a long list of
> attempts to improve memcg stats flushing. It is not a new version of
> the same patchset as it is a completely different approach. This is
> based on collected feedback from discussions on lkml in all previous
> attempts. Hopefully, this is the final attempt.

Seems that Shakeel's performance concerns have largely been set aside. 
It would be good to have some affirmative input on this patchset from
the memcg developers, please?