On 1/21/21 10:53 PM, Miaohe Lin wrote: > Hi: > On 2021/1/20 9:30, Mike Kravetz wrote: >> Use the new hugetlb page specific flag HPageMigratable to replace the >> page_huge_active interfaces. By it's name, page_huge_active implied >> that a huge page was on the active list. However, that is not really >> what code checking the flag wanted to know. It really wanted to determine >> if the huge page could be migrated. This happens when the page is actually >> added the page cache and/or task page table. This is the reasoning behind > > s/added/added to/ Thanks > >> the name change. >> ... >> @@ -1260,7 +1260,13 @@ static int scan_movable_pages(unsigned long start, unsigned long end, >> if (!PageHuge(page)) >> continue; >> head = compound_head(page); >> - if (page_huge_active(head)) >> + /* >> + * This test is racy as we hold no reference or lock. The >> + * hugetlb page could have been free'ed and head is no longer >> + * a hugetlb page before the following check. In such unlikely >> + * cases false positives and negatives are possible. >> + */ > > Is it necessary to mention that: "offline_pages() could handle these racy cases." in the comment ? > I will enhance the comment to say that calling code must deal with these possible scenarios. -- Mike Kravetz >> + if (HPageMigratable(head)) >> goto found; >> skip = compound_nr(head) - (page - head); >> pfn += skip - 1; >> > > Looks good to me. Thanks. > > Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx> >
