linux-kernel - Re: [PATCH 2/2] mm/mglru: reset page lru tier bits when activating

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOUHufb2nJ4-qEWrS_d0X_8FbLKR-+=OC3yNh1ExthKXiYYKHQ@mail.gmail.com>
Date: Tue, 15 Oct 2024 22:55:23 -0600
From: Yu Zhao <yuzhao@...gle.com>
To: Wei Xu <weixugc@...gle.com>
Cc: Brian Geffon <bgeffon@...gle.com>, Jan Alexander Steffens <heftig@...hlinux.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, Suleiman Souhlal <suleiman@...gle.com>, 
	Axel Rasmussen <axelrasmussen@...gle.com>, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] mm/mglru: reset page lru tier bits when activating

On Mon, Oct 14, 2024 at 4:12 PM Wei Xu <weixugc@...gle.com> wrote:
>
> folio_activate() calls lru_gen_add_folio() to move the folio to the
> youngest generation.  But unlike folio_update_gen()/folio_inc_gen(),
> lru_gen_add_folio() doesn't reset the folio lru tier bits
> (LRU_REFS_MASK | LRU_REFS_FLAGS).  Fix this inconsistency in
> lru_gen_add_folio() when activating a folio.
>
> Fixes: 018ee47f1489 ("mm: multi-gen LRU: exploit locality in rmap")
> Signed-off-by: Wei Xu <weixugc@...gle.com>
> ---
>  include/linux/mm_inline.h | 5 ++++-
>  include/linux/mmzone.h    | 2 ++
>  mm/vmscan.c               | 2 --
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index 6f801c7b36e2..87580e8363ef 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -222,6 +222,7 @@ static inline bool lru_gen_add_folio(struct lruvec *lruvec, struct folio *folio,
>  {
>         unsigned long seq;
>         unsigned long flags;
> +       unsigned long mask;
>         int gen = folio_lru_gen(folio);
>         int type = folio_is_file_lru(folio);
>         int zone = folio_zonenum(folio);
> @@ -257,7 +258,9 @@ static inline bool lru_gen_add_folio(struct lruvec *lruvec, struct folio *folio,
>         gen = lru_gen_from_seq(seq);
>         flags = (gen + 1UL) << LRU_GEN_PGOFF;
>         /* see the comment on MIN_NR_GENS about PG_active */
> -       set_mask_bits(&folio->flags, LRU_GEN_MASK | BIT(PG_active), flags);
> +       mask = LRU_GEN_MASK | BIT(PG_active);
> +       mask |= folio_test_active(folio) ? (LRU_REFS_MASK | LRU_REFS_FLAGS) : 0;

We shouldn't clear PG_workingset here because it can affect PSI
accounting, if the activation is due to workingset refault.

Also, nit:
  mask = LRU_GEN_MASK;
  if (folio_test_active(folio))
    mask |= LRU_REFS_MASK | BIT(PG_active) | BIT(PG_referenced);

> +       set_mask_bits(&folio->flags, mask, flags);
>
>         lru_gen_update_size(lruvec, folio, -1, gen);
>         /* for folio_rotate_reclaimable() */
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 17506e4a2835..96dea31fb211 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -403,6 +403,8 @@ enum {
>         NR_LRU_GEN_CAPS
>  };
>
> +#define LRU_REFS_FLAGS         (BIT(PG_referenced) | BIT(PG_workingset))
> +
>  #define MIN_LRU_BATCH          BITS_PER_LONG
>  #define MAX_LRU_BATCH          (MIN_LRU_BATCH * 64)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 9d1e1c4e383d..907262ebaef8 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2601,8 +2601,6 @@ static bool should_clear_pmd_young(void)
>   *                          shorthand helpers
>   ******************************************************************************/
>
> -#define LRU_REFS_FLAGS (BIT(PG_referenced) | BIT(PG_workingset))
> -
>  #define DEFINE_MAX_SEQ(lruvec)                                         \
>         unsigned long max_seq = READ_ONCE((lruvec)->lrugen.max_seq)
>
> --
> 2.47.0.rc1.288.g06298d1525-goog
>
>