Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 3 Dec 2018 12:36:09 -0800 (PST)
From:   David Rientjes <rientjes@google.com>
To:     Andrea Arcangeli <aarcange@redhat.com>
cc:     Linus Torvalds <torvalds@linux-foundation.org>, mhocko@kernel.org,
        ying.huang@intel.com, s.priebe@profihost.ag,
        mgorman@techsingularity.net,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
        alex.williamson@redhat.com, lkp@01.org, kirill@shutemov.name,
        Andrew Morton <akpm@linux-foundation.org>,
        zi.yan@cs.rutgers.edu, Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [LKP] [mm] ac5b2c1891: vm-scalability.throughput -61.3%
 regression
In-Reply-To: <20181203201214.GB3540@redhat.com>
Message-ID: <alpine.DEB.2.21.1812031226510.192288@chino.kir.corp.google.com>
References: <20181127205737.GI16136@redhat.com> <87tvk1yjkp.fsf@yhuang-dev.intel.com> <CAHk-=wjgRO-=NPaU9EmrdC3it3o7kvf4u7sewv3crtNLkE13Hg@mail.gmail.com> <CAHk-=wjgpWOA7zQ9H5=Zj6KQijm5CBXZc7J=it6C5gdEV0hb5Q@mail.gmail.com> <20181203181456.GK31738@dhcp22.suse.cz>
 <CAHk-=whrfDw4yV4h2ijbX3vpXf5m4hzJ5pGX7_v6pU31RGib-g@mail.gmail.com> <20181203183050.GL31738@dhcp22.suse.cz> <CAHk-=wgVL_sxXSbjYTiGhxp6+9wLQ9ZmSN+0R5PCF6_a9pQgWw@mail.gmail.com> <20181203185954.GM31738@dhcp22.suse.cz> <CAHk-=wiNKLH2Pbnr9z2SvmDhf7XT==U6NPRkQNX13Sg-FRk0Yw@mail.gmail.com>
 <20181203201214.GB3540@redhat.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 3 Dec 2018, Andrea Arcangeli wrote:

> In my earlier review of David's patch, it looked runtime equivalent to
> the __GFP_COMPACT_ONLY solution. It has the only advantage of adding a
> new gfpflag until we're sure we need it but it's the worst solution
> available for the long term in my view. It'd be ok to apply it as
> stop-gap measure though.
> 
> The "order == pageblock_order" hardcoding inside the allocator to
> workaround the __GFP_THISNODE flag passed from outside the allocator
> in the THP MADV_HUGEPAGE case, didn't look very attractive because
> it's not just THP allocating order >0 pages.
> 

We have two different things to consider: NUMA locality and the order of 
the allocation.  THP is preferred locally and we know the order.  For the 
other high-order pages you're referring to, I don't know if they are using 
__GFP_THISNODE or not (likely not).  I see them as two separate issues.

For thp on all platforms I have measured it on specifically for this patch 
(Broadwell, Haswell, Rome) there is a clear advantage to faulting local 
pages of the native page size over remote hugepages.  It also has the 
added effect of allowing khugepaged to collapse it into a hugepage later 
if fragmentation allows (the reason why khugepaged cares about NUMA 
locality, the same reason I do).  This is the rationale for __GFP_THISNODE 
for thp allocations.

For order == pageblock_order (or more correctly order >= pageblock_order), 
this is not based on NUMA whatsoever but is rather based on the 
implementation of memory compaction.  If it has already failed (or was 
deferred for order-HPAGE_PMD_ORDER), reclaim cannot be shown to help if 
memory compaction cannot utilize the freed memory in isolate_freepages(), 
so that reclaim has been pointless.  If compaction fails for other reasons 
(any unmovable page preventing a pageblock from becoming free), *all* 
reclaim activity has been pointless.  

> It'd be nicer if whatever compaction latency optimization that applies
> to THP could also apply to all other allocation orders too and the
> hardcoding of the THP order prevents that.
> 
> On the same lines if __GFP_THISNODE is so badly needed by
> MADV_HUGEPAGE, all other larger order allocations should also be able
> to take advantage of __GFP_THISNODE without ending in the same VM
> corner cases that required the "order == pageblock_order" hardcoding
> inside the allocator.
> 
> If you prefer David's patch I would suggest pageblock_order to be
> replaced with HPAGE_PMD_ORDER so it's more likely to match the THP
> order in all archs.
> 

That sounds fine and I will do that in my v2.