Re: [PATCH v6 1/2] mm/vmscan: Skip memcg with !usage in shrink_node_memcgs()

[Johannes Weiner <hannes@cmpxchg.org>]

On Mon, Apr 14, 2025 at 08:01:42PM +0200, Michal Koutný wrote:
> On Mon, Apr 14, 2025 at 12:47:21PM -0400, Johannes Weiner <hannes@cmpxchg.org> wrote:
> > It's not a functional change to the protection semantics or the
> > reclaim behavior.
> 
> Yes, that's how I understand it, therefore I'm wondering what does it
> change.
> 
> If this is taken:
>                if (!mem_cgroup_usage(memcg, false))
>                        continue;
> 
> this would've been taken too:
>                 if (mem_cgroup_below_min(target_memcg, memcg))
>                         continue;
> (unless target_memcg == memcg but that's not interesting for the events
> here)

D'oh.

> > The problem is if we go into low_reclaim and encounter an empty group,
> > we'll issue "low-protected group is being reclaimed" events,
> 
> How can this happen when
> 	page_counter_read(&memcg->memory) <= memcg->memory.emin
> ? (I.e. in this case 0 <= emin and emin >= 0.)
> 
> > which is kind of absurd (nothing will be reclaimed) and thus confusing
> > to users (I didn't even configure any protection!)
> 
> Yes.
>  
> > I suggested, instead of redefining the protection definitions for that
> > special case, to bypass all the checks and the scan count calculations
> > when we already know the group is empty and none of this applies.
> > 
> > https://lore.kernel.org/linux-mm/20250404181308.GA300138@cmpxchg.org/
> 
> Is this non-functional change to make shrink_node_memcgs() robust
> against possible future redefinitions of mem_cgroup_below_*()?

No, this was really just aimed to stop low events on empty groups.

But as you rightfully point out, they should not get past the min
check in the first place. So something seems missing here.