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

Yosry Ahmed <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
> @@ -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?

>  		}
>  	}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.