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

[Yosry Ahmed <yosry@kernel.org>]

On Fri, Aug 14, 2026 at 01:09:32PM +0100, Brendan Jackman wrote:
> 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 <jackmanb@google.com>
> 
> 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.

Why is it a problem if it forbids flushing while GFP_BOOT_MASK is in
effect? We should change gfp_allowed_mask before any unmapped
allocations are possible, right?

> 
> >> 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().

Yeah I think I prefer just using gfpflags_allow_blocking() if we cannot
use preemtible().

> 
> > 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?)

Duh.

> 
> > 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...

Yeah I will look into that. I like preemptible() because it encodes
exactly what we actually need.