On Wed, Apr 21, 2021 at 4:33 PM HORIGUCHI NAOYA(堀口 直也) <naoya.horiguchi@xxxxxxx> wrote: > > On Wed, Apr 21, 2021 at 10:03:34AM +0200, Michal Hocko wrote: > > [Cc Naoya] > > > > On Wed 21-04-21 14:02:59, Muchun Song wrote: > > > The possible bad scenario: > > > > > > CPU0: CPU1: > > > > > > gather_surplus_pages() > > > page = alloc_surplus_huge_page() > > > memory_failure_hugetlb() > > > get_hwpoison_page(page) > > > __get_hwpoison_page(page) > > > get_page_unless_zero(page) > > > zero = put_page_testzero(page) > > > VM_BUG_ON_PAGE(!zero, page) > > > enqueue_huge_page(h, page) > > > put_page(page) > > > > > > The refcount can possibly be increased by memory-failure or soft_offline > > > handlers, we can trigger VM_BUG_ON_PAGE and wrongly add the page to the > > > hugetlb pool list. > > > > The hwpoison side of this looks really suspicious to me. It shouldn't > > really touch the reference count of hugetlb pages without being very > > careful (and having hugetlb_lock held). > > I have the same feeling, there is a window where a hugepage is refcounted > during converting from buddy free pages into free hugepage, so refcount > alone is not enough to prevent the race. hugetlb_lock is retaken after > alloc_surplus_huge_page returns, so simply holding hugetlb_lock in > get_hwpoison_page() seems not work. Is there any status bit to show that a > hugepage is just being initialized (not in free hugepage pool or in use)? HPageFreed() can indicate whether a page is on the free pool list. > > > What would happen if the > > reference count was increased after the page has been enqueed into the > > pool? This can just blow up later. > > Yes, this is another concern. > > Thanks, > Naoya Horiguchi
