Re: [PATCH v2 09/22] mm/page_alloc: don't overload migratetype in find_suitable_fallback()

["Gupta, Pankaj" <pankaj.gupta@amd.com>]

> This function currently returns a signed integer that encodes status
> in-band, as negative numbers, along with a migratetype.
>
> This function is about to be updated to a mode where this in-band
> signaling no longer makes sense. Therefore, switch to a more
> explicit/verbose style that encodes the status and migratetype
> separately.
>
> In the spirit of making things more explicit, also create an enum to
> avoid using magic integer literals with special meanings. This enables
> documenting the values at their definition instead of in one of the
> callers.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>

Seems independent patch with no functional change.

Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>

> ---
>   mm/compaction.c |  3 ++-
>   mm/internal.h   | 14 +++++++++++---
>   mm/page_alloc.c | 40 +++++++++++++++++++++++-----------------
>   3 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 32623894a6327..25371a75471dd 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2358,7 +2358,8 @@ static enum compact_result __compact_finished(struct compact_control *cc)
>   		 * Job done if allocation would steal freepages from
>   		 * other migratetype buddy lists.
>   		 */
> -		if (find_suitable_fallback(area, order, migratetype, true) >= 0)
> +		if (find_suitable_fallback(area, order, migratetype, true, NULL)
> +		    == FALLBACK_FOUND)
>   			/*
>   			 * Movable pages are OK in any pageblock. If we are
>   			 * stealing for a non-movable allocation, make sure
> diff --git a/mm/internal.h b/mm/internal.h
> index f4c59534670e4..e3782721a588b 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1059,9 +1059,17 @@ static inline void init_cma_pageblock(struct page *page)
>   }
>   #endif
>   
> -
> -int find_suitable_fallback(struct free_area *area, unsigned int order,
> -			   int migratetype, bool claimable);
> +enum fallback_result {
> +	/* Found suitable migratetype, *mt_out is valid. */
> +	FALLBACK_FOUND,
> +	/* No fallback found in requested order. */
> +	FALLBACK_EMPTY,
> +	/* Passed @claimable, but claiming whole block is a bad idea. */
> +	FALLBACK_NOCLAIM,
> +};
> +enum fallback_result
> +find_suitable_fallback(struct free_area *area, unsigned int order,
> +		       int migratetype, bool claimable, unsigned int *mt_out);
>   
>   static inline bool free_area_empty(struct free_area *area, int migratetype)
>   {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0a1dc7866068f..ac077d98019f3 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2248,25 +2248,29 @@ static bool should_try_claim_block(unsigned int order, int start_mt)
>    * we would do this whole-block claiming. This would help to reduce
>    * fragmentation due to mixed migratetype pages in one pageblock.
>    */
> -int find_suitable_fallback(struct free_area *area, unsigned int order,
> -			   int migratetype, bool claimable)
> +enum fallback_result
> +find_suitable_fallback(struct free_area *area, unsigned int order,
> +		       int migratetype, bool claimable, unsigned int *mt_out)
>   {
>   	int i;
>   
>   	if (claimable && !should_try_claim_block(order, migratetype))
> -		return -2;
> +		return FALLBACK_NOCLAIM;
>   
>   	if (area->nr_free == 0)
> -		return -1;
> +		return FALLBACK_EMPTY;
>   
>   	for (i = 0; i < MIGRATE_PCPTYPES - 1 ; i++) {
>   		int fallback_mt = fallbacks[migratetype][i];
>   
> -		if (!free_area_empty(area, fallback_mt))
> -			return fallback_mt;
> +		if (!free_area_empty(area, fallback_mt)) {
> +			if (mt_out)
> +				*mt_out = fallback_mt;
> +			return FALLBACK_FOUND;
> +		}
>   	}
>   
> -	return -1;
> +	return FALLBACK_EMPTY;
>   }
>   
>   /*
> @@ -2376,16 +2380,16 @@ __rmqueue_claim(struct zone *zone, int order, int start_migratetype,
>   	 */
>   	for (current_order = MAX_PAGE_ORDER; current_order >= min_order;
>   				--current_order) {
> -		area = &(zone->free_area[current_order]);
> -		fallback_mt = find_suitable_fallback(area, current_order,
> -						     start_migratetype, true);
> +		enum fallback_result result;
>   
> -		/* No block in that order */
> -		if (fallback_mt == -1)
> +		area = &(zone->free_area[current_order]);
> +		result = find_suitable_fallback(area, current_order,
> +						start_migratetype, true, &fallback_mt);
> +
> +		if (result == FALLBACK_EMPTY)
>   			continue;
>   
> -		/* Advanced into orders too low to claim, abort */
> -		if (fallback_mt == -2)
> +		if (result == FALLBACK_NOCLAIM)
>   			break;
>   
>   		page = get_page_from_free_area(area, fallback_mt);
> @@ -2415,10 +2419,12 @@ __rmqueue_steal(struct zone *zone, int order, int start_migratetype)
>   	int fallback_mt;
>   
>   	for (current_order = order; current_order < NR_PAGE_ORDERS; current_order++) {
> +		enum fallback_result result;
> +
>   		area = &(zone->free_area[current_order]);
> -		fallback_mt = find_suitable_fallback(area, current_order,
> -						     start_migratetype, false);
> -		if (fallback_mt == -1)
> +		result = find_suitable_fallback(area, current_order, start_migratetype,
> +						false, &fallback_mt);
> +		if (result == FALLBACK_EMPTY)
>   			continue;
>   
>   		page = get_page_from_free_area(area, fallback_mt);
>