Re: [PATCH v2] memcg: trim the per-cpu charge stock instead of draining it

On Thu, Aug 20, 2026 at 08:44:34PM +0200, Michal Hocko wrote:
> On Thu 20-08-26 09:06:29, Shakeel Butt wrote:
> > On Thu, Aug 20, 2026 at 09:06:18AM +0200, Michal Hocko wrote:
> > > On Wed 19-08-26 18:20:10, Shakeel Butt wrote:
> > > > @@ -2254,9 +2264,12 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
> > > >  			empty_slot = i;
> > > >  		if (memcg == READ_ONCE(stock->cached[i])) {
> > > >  			stock_pages = READ_ONCE(stock->nr_pages[i]) + nr_pages;
> > > > +			if (stock_pages > MEMCG_STOCK_HIGH) {
> > > > +				memcg_uncharge(memcg,
> > > > +					       stock_pages - MEMCG_STOCK_LOW);
> > > > +				stock_pages = MEMCG_STOCK_LOW;
> > > 
> > > I would find it easier to read to keep the update in sync with
> > > memcg_uncharge, i.e.
> > > 				stock_pages = WRITE_ONCE(stock_pages - MEMCG_STOCK_LOW)
> > 
> > Sorry I am not sure I understand your suggestion. The WRITE_ONCE() is throwing
> > me off.
> 
> Sorry, brainfarth on my end. I meant to say that stock_pages should be
> updated by the uncharged decrement rather than capping it at MEMCG_STOCK_LOW.
> So in fact I meant this
> 				stock_pages = stock_pages - MEMCG_STOCK_LOW;
> but then shortcuted it to a nonsense.

To fully understand your suggestion, let me walkthrough the code:

Let's suppose stock_pages = 65

	// Original code does
	memcg_uncharge(memcg, stock_pages-MEMCG_STOCK_LOW); // uncharge 33 pages
	stock_pages = MEMCG_STOCK_LOW;			    // stock 32 pages

-----
	// You want instead
	memcg_uncharge(memcg, MEMCG_STOCK_LOW); 	// uncharge 32 pages
	stock_pages = stock_pages-MEMCG_STOCK_LOW	// stock 33 pages
	

Am I understanding you correctly?