On Wed, Aug 19, 2020 at 4:43 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Tue, Aug 18, 2020 at 09:27:05PM -0700, Alexander Duyck wrote:
> > + /*
> > + * Page is compound. We know the order before we know if it is
> > + * on the LRU so we cannot assume it is THP. However since the
> > + * page will have the LRU validated shortly we can use the value
> > + * to skip over this page for now or validate the LRU is set and
> > + * then isolate the entire compound page if we are isolating to
> > + * generate a CMA page.
> > + */
> > + if (PageCompound(page)) {
> > + const unsigned int order = compound_order(page);
> > +
> > + if (likely(order < MAX_ORDER))
> > + low_pfn += (1UL << order) - 1;
>
> Hmm. You're checking for PageCompound but then skipping 1UL << order.
> That only works if PageHead. If instead this is PageCompound because
> it's PageTail, you need to do something like:
>
> low_pfn |= (1UL << order) - 1;
>
> which will move you to the end of the page you're in the middle of.
Can you successfully call get_page_unless_zero in a tail page? I
thought their reference count was 0? There is a get_page_unless_zero
call before the PageCompound check, so I don't think we can get a tail
page.
> If PageTail can't actually happen here, then it's better to check for
> PageHead explicitly and WARN_ON if you get a PageTail (eg a page was
> combined into a compound page after you processed the earlier head page).
>
> Is it possible the page you've found is hugetlbfs? Those can have orders
> larger than MAX_ORDER.
So in theory we only need to jump pageblock_order. However there are
some architectures where that is not a fixed constant and so it would
have some additional overhead if I am not mistaken. In addition we
should have been only provided a pageblock if i am not mistaken so the
check further down that prevents low_pfn from passing end_pfn should
reset to the correct value.
