Re: [PATCH] mm/vmscan: check references from all memcgs for swapbacked memory

[Yosry Ahmed <yosryahmed@google.com>]

On Tue, Oct 4, 2022 at 4:34 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> During page/folio reclaim, we check folio is referenced using
> folio_referenced() to avoid reclaiming folios that have been recently
> accessed (hot memory). The ratinale is that this memory is likely to be

s/ratinale/rationale

> accessed soon, and hence reclaiming it will cause a refault.
>
> For memcg reclaim, we pass in sc->target_mem_cgroup to
> folio_referenced(), which means we only check accesses to the folio
> from processes in the subtree of the target memcg. This behavior was
> originally introduced by commit bed7161a519a ("Memory controller: make
> page_referenced() cgroup aware") a long time ago. Back then, refaulted
> pages would get charged to the memcg of the process that was faulting them
> in. It made sense to only consider accesses coming from processes in the
> subtree of target_mem_cgroup. If a page was charged to memcg A but only
> being accessed by a sibling memcg B, we would reclaim it if memcg A is
> under pressure. memcg B can then fault it back in and get charged for it
> appropriately.
>
> Today, this behavior still makes sense for file pages. However, unlike
> file pages, when swapbacked pages are refaulted they are charged to the
> memcg that was originally charged for them during swapout. Which
> means that if a swapbacked page is charged to memcg A but only used by
> memcg B, and we reclaim it when memcg A is under pressure, it would
> simply be faulted back in and charged again to memcg A once memcg B
> accesses it. In that sense, accesses from all memcgs matter equally when
> considering if a swapbacked page/folio is a viable reclaim target.
>
> Add folio_referenced_memcg() which decides what memcg we should pass to
> folio_referenced() based on the folio type, and includes an elaborate
> comment about why we should do so. This should help reclaim make better
> decision and reduce refaults when reclaiming swapbacked memory that is

s/decision/decisions

> used by multiple memcgs.
>
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> ---
>  mm/vmscan.c | 38 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c5a4bff11da6..f9fa0f9287e5 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1443,14 +1443,43 @@ enum folio_references {
>         FOLIOREF_ACTIVATE,
>  };
>
> +/* What memcg should we pass to folio_referenced()? */
> +static struct mem_cgroup *folio_referenced_memcg(struct folio *folio,
> +                                                struct mem_cgroup *target_memcg)
> +{
> +       /*
> +        * We check references to folios to make sure we don't reclaim hot
> +        * folios that are likely to be refaulted soon. We pass a memcg to
> +        * folio_referenced() to only check references coming from processes in
> +        * that memcg's subtree.
> +        *
> +        * For file folios, we only consider references from processes in the
> +        * subtree of the target memcg. If a folio is charged to
> +        * memcg A but is only referenced by processes in memcg B, we reclaim it
> +        * if memcg A is under pressure. If it is later accessed by memcg B it
> +        * will be faulted back in and charged to memcg B. For memcg A, this is
> +        * called memory that should be reclaimed.

s/called/cold

> +        *
> +        * On the other hand, when swapbacked folios are faulted in, they get
> +        * charged to the memcg that was originally charged for them at the time
> +        * of swapping out. This means that if a folio that is charged to
> +        * memcg A gets swapped out, it will get charged back to A when *any*
> +        * memcg accesses it. In that sense, we need to consider references from
> +        * *all* processes when considering whether to reclaim a swapbacked
> +        * folio.
> +        */
> +       return folio_test_swapbacked(folio) ? NULL : target_memcg;
> +}
> +
>  static enum folio_references folio_check_references(struct folio *folio,
>                                                   struct scan_control *sc)
>  {
>         int referenced_ptes, referenced_folio;
>         unsigned long vm_flags;
> +       struct mem_cgroup *memcg = folio_referenced_memcg(folio,
> +                                               sc->target_mem_cgroup);
>
> -       referenced_ptes = folio_referenced(folio, 1, sc->target_mem_cgroup,
> -                                          &vm_flags);
> +       referenced_ptes = folio_referenced(folio, 1, memcg, &vm_flags);
>         referenced_folio = folio_test_clear_referenced(folio);
>
>         /*
> @@ -2581,6 +2610,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
>
>         while (!list_empty(&l_hold)) {
>                 struct folio *folio;
> +               struct mem_cgroup *memcg;
>
>                 cond_resched();
>                 folio = lru_to_folio(&l_hold);
> @@ -2600,8 +2630,8 @@ static void shrink_active_list(unsigned long nr_to_scan,
>                 }
>
>                 /* Referenced or rmap lock contention: rotate */
> -               if (folio_referenced(folio, 0, sc->target_mem_cgroup,
> -                                    &vm_flags) != 0) {
> +               memcg = folio_referenced_memcg(folio, sc->target_mem_cgroup);
> +               if (folio_referenced(folio, 0, memcg, &vm_flags) != 0) {
>                         /*
>                          * Identify referenced, file-backed active folios and
>                          * give them one more trip around the active list. So
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>