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

["Brendan Jackman" <brendan.jackman@linux.dev>]

On Tue Jun 30, 2026 at 1:36 PM UTC, Harry Yoo wrote:
>> 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.
>> 
>> While adding this flag to a bunch of places, create ALLOC_DEFAULT to
>> avoid a mysterious literal 0 in most places.
>>
>> alloc_frozen_pages_noprof() is defined above the alloc flags
>
> The function is defined below the alloc flags, no?

Yep this paragraph is stale since I created mm/page_alloc.h, will remove
it.

>> so just leave that as a slightly messy
>> exception instead of trying to fully reorder mm/internal.h for that one
>> case.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Brendan Jackman <jackmanb@google.com>
>> ---
>>  mm/hugetlb.c    |   3 +-
>>  mm/mempolicy.c  |  10 ++--
>>  mm/page_alloc.c | 178 +++++++++++++++++++++++++++++---------------------------
>>  mm/page_alloc.h |   6 +-
>>  mm/slub.c       |   6 +-
>>  5 files changed, 108 insertions(+), 95 deletions(-)
>> 
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index a3ba63c7f9199..8d409d075e3e9 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5271,24 +5271,98 @@ void free_pages_bulk(struct page **page_array, unsigned long nr_pages)
>>  	}
>>  }
>>  
>> +static inline bool alloc_trylock_allowed(void)
>> +{
>> +	/*
>> +	 * In PREEMPT_RT spin_trylock() will call raw_spin_lock() which is
>> +	 * unsafe in NMI. If spin_trylock() is called from hard IRQ the current
>> +	 * task may be waiting for one rt_spin_lock, but rt_spin_trylock() will
>> +	 * mark the task as the owner of another rt_spin_lock which will
>> +	 * confuse PI logic, so return immediately if called from hard IRQ or
>> +	 * NMI.
>> +	 *
>> +	 * Note, irqs_disabled() case is ok. This function can be called
>> +	 * from raw_spin_lock_irqsave region.
>> +	 */
>> +	if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
>> +		return false;
>> +
>> +	/* On UP, spin_trylock() always succeeds even when it is locked */
>> +	if (!IS_ENABLED(CONFIG_SMP) && in_nmi())
>> +		return false;
>
> Except for deferred_pages_enabled(), it's not specific to the page
> allocator. SLUB has
>
> 	/*
> 	 * See the comment for the same check in
> 	 * alloc_frozen_pages_nolock_noprof()
> 	 */
>
> ... and repeats the same thing as above.
>
> Perhaps let's factor it out into a helper
> rather than trying not to forget to update the other place?

Hm, not sure about this. I think I would say it's a "coincidence" that
these two bits of code look the same? Like, page_alloc.c uses
spin_trylock() so you can't do alloc_pages_nolock() from IRQ on
PREEMPT_RT. slub.c ALSO uses spin_trylock(), so you ALSO can't use
kmalloc_nolock() in those scenarios. But those are two different facts
that just happen to be isomorphic? Putting them into a shared helper
would kinda imply that these are part of a single system with inherently
coupled constraints.

I dunno I'm being a bit of a ponderous philosopher there, I don't have
particularly strong feelings. But I'd lean towards leaving this out of
the patchset since the potential deduplication isn't really related to
the other cleanups anyway.