Re: [PATCH V3 3/3] mm: page_alloc: drain pcp lists before oom kill

[Michal Hocko]

On Fri 04-09-26 09:35:55, Yosry Ahmed wrote:
> On Fri, Sep 4, 2026 at 9:27 AM Michal Hocko <mhocko@xxxxxxxx> wrote:
[...]
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 8d79f76cdd0e1..98e9079240ad5 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -4592,11 +4592,12 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask,
> > > unsigned int order,
> > >         psi_memstall_enter(&pflags);
> > >         *did_some_progress = __perform_reclaim(gfp_mask, order, ac);
> > >         if (unlikely(!(*did_some_progress)))
> > > -               goto out;
> > > +               goto drain;
> > >
> > >  retry:
> > >         page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
> > >
> > > +drain:
> > >         /*
> > >          * If an allocation failed after direct reclaim, it could be because
> > >          * pages are pinned on the per-cpu lists or in high alloc reserves.
> > > @@ -4608,7 +4609,6 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask,
> > > unsigned int order,
> > >                 drained = true;
> > >                 goto retry;
> > >         }
> > > -out:
> > >         psi_memstall_leave(&pflags);
> >
> > Ideally if we can make the function call less hairy. Maybe we want to
> > make draining part of the reclaim as the last resort when normal reclaim
> > fails.
> 
> Do you mean do the draining in __perform_reclaim(), or deeper into the
> reclaim stack?
> 
> The thing is that __alloc_pages_direct_reclaim() currently drains when
> __perform_reclaim() fails to make any progress and we still cannot
> allocate. The change above makes it drain if it cannot allocate after
> __perform_reclaim(), regardless of progress. So if you want to move it
> into __perform_reclaim(), we'll have it in both places.
> 
> Or maybe I just don't understand what you meant :)

Sorry for not being clear enough. I meant to pull draining out of
__alloc_pages_direct_reclaim and instead have it somewhere in the
reclaim path. It is not entirely clear to me  where at the moment but we
do not need to have the same behavior as now. The idea behind the code
is to not drain way too much. Maybe we want to drain when dropping the
priority down to 0.

-- 
Michal Hocko
SUSE Labs