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?
> @@ -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?
> @@ -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.
