On 4/4/25 3:38 PM, Johannes Weiner wrote:
On Fri, Apr 04, 2025 at 02:55:35PM -0400, Waiman Long wrote:
On 4/4/25 2:13 PM, Johannes Weiner wrote:
- Waiman points out that the weirdness is seeing low events without having a low configured. Eh, this isn't really true with recursive propagation; you may or may not have an elow depending on parental configuration and sibling behavior.
Do you mind if we just don't update the low event count if low isn't set, but leave the rest the same like
What's the motivation for doing anything beyond the skip-on-!usage?
It is to avoid making further change. I am fine with modifying the test to allow low event even when low isn't set.
@@ -659,21 +659,25 @@ static inline bool mem_cgroup_unprotected(struct mem_cgro> static inline bool mem_cgroup_below_low(struct mem_cgroup *target, struct mem_cgroup *memcg) { + unsigned long elow;
- if (mem_cgroup_unprotected(target, memcg)) return false;
- return READ_ONCE(memcg->memory.elow) >= - page_counter_read(&memcg->memory); + elow = READ_ONCE(memcg->memory.elow); + return elow && (page_counter_read(&memcg->memory) <= elow); }
static inline bool mem_cgroup_below_min(struct mem_cgroup *target, struct mem_cgroup *memcg) { + unsigned long emin;
- if (mem_cgroup_unprotected(target, memcg)) return false;
- return READ_ONCE(memcg->memory.emin) >= - page_counter_read(&memcg->memory); + emin = READ_ONCE(memcg->memory.emin); + return emin && (page_counter_read(&memcg->memory) <= emin); }
This still redefines the empty case to mean excess. That's a quirk I would have liked to avoid. I don't see why you would need it?
OK, I will drop that.
@@ -5919,7 +5923,8 @@ static void shrink_node_memcgs(pg_data_t *pgdat, struct s> sc->memcg_low_skipped = 1; continue; } - memcg_memory_event(memcg, MEMCG_LOW); + if (memcg->memory.low) + memcg_memory_event(memcg, MEMCG_LOW);
That's not right. In setups where protection comes from the parent, no breaches would ever be counted.
OK. Will post a v3 to incorporate your suggestion.
Thanks, Longman