Re: [PATCH v3 13/26] mm: introduce freetype_t

On Wed Aug 5, 2026 at 12:02 AM BST, Yosry Ahmed wrote:
>> @@ -3190,9 +3272,9 @@ EXPORT_SYMBOL_GPL(split_page);
>>  int __isolate_free_page(struct page *page, unsigned int order)
>>  {
>>  	struct zone *zone = page_zone(page);
>> -	int mt = get_pageblock_migratetype(page);
>> +	freetype_t ft = get_pageblock_freetype(page);
>>  
>> -	if (!is_migrate_isolate(mt)) {
>> +	if (!is_migrate_isolate(free_to_migratetype(ft))) {
>>  		unsigned long watermark;
>>  		/*
>>  		 * Obey watermarks as if the page was being allocated. We can
>> @@ -3205,7 +3287,7 @@ int __isolate_free_page(struct page *page, unsigned int order)
>>  			return 0;
>>  	}
>>  
>> -	del_page_from_free_list(page, zone, order, mt);
>> +	del_page_from_free_list(page, zone, order, ft);
>>  
>>  	/*
>>  	 * Set the pageblock if the isolated page is at least half of a
>> @@ -3214,14 +3296,16 @@ int __isolate_free_page(struct page *page, unsigned int order)
>>  	if (order >= pageblock_order - 1) {
>>  		struct page *endpage = page + (1 << order) - 1;
>>  		for (; page < endpage; page += pageblock_nr_pages) {
>> -			int mt = get_pageblock_migratetype(page);
>> +			freetype_t old_ft = get_pageblock_freetype(page);
>> +			freetype_t new_ft = freetype_with_migrate(old_ft,
>> +				MIGRATE_MOVABLE);
>> +
>>  			/*
>>  			 * Only change normal pageblocks (i.e., they can merge
>>  			 * with others)
>>  			 */
>> -			if (migratetype_is_mergeable(mt))
>> -				move_freepages_block(zone, page, mt,
>> -						     MIGRATE_MOVABLE);
>> +			if (migratetype_is_mergeable(free_to_migratetype(ft)))
>> +				move_freepages_block(zone, page, old_ft, new_ft);
>
> While poking at the code with AI, it pointed out that new_ft here may be
> an invalid freetype (e.g. unmapped movable). I don't think anything in
> move_freepages_block() or its callees checks against this. There may not
> be an actual code path that would lead to this, 

Yeah I think the bug is impossible in practice because of the
freetype-flags(a) == freetype_flags(b) in can_merge_freetypes()...

> but it's very subtle.

... but yeah. Also there will be other bugs in this form (failure to
handle a no-freelist freetype), I have had several of them already while
developing these patches. I have specifically asked AI to look for them
and it didn't find this one. (Well, it's actually also quite likely
it did but I didn't read it properly or something).

> We can add a check to can_merge_freetypes() (introduced in later
> patches) to check for invalid types.
>
> But I think we may actually want a check in prep_move_freepages_block(),
> which is called by move_freepages_block() and others before moving a
> pageblock. WDYT?

Yeah I think that's the answer, we just have to put those checks in the
right places. 

FWIW my original nerd instinct here was to create some freelist
abstraction that makes the problem go away by making "no freelist of
that type" look just like an empty freelist. But aside from being more
galaxy-brained code to review, that doesn't work here at all:
move_to_free_list() targeting a nonexistent freetype is inherently a
bug.