Re: [PATCH] mm: memcg: flush empty per-cpu stock slots on memcg offlining

Shakeel Butt <[email protected]>
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
On Mon, Aug 17, 2026 at 09:58:39PM +0800, Song Hu wrote:
> Hi,Michal
> 
> 在 2026/8/17 21:29, Michal Hocko 写道:
> > On Mon 17-08-26 21:12:21, Song Hu wrote:
> >> On Mon 17-08-26, Michal Hocko wrote:
> >>> Is there any specific reason why the memcg stays in the cache slot
> >>> without any pages?
> >>
> >> consume_stock() doesn't release the slot when nr_pages hits zero.  It
> >> is kept for the next charge of the same task and only gets displaced
> >> by a charge under a different memcg or by CPU hotplug.  The problem is
> >> that the offlining drain skips empty slots, so the css reference they
> >> hold is never dropped unless something unrelated displaces them.
> > 
> > This doesn't answer my question, really, does it? Is there any good
> > reason for this implementation? Why do we need to drop references
> > remotely when we can do so when the last cached charge is consumed?
> > 
> 
> Fair enough.  There is no strong reason.  Keeping the slot
> populated after the last page is consumed only saves a
> css_get()/css_put() pair when the same memcg charges again on that
> CPU - a micro-optimization from the original single-slot
> implementation.
> 
> Dropping the reference in consume_stock() when the slot empties is
> the better place.  Empty slots stop existing, so the offlining drain
> has nothing left to miss and is_memcg_drain_needed() stays as it
> is.  This also makes Joshua's concern about the full-stock drain go
> away entirely.  The cost is one refcount pair per emptied slot, at
> most once per MEMCG_CHARGE_BATCH pages.
> 
> Joshua, this supersedes the css_is_dying gating you suggested and
> that I said I would do - with no empty slots left, the check would
> never fire, and the kill_css_sync() ordering argument becomes moot
> as well.  Since you are reworking this code, I'd appreciate a
> sanity check on releasing from consume_stock().
> 
> If this direction works for you both, I'll rework the patch
> accordingly.

Yes, this seems reasonable. Go ahead with the rework.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.