On Mon, Aug 31, 2020 at 04:06:54PM -0700, Mike Kravetz wrote: >On 8/30/20 7:23 PM, Wei Yang wrote: >> Function dequeue_huge_page_node_exact() iterates the free list and >> return the first valid free hpage. >> >> Instead of break and check the loop variant, we could return in the loop >> directly. This could reduce some redundant check. >> >> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx> >> --- >> mm/hugetlb.c | 20 ++++++++------------ >> 1 file changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 7b3357c1dcec..709be7ab65af 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -1040,21 +1040,17 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid) >> if (nocma && is_migrate_cma_page(page)) >> continue; >> >> - if (!PageHWPoison(page)) >> + if (PageHWPoison(page)) >> break; > >Previously, when encountering a PageHWPoison(page) the loop would continue >and check the next page in the list. It now breaks the loop and returns >NULL. Is not this a change in behavior? Perhaps you want to change that >break to a continue. Or, restructure in some other way. Shame on me. You are right. I should change it to continue. Andrew, Sorry for the inconvenience. >-- >Mike Kravetz > >> + >> + list_move(&page->lru, &h->hugepage_activelist); >> + set_page_refcounted(page); >> + h->free_huge_pages--; >> + h->free_huge_pages_node[nid]--; >> + return page; >> } >> >> - /* >> - * if 'non-isolated free hugepage' not found on the list, >> - * the allocation fails. >> - */ >> - if (&h->hugepage_freelists[nid] == &page->lru) >> - return NULL; >> - list_move(&page->lru, &h->hugepage_activelist); >> - set_page_refcounted(page); >> - h->free_huge_pages--; >> - h->free_huge_pages_node[nid]--; >> - return page; >> + return NULL; >> } >> >> static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask, int nid, >> -- Wei Yang Help you, Help me
