Re: [PATCH v2 1/7] mm: list_lru: lock_list_lru_of_memcg() cannot return NULL if !skip_empty

[Shakeel Butt]

On Wed, Mar 18, 2026 at 03:25:29PM -0400, Johannes Weiner wrote:
> On Wed, Mar 18, 2026 at 10:56:55AM -0700, Shakeel Butt wrote:
> > On Thu, Mar 12, 2026 at 04:51:49PM -0400, Johannes Weiner wrote:
> > > skip_empty is only for the shrinker to abort and skip a list that's
> > > empty or whose cgroup is being deleted.
> > > 
> > > For list additions and deletions, the cgroup hierarchy is walked
> > > upwards until a valid list_lru head is found, or it will fall back to
> > > the node list. Acquiring the lock won't fail. Remove the NULL checks
> > > in those callers.
> > > 
> > > Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> > > ---
> > 
> > What do you think about squashing the following into this patch?
> > 
> > From bd56ea4505f792e00079b1a8dd98cb6f7a5e7215 Mon Sep 17 00:00:00 2001
> > From: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> > Date: Wed, 18 Mar 2026 10:43:53 -0700
> > Subject: [PATCH] list_lru: cleanup
> > 
> > Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> 
> Thanks for taking a look!
> 
> There is some overlap and conflict between your delta and what later
> patches in the series do.
> 
> AFAICS, the main thing left over would be: to have
> __lock_list_lru_of_memcg() for the reclaimer (which does not walk the
> parents during a cgroup deletion race) and lock_list_lru_of_memcg()
> which does. Thereby eliminating the @skip_empty bool.

Yeah, I saw your discussion with David and thought on how can we further reduce
the params.

> The downside
> would be to have another level in the lock function stack which is
> duplicated for CONFIG_MEMCG and !CONFIG_MEMCG, and the !CONFIG_MEMCG
> versions are identical.
> 
> I'm not sure that's worth it?

I am fine with whatever route you take. I know you have next version ready to
send, I will review the remaining patches for the next version (though I have
taken a look on the current series but will add my tags for the next one :P).