On 2/25/20 7:44 PM, Rik van Riel wrote: >> Also PageTransHuge() is basically just a PageHead() so for each >> non-hugetlbfs compound page this will assume it's a THP, while >> correctly >> it should reach the __PageMovable() || PageLRU(page) tests below. >> >> So probably this should do something like. >> >> if (PageHuge(page) || PageTransCompound(page)) { >> ... >> if (PageHuge(page) && !hpage_migration_supported)) return page. > > So far so good. > >> if (!PageLRU(head) && !__PageMovable(head)) return page > > I don't get this one, though. What about a THP that has > not made it onto the LRU list yet for some reason? Uh, is it any different from base pages which have to pass the same check? I guess the caller could do e.g. lru_add_drain_all() first. > I don't think anonymous pages are marked __PageMovable, > are they? It looks like they only have the PAGE_MAPPING_ANON > flag set, not the PAGE_MAPPING_MOVABLE one. > > What am I missing? My point is that we should not accept compound pages that are neither a migratable hugetlbfs page nor a THP, as movable. And your PageTransHuge() test and my PageTransCompound() is really just a test for all compound pages, not "hugetlbfs or THP only". I should have perhaps suggested PageCompound() instead of the PageTransCompound() wrapper, to make it more obvious. So we should test non-hugetlbfs pages first whether they are the kind of compound pages that are migratable. THP's should pass this test by PageLRU(), other compound movable pages by __PageMovable(head). > >> ... >> >> > struct page *head = compound_head(page); >> > unsigned int skip_pages; >> > >> > - if >> > (!hugepage_migration_supported(page_hstate(head))) >> > + if (PageHuge(page) && >> > + !hugepage_migration_supported(page_hstate(h >> > ead))) >> > return page; >> > >> > skip_pages = compound_nr(head) - (page - head); >> > >> >> >