Re: [PATCH v5 05/18] mm/page_alloc: unify __alloc_frozen_pages[_nolock]_noprof()

[Zi Yan]

On Fri Jul 3, 2026 at 8:31 AM EDT, Brendan Jackman wrote:
> Currently the core allocator code is controlled by ALLOC_NOLOCK, but the
> main entry point function is significantly different from the normal
> __alloc_frozen_pages_nolock(), this is tiring when reading the code.
>
> Plumb the ALLOC_NOLOCK control one layer up in the call stack: create
> an alloc_flags argument to __alloc_frozen_pages_nolock() (which is only
> exposed to mm/) and then turn the nolock variant into a thin wrapper
> that just sets that flag (as well as handling NUMA_NO_NODE, similar to
> how some of the wrappers in gfp.h do).
>
> For consistency, set ALLOC_WMARK_MIN explicitly in fastpath_alloc_flags
> for the new ALLOC_NOLOCK path. This was already "done" silently in
> __alloc_frozen_pages_nolock_noprof(): ALLOC_WMARK_MIN is 0.
>
> Rationale that this doesn't change anything:
>
> 1. Simple bits: A bunch of the nolock-specific handling is just moved to
>    the new alloc_order_allowed(), alloc_nolock_allowed() and
>    gfp_nolock.
>
> 2. __alloc_frozen_pages_noprof() has some extra logic that wasn't
>    previously in the nolock variant:
>
>    a. Application of gfp_allowed_mask; this only affects early boot,
>       only flags that affect the slowpath get changed here, and the
>       nolock allocation path isn't allowed to the GFP_BOOT_MASK flags.
>
>    b. Application of current_gfp_context() - also only affects the
>       slowpath
>
> 3. The slowpath itself: this is now just explicitly skipped under
>    !ALLOC_TRYLOCK.

s/TRYLOCK/NOLOCK

>
> Ulterior motive: adding an alloc_flags arg to the allocator's
> mm-internal entrypoint can later be used to do more allocation
> customisation without needing to create new GFP flags.
>
> No functional change intended.
>
> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>
> Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
> ---
>  mm/hugetlb.c    |   3 +-
>  mm/mempolicy.c  |  10 +--
>  mm/page_alloc.c | 192 +++++++++++++++++++++++++++++---------------------------
>  mm/page_alloc.h |   6 +-
>  mm/slub.c       |   6 +-
>  5 files changed, 117 insertions(+), 100 deletions(-)
>

<snip>

> +/*
> + * This is the 'heart' of the zoned buddy allocator.
> + */
> +struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
> +		int preferred_nid, nodemask_t *nodemask, unsigned int alloc_flags)
> +{
> +	struct page *page;
> +	gfp_t alloc_gfp; /* The gfp_t that was actually used for allocation */
> +	struct alloc_context ac = { };
> +	unsigned int fastpath_alloc_flags = alloc_flags;
> +
> +	/* Other flags could be supported later if needed. */
> +	if (WARN_ON(alloc_flags & ~ALLOC_NOLOCK))
>  		return NULL;
>  
> +	if (!alloc_order_allowed(gfp, order, alloc_flags))
> +		return NULL;
> +
> +	if (alloc_flags & ALLOC_NOLOCK) {
> +		VM_WARN_ON_ONCE(gfp & ~__GFP_ACCOUNT);
> +		if (!alloc_nolock_allowed())
> +			return NULL;

At first look, I wonder why __alloc_frozen_pages_noprof() needs to care
about alloc_nolock_allowed(). But the patch's idea is to centralize all
allocation policies, so it makes sense.

Ideally, I would want alloc_frozen_pages_nolock_noprof() to filter as
much as possible, so that __alloc_frozen_pages_noprof() has minimal/no
awareness of ALLOC_NOLOCK. But ALLOC_NOLOCK has different preferences
compared to the default __alloc_frozen_pages_noprof() policy like
ALLOC_WMARK_MIN vs ALLOC_WMARK_LOW, skip slowpath, and more. Maybe we
could do something like:

__alloc_frozen_pages_noprof()
{
     alloc_fastpath();
     alloc_slowpath();
}

alloc_frozen_pages_nolock_noprof()
{
    alloc_order_allowed();
    alloc_nolock_allow();
    alloc_fastpath();
}

But it still cannot remove ALLOC_NOLOCK completely from
__alloc_frozen_pages_noprof(), like the nofragment skip. Anyway, this
patch is a reasonable cleanup. Thanks.

Acked-by: Zi Yan <ziy@xxxxxxxxxx>


-- 
Best Regards,
Yan, Zi