Re: [PATCH v2 1/3] mm: page_alloc: add a watermark check that knows the migratetype

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 11/11/26 20:49, Kiryl Shutsemau wrote:
> On Thu, Nov 05, 2026 at 05:06:39PM +0000, Harry Yoo wrote:
>> On Mon, Nov 02, 2026 at 09:39:34AM +0000, Kiryl Shutsemau (Meta) wrote:
>> > The high-order check in __zone_watermark_ok() accepts a free page of any
>> > migratetype.  Under ALLOC_NOFRAGMENT, a request below pageblock_order can
>> > only take a page of its own migratetype or claim a whole free block, so
>> > the check can pass when the allocation cannot succeed.
>> > 
>> > Telling those apart needs the request's migratetype, but not every check
>> > wants them told apart.  A check whether an allocation can be served now
>> > has to count only the pages it can take.
>> >
>> > A check whether there is enough free memory counts every migratetype:
>> > kswapd and kcompactd balancing a zone, order-0 thresholds, the
>> > slowpath's forecast whether reclaim and compaction can still make
>> > progress.  Reclaim and compaction work across migratetypes to free whole
>> > pageblocks.
>> 
>> nit: perhaps we could make the changelog above bit more concise.
>> 
>> I think the necessary bit here is "With defrag_mode, allocations cannot
>> steal a page from pageblocks of other migrate type, thus
>> __zone_watermark_ok() needs to take that into account."
> 
> 
> It is ALLOC_NOFRAGMENT, not defrag_mode: Normal zone allocations get it
> on the first attempt when DMA32 is populated, with defrag_mode off. And
> such an allocation can still claim a whole free pageblock of another
> migratetype.
> 
> But fair, it can be shorter. A tighter version is below; it also says
> why kcompactd passes MIGRATE_MOVABLE.
> 
>> nit: maybe let's set cc.migratetype and pass it?
> 
> The value is not read there: __zone_watermark_ok_mt() uses the
> migratetype only under ALLOC_NOFRAGMENT, and kcompactd never passes it.
> kcompactd_node_suitable() makes the same call and has no compact_control
> to take it from.
> 
> Thanks for the review.
> 
> The new changelog:
> 
> 	The high-order check in __zone_watermark_ok() accepts a free page of any
> 	migratetype.  Under ALLOC_NOFRAGMENT, a request below pageblock_order can
> 	only take a page of its own migratetype or claim a whole free block, so
> 	the check can pass when the allocation cannot succeed.
> 
> 	Only a check whether one allocation can be served now should count just
> 	the pages it can take.

Please try actually reading this sentence, does it make any sense? It
instantly reminded me of the meme classic: "Has anyone really been far even
as decided to use even go want to do look more like?"

> A check whether a zone has enough free memory
> 	serves no single allocation and counts every migratetype: reclaim and
> 	compaction free whole pageblocks across migratetypes.

Oh yeah always this damned style again. What did they train the models on?

> 	Add __zone_watermark_ok_mt(), which takes the migratetype, for the first
> 	kind:
> 
> 	  - the allocator fast path, zone_watermark_fast();
> 	  - its recheck after node reclaim in get_page_from_freelist();
> 	  - compaction's up-front check, compaction_suit_allocation_order().
> 
> 	__zone_watermark_ok() wraps it for the second kind.  kcompactd shares
> 	compaction_suit_allocation_order() with direct compaction and passes
> 	MIGRATE_MOVABLE, which is not read without ALLOC_NOFRAGMENT.
> 
> 	Declare both in mm/internal.h: nothing outside mm uses them.
> 
> 	Nothing reads the migratetype yet: no functional change.  Preparation for
> 	making the high-order check honour ALLOC_NOFRAGMENT.
> 
> Looks good?

Not really :(