Re: [RFC] mm/swap.c: Enable promotion of unmapped MGLRU page cache pages

[Matthew Wilcox <willy@infradead.org>]

On Thu, Jan 16, 2025 at 04:40:21PM +0530, Donet Tom wrote:
> > Should there be an 'else' clause here?  Or should it be:
> > 
> > 		lruvec = mem_cgroup_lruvec(memcg, folio_pgdat(folio));
> > 		gen = folio_lru_gen(folio);
> > 		if ((gen < MAX_NR_GENS) && lru_gen_is_active(lruvec, gen))
> > 			promotion_candidate(folio);
> > 
> Yes, there should be an else block to handle the promotion of non memcg
> folios.
> 
> Does this approach look correct to you?

No.  Look at the implementation of folio_memcg(), and
mem_cgroup_lruvec() for that matter.

>         } while (!try_cmpxchg(&folio->flags, &old_flags, new_flags));
> +
> +promo_candid:
> +       if (!folio_test_isolated(folio) &&
> +               (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING)
> &&
> +               numa_pagecache_promotion_enabled) {
> +               pgdat = folio_pgdat(folio);
> +               gen = folio_lru_gen(folio);
> +
> +               if (mem_cgroup_disabled())
> +                       lruvec = &pgdat->__lruvec;
> +               else {
> +                       memcg = folio_memcg(folio);
> +                       lruvec = mem_cgroup_lruvec(memcg, pgdat);
> +               }
> +
> +               if ((gen < MAX_NR_GENS) && lru_gen_is_active(lruvec, gen))
> +                       promotion_candidate(folio);
> +       }
>  }
> 
>