Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230522112058.2965866-1-talumbau@google.com> <20230522112058.2965866-4-talumbau@google.com>
In-Reply-To: <20230522112058.2965866-4-talumbau@google.com>
From:   Yuanchu Xie <yuanchu@google.com>
Date:   Thu, 1 Jun 2023 16:02:58 +0800
Message-ID: <CAJj2-QHSBPNhuM2EY3Hki=-Xq_H-k2zutm7xJGCP+fCqfCqt1g@mail.gmail.com>
Subject: Re: [PATCH mm-unstable 4/4] mm: multi-gen LRU: cleanup lru_gen_test_recent()
To:     "T.J. Alumbaugh" <talumbau@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Yu Zhao <yuzhao@google.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-mm@google.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, May 22, 2023 at 7:21=E2=80=AFPM T.J. Alumbaugh <talumbau@google.com=
> wrote:
>
> Avoid passing memcg* and pglist_data* to lru_gen_test_recent()
> since we only use the lruvec anyway.
>
> Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
> ---
>  mm/workingset.c | 46 ++++++++++++++++------------------------------
>  1 file changed, 16 insertions(+), 30 deletions(-)
>
> diff --git a/mm/workingset.c b/mm/workingset.c
> index 90ae785d4c9c..5796e927e6d7 100644
> --- a/mm/workingset.c
> +++ b/mm/workingset.c
> @@ -257,59 +257,46 @@ static void *lru_gen_eviction(struct folio *folio)
>
>  /*
>   * Tests if the shadow entry is for a folio that was recently evicted.
> - * Fills in @memcgid, @pglist_data, @token, @workingset with the values
> - * unpacked from shadow.
> + * Fills in @lruvec, @token, @workingset with the values unpacked from s=
hadow.
>   */
> -static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
> -               struct pglist_data **pgdat, unsigned long *token, bool *w=
orkingset)
> +static bool lru_gen_test_recent(void *shadow, bool file, struct lruvec *=
*lruvec,
> +                               unsigned long *token, bool *workingset)
>  {
> -       struct mem_cgroup *eviction_memcg;
> -       struct lruvec *lruvec;
> -       struct lru_gen_folio *lrugen;
> +       int memcg_id;
>         unsigned long min_seq;
> +       struct mem_cgroup *memcg;
> +       struct pglist_data *pgdat;
>
> -       unpack_shadow(shadow, memcgid, pgdat, token, workingset);
> -       eviction_memcg =3D mem_cgroup_from_id(*memcgid);
> +       unpack_shadow(shadow, &memcg_id, &pgdat, token, workingset);
>
> -       lruvec =3D mem_cgroup_lruvec(eviction_memcg, *pgdat);
> -       lrugen =3D &lruvec->lrugen;
> +       memcg =3D mem_cgroup_from_id(memcg_id);
> +       *lruvec =3D mem_cgroup_lruvec(memcg, pgdat);
>
> -       min_seq =3D READ_ONCE(lrugen->min_seq[file]);
> +       min_seq =3D READ_ONCE((*lruvec)->lrugen.min_seq[file]);
>         return (*token >> LRU_REFS_WIDTH) =3D=3D (min_seq & (EVICTION_MAS=
K >> LRU_REFS_WIDTH));
>  }
>
>  static void lru_gen_refault(struct folio *folio, void *shadow)
>  {
>         int hist, tier, refs;
> -       int memcg_id;
>         bool workingset;
>         unsigned long token;
> -       unsigned long min_seq;
>         struct lruvec *lruvec;
>         struct lru_gen_folio *lrugen;
> -       struct mem_cgroup *memcg;
> -       struct pglist_data *pgdat;
>         int type =3D folio_is_file_lru(folio);
>         int delta =3D folio_nr_pages(folio);
>
>         rcu_read_lock();
>
> -       if (!lru_gen_test_recent(shadow, type, &memcg_id, &pgdat, &token,
> -                       &workingset))
> +       if (!lru_gen_test_recent(shadow, type, &lruvec, &token, &workings=
et))
>                 goto unlock;
>
> -       memcg =3D folio_memcg_rcu(folio);
> -       if (memcg_id !=3D mem_cgroup_id(memcg))
> +       if (lruvec !=3D folio_lruvec(folio))
>                 goto unlock;
>
> -       if (pgdat !=3D folio_pgdat(folio))
> -               goto unlock;
> -
> -       lruvec =3D mem_cgroup_lruvec(memcg, pgdat);
>         lrugen =3D &lruvec->lrugen;
> -       min_seq =3D READ_ONCE(lrugen->min_seq[type]);
>
> -       hist =3D lru_hist_from_seq(min_seq);
> +       hist =3D lru_hist_from_seq(READ_ONCE(lrugen->min_seq[type]));
>         /* see the comment in folio_lru_refs() */
>         refs =3D (token & (BIT(LRU_REFS_WIDTH) - 1)) + workingset;
>         tier =3D lru_tier_from_refs(refs);
> @@ -339,8 +326,8 @@ static void *lru_gen_eviction(struct folio *folio)
>         return NULL;
>  }
>
> -static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
> -               struct pglist_data **pgdat, unsigned long *token, bool *w=
orkingset)
> +static bool lru_gen_test_recent(void *shadow, bool file, struct lruvec *=
*lruvec,
> +                               unsigned long *token, bool *workingset)
>  {
>         return false;
>  }
> @@ -435,8 +422,7 @@ bool workingset_test_recent(void *shadow, bool file, =
bool *workingset)
>         unsigned long eviction;
>
>         if (lru_gen_enabled())
> -               return lru_gen_test_recent(shadow, file, &memcgid, &pgdat=
, &eviction,
> -                       workingset);
> +               return lru_gen_test_recent(shadow, file, &eviction_lruvec=
, &eviction, workingset);
>
>         unpack_shadow(shadow, &memcgid, &pgdat, &eviction, workingset);
>         eviction <<=3D bucket_order;
> --
> 2.40.1.698.g37aff9b760-goog
>
Reviewed-by: Yuanchu Xie <yuanchu@google.com>