Re: [PATCH v4 1/8] mm/page_alloc: reset can_retry_reserves when the slowpath restarts

["Gregory Price (Meta)" <gourry@gourry.net>]

On 2026-10-29 14:37 +0100, Vlastimil Babka (SUSE) wrote:
> On 10/28/26 22:19, Gregory Price (Meta) wrote:
> > Commit 2c4c3e29897d ("mm/page_alloc: simplify __alloc_pages_slowpath()
> > flow") retries an allocation once with reserve_flags when the caller
> > becomes eligible for reserves, guarded by can_retry_reserves.  The flag
> > is set at its declaration and never reset.
> > 
> > A restart after a cpuset update or a zonelist rebuild drops
> > reserve_flags
> 
> I don't follow, nothing "drops reserve_flags"? after the restart,
> there will still be a get_page_from_freelist() without them followed by
> get_page_from_freelist() with them, right?
> 

"Drops" is the wrong word, sorry.

After a restart the reserve bits are recomputed, but can_retry_reserves
is already false.  As a result no-reclaim/PF_MEMALLOC callers don't
get a reserve-retry attempt (we just exit w/ nopage).

Reclaim callers are fine.

This problem exists regardless of the rest of the series.

But you're right, I will rework the commit message.


> > Reset can_retry_reserves at restart.  Only a cpuset or zonelist change
> > restarts the slowpath, so the retry still runs at most once per change.
> > 
> > With kprobes failing every attempt without reserves and the first with
> > them, and a memory block onlined during the walk, the allocation
> > returned NULL in 3 of 3 runs, and a page in 3 of 3 with this change.
> 
> So IIUC the allocation was racing with a memory online in a way that it
> would fail if the online happened fully after the allocation. So this just
> turns part of the race window into the allocation's favor?
>
> I mean it's not wrong and if patch 5 depends on it (unless it also only
> fixes a racy window in the same way I think is happening here) then so be
> it. But the commit log would need correcting.
>

Not quite - without the online it would have succeeded.  The zonelist
rebuild rewrites the lists in-place, so a concurrent walk can miss valid
zones or pick the wrong zone.  The rest of the series detects more of
these restart scenarios.

Those restarts then exacerbate the issue this patch resolves.  For
example, patch 5 would free a reserve allocation that succeeded - and
then fail because can_retry_reserves is already false.

-- 
~Gregory