Re: [PATCH 03/11] mm/hugetlb: introduce alloc_control structure to simplify migration target allocation APIs

[Mike Kravetz]

On 5/17/20 6:20 PM, js1304@xxxxxxxxx wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> 
> Currently, page allocation functions for migration requires some arguments.
> More worse, in the following patch, more argument will be needed to unify
> the similar functions. To simplify them, in this patch, unified data
> structure that controls allocation behaviour is introduced.

As a followup to Roman's question and your answer about adding a suffix/prefix
to the new structure.  It 'may' be a bit confusing as alloc_context is already
defined and *ac is passsed around for page allocations.  Perhaps, this new
structure could somehow have migrate in the name as it is all about allocating
migrate targets?

> 
> For clean-up, function declarations are re-ordered.
> 
> Note that, gfp_mask handling on alloc_huge_page_(node|nodemask) is
> slightly changed, from ASSIGN to OR. It's safe since caller of these
> functions doesn't pass extra gfp_mask except htlb_alloc_mask().
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>

Patch makes sense.

> diff --git a/mm/migrate.c b/mm/migrate.c
> index a298a8c..94d2386 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1526,10 +1526,15 @@ struct page *new_page_nodemask(struct page *page,
>  	unsigned int order = 0;
>  	struct page *new_page = NULL;
>  
> -	if (PageHuge(page))
> -		return alloc_huge_page_nodemask(
> -				page_hstate(compound_head(page)),
> -				preferred_nid, nodemask);
> +	if (PageHuge(page)) {
> +		struct hstate *h = page_hstate(page);

I assume the removal of compound_head(page) was intentional?  Just asking
because PageHuge will look at head page while page_hstate will not.  So,
if passed a non-head page things could go bad.

-- 
Mike Kravetz