Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200819041852.23414.95939.stgit@localhost.localdomain>
 <20200819042705.23414.84098.stgit@localhost.localdomain> <20200819114309.GB17456@casper.infradead.org>
In-Reply-To: <20200819114309.GB17456@casper.infradead.org>
From:   Alexander Duyck <alexander.duyck@gmail.com>
Date:   Wed, 19 Aug 2020 07:48:11 -0700
Message-ID: <CAKgT0UfVdaMfg=TiiqOK1axUdmViEeaU+R8sisf5WpOKqa-65w@mail.gmail.com>
Subject: Re: [RFC PATCH v2 1/5] mm: Identify compound pages sooner in isolate_migratepages_block
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Alex Shi <alex.shi@linux.alibaba.com>,
        Yang Shi <yang.shi@linux.alibaba.com>,
        kbuild test robot <lkp@intel.com>,
        Rong Chen <rong.a.chen@intel.com>,
        Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
        "Kirill A. Shutemov" <kirill@shutemov.name>,
        Hugh Dickins <hughd@google.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Daniel Jordan <daniel.m.jordan@oracle.com>,
        linux-mm <linux-mm@kvack.org>,
        Shakeel Butt <shakeelb@google.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Tejun Heo <tj@kernel.org>, cgroups@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Wei Yang <richard.weiyang@gmail.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Aug 19, 2020 at 4:43 AM Matthew Wilcox <willy@infradead.org> 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.