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

[Yosry Ahmed <yosry@kernel.org>]

> @@ -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, but it's very subtle.

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?

>  		}
>  	}