Re: [PATCH v3 19/26] mm/page_alloc: rename ALLOC_NON_BLOCK back to _HARDER

On Tue Aug 4, 2026 at 10:50 PM BST, Yosry Ahmed wrote:
> On Fri, Jul 31, 2026 at 04:52:59PM +0200, Vlastimil Babka (SUSE) wrote:
>> On 7/27/26 00:22, Brendan Jackman wrote:
>> > Commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
>> > non-blocking allocations accesses reserves") renamed ALLOC_HARDER to
>> > ALLOC_NON_BLOCK because the former is "a vague description".
>> > 
>> > However, vagueness is accurate here, this is a vague flag. It is not set
>> > for __GFP_NOMEMALLOC. It doesn't really mean "allocate without blocking"
>> > but rather "allow dipping into atomic reserves, _because_ of the need
>> > not to block".
>> > 
>> > A later commit will need an alloc flag that really means "don't block
>> > here", so go back to the flag's old name and update the commentary
>> > to try and give it a slightly clearer meaning.
>> > 
>> > Signed-off-by: Brendan Jackman <[email protected]>

Writing this to get it clear in my head, so I'll also dump it in the
mail in case it helps get us on the same page... 

What we actually want here is a flag that tells us when we can do a TLB
shootdown. That means (on x86) that IRQs must be on and we mustn't be
holding some random spinlock (most spinlocks would actually be fine but
I think it's simpler to assume we can't hold any). 

It must never be over-permissive i.e. tell us we can do a TLB flush
when we can't. It's fine to _sometimes_ be over-restrictive i.e. tell us
we can't do a TLB flush when we can, but if it always forbids flushing
while GFP_BOOT_MASK is in effect then we'll fail critical allocations
and crash.

>> I wonder if we need to do this, and instead we could repurpose
>> ALLOC_NON_BLOCK directly. AFAIU it's about removing the side-effect of
>> gfp_allowed_mask in the next patch. But what would happen if we did that
>> using the existing ALLOC_NON_BLOCK (or maybe just renamed to ALLOC_NOBLOCK?).
>>
>> - in __zone_watermark_ok(), ALLOC_NON_BLOCK could now be *not* set in
>> situations where previously it was set due to gfp_allowed_mask masking out
>> __GFP_DIRECT_RECLAIM. 

Hm, I can't follow this. The first paragraph sounds like you're
proposing that we set ALLOC_NON_BLOCk regardless of gfp_allowed_mask. I
think that would be fine. But then the second paragraph is saying
ALLOC_NON_BLOCK would now be unset in places where it's formerly set,
whereas I think the proposal means it gets set in places it was formerly
unset.

>> But it only has an effect on top of __GFP_HIGH (thus
>> ALLOC_MIN_RESERVE... which seems contradicting the ALLOC_NON_BLOCK
>> description comment btw). Also __GFP_DIRECT_RECLAIM is only masked out by
>> GFP_BOOT_MASK when all memory is free, so it's kinda moot?

... but yes, I do think making the existing ALLOC_NON_BLOCK ignore
gfp_allowed_mask would probably work.

Aside from gfp_allowed_mask the other thing about ALLOC_NON_BLOCK is
that it gets disabled if __GFP_NOMEMALLOC is set. That is fine for the
current usecase, but it's pretty confusing...

>> - in rmqueue_buddy() we allow access to highatomic reserves since
>> 281dd25c1a018. That commit describes GFP_ATOMIC so we could have been
>> checking ALLOC_MIN_RESERVE. But we can also leave this alone because it
>> doesn't actually matter when GFP_BOOT_MASK is set, as above.
>
> IIUC we are trying to find out if the callers either has interrupts
> disabled or is holding a lock, and using __GFP_DIRECT_RECLAIM as an
> indicator. As you mention, it seems like __GFP_DIRECT_RECLAIM is only
> masked during boot, presumably before we can allocate any unmapped
> memory (should always be user memory?).

Exactly.

> So maybe we should just use gfpflags_allow_blocking()?

Oh yeah, it definitely should. This doesn't change any of the plumbing
challenges though since we've lost the GFP flags by the time we get to
__rmqueue_direct_map().

> I also wonder if restricing to callers __GFP_DIRECT_RECLAIM is too
> restrictive, 

It is overly restrictive, but I dont' know of anything better, and if it
existed I think it would be in gfpflags_allow_blocking().

> we can probably key off __GFP_ATOMIC as I assume any
> callers with IRQs disabled or wiht a lock have to set it.

There's no such thing as __GFP_ATOMIC. (I think there used to be?)

> Maybe we can also use preemptible(), but that creates a dependency on
> CONFIG_PREEMPT_COUNT as far as I can tell.

Yeah. Which...  maybe is fine nowadays? Since commit 7dadeaa6e851
("sched: Further restrict the preemption modes") you can only set
PREEMPT_NONE on alpha/hexagon/m68k. And this restriction only actually
matters on x86 anyway...