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);
> + }
> }
>
>
