Re: [PATCH] mm: deal with active/inactive inversion in lru_gen_is_active

[Yu Zhao]

On Mon, Sep 30, 2024 at 12:39 AM zhaoyang.huang
<zhaoyang.huang@xxxxxxxxxx> wrote:
>
> From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
>
> All gens will be deemed as active by lru_gen_is_active when there are
> only two generations within the node, which could have the folios
> to be active in the belowing scenarios. This commit would like to
> solve this by judging if numbers of file type gens greater than two
> since anon gens could be remains as 3

> when swap is constrained

Technically, yes. But this patch doesn't detect whether swap is
constrained or not. Even if it did, I don't think it would matter
because no one should care about active/inactive anon when swap is
permanently disabled. One could argue the temporary case, but then why
would/should we care about a transient condition if that's the problem
you are trying to solve?

Most importantly, this breaks the proactive aging via the debugfs
interface, which can override swappiness (swap being disabled).

> and file
> type seq advanced by 1.
>
> 1. New folios after migration done.
> 2. Dropped folios from none-reclaiming context(drop_cache, madvise)
>    when they are failed of being reclaimed and go back to lru.
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> ---
>  include/linux/mm_inline.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index 6f801c7b36e2..5e4017dbec96 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -165,11 +165,16 @@ static inline int folio_lru_gen(struct folio *folio)
>  static inline bool lru_gen_is_active(struct lruvec *lruvec, int gen)
>  {
>         unsigned long max_seq = lruvec->lrugen.max_seq;
> +       unsigned long min_seq = lruvec->lrugen.min_seq[LRU_GEN_FILE];
>
>         VM_WARN_ON_ONCE(gen >= MAX_NR_GENS);
>
> -       /* see the comment on MIN_NR_GENS */
> -       return gen == lru_gen_from_seq(max_seq) || gen == lru_gen_from_seq(max_seq - 1);
> +       /* see the comment on MIN_NR_GENS
> +        * judge if there is active/inactive inversion by the number of file
> +        * type gens.
> +        */
> +       return gen == lru_gen_from_seq(max_seq) ||
> +               max_seq - min_seq >= 2 ? gen == lru_gen_from_seq(max_seq - 1) : 0;
>  }
>
>  static inline void lru_gen_update_size(struct lruvec *lruvec, struct folio *folio,
> --
> 2.25.1
>
>