On Thu, Oct 04, 2018 at 01:16:32PM -0700, David Rientjes wrote:
On Tue, 25 Sep 2018, Michal Hocko wrote:
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index da858f794eb6..149b6f4cf023 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -2046,8 +2046,36 @@ alloc_pages_vma(gfp_t gfp, int order, struct vm_area_struct *vma, nmask = policy_nodemask(gfp, pol); if (!nmask || node_isset(hpage_node, *nmask)) { mpol_cond_put(pol);
page = __alloc_pages_node(hpage_node,gfp | __GFP_THISNODE, order);
/** We cannot invoke reclaim if __GFP_THISNODE* is set. Invoking reclaim with* __GFP_THISNODE set, would cause THP* allocations to trigger heavy swapping* despite there may be tons of free memory* (including potentially plenty of THP* already available in the buddy) on all the* other NUMA nodes.** At most we could invoke compaction when* __GFP_THISNODE is set (but we would need to* refrain from invoking reclaim even if* compaction returned COMPACT_SKIPPED because* there wasn't not enough memory to succeed* compaction). For now just avoid* __GFP_THISNODE instead of limiting the* allocation path to a strict and single* compaction invocation.** Supposedly if direct reclaim was enabled by* the caller, the app prefers THP regardless* of the node it comes from so this would be* more desiderable behavior than only* providing THP originated from the local* node in such case.*/if (!(gfp & __GFP_DIRECT_RECLAIM))gfp |= __GFP_THISNODE;page = __alloc_pages_node(hpage_node, gfp, order); goto out;This causes, on average, a 13.9% access latency regression on Haswell, and the regression would likely be more severe on Naples and Rome.
That assumes that fragmentation prevents easy allocation which may very well be the case. While it would be great that compaction or the page allocator could be further improved to deal with fragmentation, it's outside the scope of this patch.
There exist libraries that allow the .text segment of processes to be remapped to memory backed by transparent hugepages and use MADV_HUGEPAGE to stress local compaction to defragment node local memory for hugepages at startup.
That is taking advantage of a co-incidence of the implementation. MADV_HUGEPAGE is *advice* that huge pages be used, not what the locality is. A hint for strong locality preferences should be separate advice (madvise) or a separate memory policy. Doing that is outside the context of this patch but nothing stops you introducing such a policy or madvise, whichever you think would be best for the libraries to consume (I'm only aware of libhugetlbfs but there might be others).
The cost, including the statistics Mel gathered, is acceptable for these processes: they are not concerned with startup cost, they are concerned only with optimal access latency while they are running.
Then such applications at startup have the option of setting zone_reclaim_mode during initialisation assuming a privileged helper can be created. That would be somewhat heavy handed and a longer-term solution would still be to create a proper memory policy of madvise flag for those libraries.
So while it may take longer to start the process because memory compaction is attempting to allocate hugepages with __GFP_DIRECT_RECLAIM, in the cases where compaction is successful, this is a very significant long-term win. In cases where compaction fails, falling back to local pages of the native page size instead of remote thp is a win for the remaining time this process wins: as stated, 13.9% faster for all memory accesses to the process's text while it runs on Haswell.
Again, I remind you that it only benefits applications that prefectly fit into NUMA nodes. Not all applications are created with that level of awareness and easily get thrashed if using MADV_HUGEPAGE and do not fit into a NUMA node.
While it is unfortunate that there are specialised applications that benefit from the current configuration, I bet there is heavier usage of qemu affected by the bug this patch addresses than specialised applications that both fit perfectly into NUMA nodes and are extremely sensitive to access latencies. It's a question of causing the least harm to the most users which is what this patch does.
If you need behaviour for more agressive reclaim or locality hints then kindly introduce them and do not depend in MADV_HUGEPAGE accidentically doubling up as hints about memory locality.