Re: [RFC PATCH 2/5] mm/page_alloc: count a merged buddy's pages under the merged type

[Dev Jain]

On 14/10/26 7:57 pm, Rik van Riel wrote:
> On Wed, 2026-10-14 at 19:46 +0530, Dev Jain wrote:
>>
>>
>>> +++ b/mm/page_alloc.c
>>> @@ -993,6 +993,11 @@ static inline void __free_one_page(struct page
>>> *page,
>>>  			account_freepages(zone, 1 << order,
>>> migratetype);
>>>  		} else {
>>>  			__del_page_from_free_list(buddy, zone,
>>> order, buddy_mt);
>>> +			/* The buddy's free pages join the merged
>>> block's type. */
>>> +			if (unlikely(buddy_mt != migratetype)) {
>>> +				account_freepages(zone, -(1 <<
>>> order), buddy_mt);
>>> +				account_freepages(zone, 1 <<
>>> order, migratetype);
>>> +			}
>>>  		}
>>>  
>>
>> MAX_PAGE_ORDER > pageblock_order confuses things so much : (
> 
> I'm not a fan, either. It trips me up every time.
> 
>>
>> Sashiko raises something:
>> https://sashiko.dev/#/patchset/20261007021250.1665929-1-riel%40surriel.com
>>
>> Although feels like it is exaggerating.
> 
> I suppose if we switch the two lines around, doing
> the increment on migratetype before decrementing
> buddy_mt we could avoid that transient underflow?

That would become a transient overflow. See below.


> 
>>
>> My concern is for systems with a high diff between MAX_PAGE_ORDER and
>> pageblock_order. So on arm64 systems with 64K pagesize but pageblock
>> size as 2M (and PMD size 512M), the pageblock order is 5. We use this
>> configuration to reliably get TLB optimization from contpte size
>> mappings.
>>
>> Will we then repeatedly overflow the pcpu counter into the zone
>> counter,
>> regressing stuff?
> 
> The per_cpu_pages struct does not seem to keep
> counts on a per migratetype basis at all. It
> has per migratetype lists, but only one
> pcp->count.
> 
> Also doesn't the pcp only carry order zero
> pages currently?
> 
> I don't really understand your worry.

Sorry I was not talking about the pcp list (also, pcp carries
order 0 till 3?)

In __mod_zone_page_state, we use the per-cpu variable pcp->vm_stat_diff,
and when it overflows or underflows past a pcp->stat_threshold, we
push the diff to the global (zone) state.

The reason we do so is to optimize by batching zone operations into
per-cpu local operations.

My concern is that for large delta, we will always overflow
or underflow, defeating the purpose of the per-cpu caching.

So on said arm64 systems, we may frequently enter your

if (unlikely(buddy_mt != migratetype)

branch, and if pcp->stat_threshold is not enough, we may repeatedly
overflow and underflow, defeating this per-cpu optimization.

> 
>>
>> Perhaps I am overthinking. Your approach is clean, but perhaps we
>> could
>> also do:
>>
>> void account_freepages_move(zone, order, buddy_mt, mt)
>>
>> 	/* PCP-types do not have per-migratetype free counts. */
>> 	if (likely (buddy_mt < MIGRATE_PCPTYPES && mt <
>> MIGRATE_PCPTYPES))
>> 		return;
> 
> Wouldn't that throw off the counts when we merge
> e.g. a MIGRATE_UNMOVABLE page with a MIGRATE_MOVABLE
> buddy?
> 

As you said, in account_freepages(), there is currently
no conditional based on MOVABLE and UNMOVABLE. So we
are essentially doing -x and +x, netting to zero.

So we could write my function, make it clear that currently
this is not needed for the MIGRATE_PCPTYPES, as opposed
to MIGRATE_CMA and MIGRATE_HIGHATOMIC which have separate
accounting.

> With your test we would only ever do the free count
> adjustment for mismatched types if one of the pages
> was HIGHATOMIC or CMA. That leaves the common case
> without coverage.
>