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
