On 1/15/25 18:55, Matthew Wilcox wrote:
On Wed, Jan 15, 2025 at 06:06:25AM -0600, Donet Tom wrote:
An MGLRU page cache page is eligible for promotion when:
1. Memory Tiering and pagecache_promotion_enabled are enabled
2. It resides in a lower memory tier.
3. It is referenced.
4. It is part of the working set.
5. It belongs to the active list.
For MGLRU, the youngest generation and the youngest generation - 1
are treated as the active list.
... why do you only promote folios if they belong to a memcg?
I missed adding non memcg folios for promotion . I will add them too.
+promo_candid:
+ if (!folio_test_isolated(folio) &&
+ (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&
+ numa_pagecache_promotion_enabled) {
+ memcg = folio_memcg(folio);
+ if (memcg) {
+ 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);
+ }
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?
} 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);
+ }
}