Re: [PATCH v3 1/2] mm/page_alloc: Clarify terminology in migratetype fallback code

[Vlastimil Babka <vbabka@suse.cz>]

On 2/27/25 11:13, Brendan Jackman wrote:
> On Wed, Feb 26, 2025 at 04:08:09PM +0100, Vlastimil Babka wrote:
>> On 2/25/25 4:29 PM, Brendan Jackman wrote:
>> >  /*
>> >   * When we are falling back to another migratetype during allocation, try to
>> > - * steal extra free pages from the same pageblocks to satisfy further
>> > - * allocations, instead of polluting multiple pageblocks.
>> > + * claim entire blocks to satisfy further allocations, instead of polluting
>> > + * multiple pageblocks.
>> >   *
>> > - * If we are stealing a relatively large buddy page, it is likely there will
>> > - * be more free pages in the pageblock, so try to steal them all. For
>> > - * reclaimable and unmovable allocations, we steal regardless of page size,
>> > - * as fragmentation caused by those allocations polluting movable pageblocks
>> > - * is worse than movable allocations stealing from unmovable and reclaimable
>> > - * pageblocks.
>> > + * If we are stealing a relatively large buddy page, it is likely there will be
>> > + * more free pages in the pageblock, so try to claim the whole block. For
>> > + * reclaimable and unmovable allocations, we claim the whole block regardless of
>> 
>> It's also "try to claim" here as it may still fail due to not enough
>> free/compatible pages even for those migratetypes. 
> 
> Yeah thanks, I seems worthwhile to be precise about this.
> 
>> Maybe the question
>> (out of scope of the patch) if they should get a lower threshold than
>> half. Before migratetype hygiene, the "we steal regardless" meant that
>> we really would steal all free pages even if not claiming the pageblock.
> 
> I wonder if there's some benefit to deciding when to claim blocks also
> based on the count of blocks per type or something? Like, if we're
> under memory pressure so all the freelists are empty, but there are
> already a bunch of unmovable blocks (that currently have movable pages
> in them), there's not much point in claiming a block, instead we'd
> prefer to reclaim/migrate those movable pages?

Hm we don't currently have counters for that, do we? But that would be the
feasible part. "that currently have movable pages in them" would be
expensive to determine, and an attempt to reclaim/migrate would be expensive
too. At best we could try kicking some asynchronous attempt I think.

>> > + * page size, as fragmentation caused by those allocations polluting movable
>> > + * pageblocks is worse than movable allocations stealing from unmovable and
>> > + * reclaimable pageblocks.
>> >   */
>> > -static bool can_steal_fallback(unsigned int order, int start_mt)
>> > +static bool should_claim_block(unsigned int order, int start_mt)
>> 
>> So technically it's should_try_claim_block() if we want to be precise
>> (but longer).
> 
> Yep, that seems worth it (especially given the function is effectively
> just an internal block of find_suitable_fallback()).
> 
>> >  {
>> >  	/*
>> >  	 * Leaving this order check is intended, although there is
>> >  	 * relaxed order check in next check. The reason is that
>> > -	 * we can actually steal whole pageblock if this condition met,
>> > +	 * we can actually claim the whole pageblock if this condition met,
>> 
>> try claiming
> 
> But I think in this case we really do claim unconditionally.

Ah right, thanks.