[Yosry Ahmed <yosry@kernel.org>]

Bcc:
Subject: Re: [PATCH V3 3/3] mm: page_alloc: drain pcp lists before oom kill
Message-ID: <aphUaA2FlniZ_h9v@google.com>
Reply-To:
In-Reply-To: <aphSYFTd12oia8K2@google.com>

On Wed, Sep 02, 2026 at 04:49:48PM +0000, Yosry Ahmed wrote:
> On Sun, Nov 05, 2023 at 06:20:50PM +0530, Charan Teja Kalla wrote:
> > pcp lists are drained from __alloc_pages_direct_reclaim(), only if some
> > progress is made in the attempt.
> > 
> > struct page *__alloc_pages_direct_reclaim() {
> >     .....
> >    *did_some_progress = __perform_reclaim(gfp_mask, order, ac);
> >    if (unlikely(!(*did_some_progress)))
> >       goto out;
> > retry:
> >     page = get_page_from_freelist();
> >     if (!page && !drained) {
> >         drain_all_pages(NULL);
> >         drained = true;
> >         goto retry;
> >     }
> > out:
> > }
> > 
> > After the above, allocation attempt can fallback to
> > should_reclaim_retry() to decide reclaim retries. If it too return
> > false, allocation request will simply fallback to oom kill path without
> > even attempting the draining of the pcp pages that might help the
> > allocation attempt to succeed.
> > 
> > VM system running with ~50MB of memory shown the below stats during OOM
> > kill:
> > Normal free:760kB boost:0kB min:768kB low:960kB high:1152kB
> > reserved_highatomic:0KB managed:49152kB free_pcp:460kB
> > 
> > Though in such system state OOM kill is imminent, but the current kill
> > could have been delayed if the pcp is drained as pcp + free is even
> > above the high watermark.
> > 
> > Fix this missing drain of pcp list in should_reclaim_retry() along with
> > unreserving the high atomic page blocks, like it is done in
> > __alloc_pages_direct_reclaim().
> > 
> > Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>
> 
> [Sorry for thread necromancy]
> 
> Hi Charan,

Charan's email bounced, trying another one I found on lore, and adding a
few folks from other replies.

> 
> Are you planning to respin this patch?
> 
> I know that Michal was questioning the need for it. While doing some
> stress testing I came across a couple of OOM kills that had significant
> amount of memory in pcplists. Something that would have been prevented
> by this patch.
> 
> It's not something I can reproduce reliably and it is an artificial
> workload, so not very concerning, but the patch seems like generally a
> good idea and hopefully pretty harmless. It's also consistent with the
> code in __alloc_pages_direct_reclaim().
> 
> For the record, we have been carrying it internally for a while and
> there haven't been any issues AFAICT.
> 
> Michal, WDYT?
> 
> > ---
> >  mm/page_alloc.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index b91c99e..8eee292 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3857,8 +3857,10 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
> >  		cond_resched();
> >  out:
> >  	/* Before OOM, exhaust highatomic_reserve */
> > -	if (!ret)
> > -		return unreserve_highatomic_pageblock(ac, true);
> > +	if (!ret) {
> > +		ret =  unreserve_highatomic_pageblock(ac, true);
> > +		drain_all_pages(NULL);
> > +	}
> >  
> >  	return ret;
> >  }
> > -- 
> > 2.7.4
> > 
> > 
>