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

[Brendan Jackman]

On Fri Jun 19, 2026 at 8:17 AM UTC, Brendan Jackman wrote:
> On Fri Jun 19, 2026 at 3:56 AM UTC, Matthew Wilcox wrote:
>> On Wed, Jun 17, 2026 at 03:29:42PM +0000, Brendan Jackman wrote:

>>> +/*
>>> + * GFP flags to set for ALLOC_TRYLOCK i.e. alloc_pages_nolock().
>>> + *
>>> + * Do not specify __GFP_DIRECT_RECLAIM, since direct claim is not allowed.
>>> + * Do not specify __GFP_KSWAPD_RECLAIM either, since wake up of kswapd
>>> + * is not safe in arbitrary context.
>>> + *
>>> + * These two are the conditions for gfpflags_allow_spinning() being true.
>>> + *
>>> + * Specify __GFP_NOWARN since failing alloc_pages_nolock() is not a reason
>>> + * to warn. Also warn would trigger printk() which is unsafe from
>>> + * various contexts. We cannot use printk_deferred_enter() to mitigate,
>>> + * since the running context is unknown.
>>> + *
>>> + * Specify __GFP_ZERO to make sure that call to kmsan_alloc_page() below
>>> + * is safe in any context. Also zeroing the page is mandatory for
>>> + * BPF use cases.
>>
>> It may be mandatory for BPF, but it seems wasteful for other uses.
>
> True, don't see why we shouldn't push this out to the caller, I can do
> it as part of this series.

Wait, I wasn't paying attention to the bit about kmsan_alloc_page().
__GFP_ZERO is load bearing for the implementation because otherwise
kmsan_alloc_page() does things that might not be safe in the current
context. 

> >> +	if (alloc_flags & ALLOC_TRYLOCK) {
> >> +		VM_WARN_ON_ONCE(gfp & ~__GFP_ACCOUNT);
> >
> > So the only GFP flag the user is allowed to specify is __GFP_ACCOUNT?
> > That seems bogus; other flags would be reasonable including all the ones
> > in gfp_trylock, as well as GFP_HIGHMEM, GFP_DMA, GFP_MOVABLE, GFP_HARDWALL.
> 
> Definitely makes sense for the ones in gfp_trylock.
> 
> For the others, I'm not sure - this "nolock" functionality is a bit
> weird and sketchy, 

...

> I suspect the reason for the WARN here is "let's make
> sure we have a proper think before we allow it to grow usecases that are
> meaningfully different from the other ones". I think I like that
> conservatism here, I would lean towards keeping it? Not a passionately
> held opinion though.

I'd probably still lean towards pushing __GFP_ZERO to the caller though,
and just WARN+fail if it isn't passed, to make it explicit?