Re: [PATCH for-7.5 4/5] memcg: avoid queuing high_work from reclaim

[Michal Hocko <mhocko@suse.com>]

On Wed 14-10-26 10:55:21, Shakeel Butt wrote:
> On Wed, Oct 14, 2026 at 06:59:30PM +0200, Michal Hocko wrote:
> > On Wed 14-10-26 09:06:00, Shakeel Butt wrote:
> > > On Wed, Oct 14, 2026 at 01:39:06PM +0200, Michal Hocko wrote:
> > > > On Sat 10-10-26 14:09:56, Shakeel Butt wrote:
> > > > > Kernel threads and remote chargers use high_work when memory.high is
> > > > > exceeded. Their allocations during reclaim can also charge memory,
> > > > > for example when zswap stores a page.
> > > > > 
> > > > > Without a check, the high_work worker can queue itself from its own
> > > > > reclaim. The pending bit is cleared before high_work runs, so each run
> > > > > can queue another run.
> > > > 
> > > > What does that mean really? Does this delays the actual run?
> > > 
> > > It will not delay the actual charge request. This mainly means that we keep
> > > enqueing the high reclaim work even from high reclaim context and can
> > > potentially keep wasting CPU if high reclaim is not very successful.
> > 
> > Hmm, so is there any practical problem if we do that? What exactly do you
> > mean by "keep wasting CPU"?
> 
> More specifically if high_work kworker keeps enqueing high_work, most probably
> the memcg does not have reclaimable memory and keep retrying it wastes CPU.

Yes but this is not specific to PF_MEMALLOC context, or do I miss
something?
 
> > > > > Skip memory.high enforcement for charges from task context with
> > > > > PF_MEMALLOC set when they would use high_work. This covers kernel
> > > > > threads and remote charges. The memory is still charged. Interrupt
> > > > > charges keep their current path.
> > > > > 
> > > > > In a VM with incompressible zswap data, the worker queued itself 163 times
> > > > > without this check.
> > > > > 
> > > > > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> > > > 
> > > > The patch itself looks OK. I am wondering whether we can unify the check
> > > > with the existing one which forces the allocation through in main
> > > > charging path.
> > > 
> > > The force charge (and ENOMEM) paths bypass the memory.high enforcement
> > > altogether.
> > 
> > Exactly. Isn't that exactl what we want to achieve here as well?
> > 
> > > Only successful charges (done_restock label) goes through
> > > memory.high enforcement. So, even if we move this check in try_charge_memcg, it
> > > will still be a separate check before calling high enforcement function. I think
> > > this check is very high_work specific and better to be in the high enforcement
> > > function.
> > 
> > I meant something like the following
> 
> Oh ok this makes sense. Do you want to send a patch or should I add this in this
> series replacing the 4th patch?

Feel free to take it. I haven't even tried to compile this. Just wanted
to show what I have in mind.

-- 
Michal Hocko
SUSE Labs