On 1/27/21 2:25 AM, Michal Hocko wrote: > On Fri 22-01-21 11:52:28, 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 to the page cache and/or task page table. This is the reasoning >> behind the name change. > > yeah, definitely less confusing! > >> The VM_BUG_ON_PAGE() calls in the *_huge_active() interfaces are not >> really necessary as we KNOW the page is a hugetlb page. Therefore, they >> are removed. >> >> The routine page_huge_active checked for PageHeadHuge before testing the >> active bit. This is unnecessary in the case where we hold a reference or >> lock and know it is a hugetlb head page. page_huge_active is also called >> without holding a reference or lock (scan_movable_pages), and can race with >> code freeing the page. The extra check in page_huge_active shortened the >> race window, but did not prevent the race. Offline code calling >> scan_movable_pages already deals with these races, so removing the check >> is acceptable. Add comment to racy code. >> >> Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> >> Reviewed-by: Oscar Salvador <osalvador@xxxxxxx> >> Reviewed-by: Muchun Song <songmuchun@xxxxxxxxxxxxx> >> Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx> > > Acked-by: Michal Hocko <mhocko@xxxxxxxx> > > One nit below > [...] >> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h >> index a7eb05315c6e..58be44a915d1 100644 >> --- a/include/linux/hugetlb.h >> +++ b/include/linux/hugetlb.h >> @@ -480,9 +480,13 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr, >> * HPG_restore_reserve - Set when a hugetlb page consumes a reservation at >> * allocation time. Cleared when page is fully instantiated. Free >> * routine checks flag to restore a reservation on error paths. >> + * HPG_migratable - Set after a newly allocated page is added to the page >> + * cache and/or page tables. Indicates the page is a candidate for >> + * migration. > > The state change is synchronized by hugetlb_lock. > > Pls. I will update/explain this. But ... hugetlb_lock does not synchronize all changes of this flag. The flag is set without holding the lock for newly allocated pages after being added to page cache and/or page tables. This 'signals' the page is a candidate for migration. When the migration code (isolation, putback, move state) deals with the flag, it does hold the hugetlb_lock. This patch did not change any of the synchronization. -- Mike Kravetz
