Re: [PATCH v3 2/2] mm: Make alloc_contig_range handle in-use hugetlb pages

[Oscar Salvador]

On Fri, Feb 26, 2021 at 01:46:21PM +0100, Michal Hocko wrote:
> Well, I will leave it to others. I do not feel strongly about this but
> to me it makes the code harder to think about because the situation is
> unstable and any of those condition can change as they are evaluated. So
> an explicit checks makes the code harder in the end. I would simply got
> with 
> 	if (isolate_huge_page(head, list) || !alloc_and_dissolve_huge_page())
> 		ret = true;
> 
> if either of the conditional needs a retry then it should be done
> internally. Like alloc_and_dissolve_huge_page already does to stabilize
> the PageFreed flag. An early bail out on non-free hugetlb page would
> also better be done inside alloc_and_dissolve_huge_page.

The retry could be done internally in alloc_and_dissolve_huge_page in
case someoen grabbed the page from under us, but calling
isolate_huge_page from there seemed a bit odd to me, that is why I
placed the logic in the outter function.
It looks more logic to me, but of course, that is just my taste.

I do not think it makes the code that hard to follow, but I will leave
it to the others.
If there is a consensus that a simplistic version is prefered, I do not
have a problem to go with that.

Mike, what is your take on this?

Thanks


-- 
Oscar Salvador
SUSE L3