Re: [PATCH v3 05/16] mm/page_alloc: unify __alloc_frozen_pages[_nolock]_noprof()

[Harry Yoo]

On 7/1/26 1:56 AM, Brendan Jackman wrote:
> On Tue Jun 30, 2026 at 3:34 PM UTC, Vlastimil Babka (SUSE) wrote:
>> On 6/30/26 15:36, Harry Yoo wrote:
>>>
>>>
>>> On 6/29/26 10:11 PM, 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).
>>>>
>>>> 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_trylock_allowed() and
>>>>    gfp_trylock.
>>>
>>> Right.
>>>
>>>> 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, and
>>>>       only flags that affect the slowpath get changed here.
>>>
>>> gfp_allowed_mask clears __GFP_RECLAIM, and that means now allocations
>>> with GFP_KERNEL during early boot would see
>>> gfpflags_allow_spinning() = false.
>>
>> Is it a problem though? non-nolock allocations were affected before (the
>> masking existed for those already) and will be affected now the same, and
>> _nolock() allocations don't pass __GFP_RECLAIM in the first place, so the
>> masking can't affect them?
>
> This was my thinking too.

Oh! You guys are right :) I was confused.

>>> The helper is not used in in the page allocator, but used in
>>> memcg/stackdepot/page_owner.
>>>
>>>>    b. Application of current_gfp_context() - also only affects the
>>>>       slowpath
>>>
>>> PF_MEMALLOC_PIN affects the fast path, but ALLOC_NOLOCK users
>>> won't be affected.
>>
>> And it wouldn't be wrong if they were? It only clears __GFP_MOVABLE?

Nothing wrong, but wanted to mention things that were not mentioned in
the changelog as it argues there's no functional change intended.

>>> What about alloc_flags_nofragment/nonblocking()?
>>
>> ALLOC_NOFRAGMENT due to e.g. defrag_mode could be a problem indeed, if
>> there's no slowpath. Make ALLOC_NOLOCK override it?
> 
> Yeah calling alloc_flags_nofragment() here is a bug in the patch,
> and Sashiko also complained: 
> 
> https://lore.kernel.org/all/20260629142921.9A05A1F000E9@xxxxxxxxxxxxxxx/
> 
> Like I said in the reply to that thread I think maybe we _do_ want to
> set ALLOC_NOFRAGMENT for nolock allocations? But, that is a functional
> change, it doesn't belong in this series.

I don't think it was intentional to bypass ALLOC_NOFRAGMENT for nolock
allocations. But yeah that's a functional change.

>> nonblocking() is probably fine?
> 
> Yeah, I believe this is fine.

Agreed.

-- 
Cheers,
Harry / Hyeonggon