On Mon, Oct 15, 2018 at 03:30:17PM -0700, David Rientjes wrote:
At the risk of beating a dead horse that has already been beaten, what are the plans for this patch when the merge window opens? It would be rather unfortunate for us to start incurring a 14% increase in access latency and 40% increase in fault latency. Would it be possible to test with my patch[*] that does not try reclaim to address the thrashing issue? If that is satisfactory, I don't have a strong preference if it is done with a hardcoded pageblock_order and __GFP_NORETRY check or a new __GFP_COMPACT_ONLY flag.
I don't like the pageblock size hardcoding inside the page allocator. __GFP_COMPACT_ONLY is fully runtime equivalent, but it at least let the caller choose the behavior, so it looks more flexible.
To fix your 40% fault latency concern in the short term we could use __GFP_COMPACT_ONLY, but I think we should get rid of __GFP_COMPACT_ONLY later: we need more statistical data in the zone structure to track remote compaction failures triggering because the zone is fully fragmented.
Once the zone is fully fragmented we need to do a special exponential backoff on that zone when the zone is from a remote node.
Furthermore at the first remote NUMA node zone failure caused by full fragmentation we need to interrupt compaction and stop trying with all remote nodes.
As long as compaction returns COMPACT_SKIPPED it's ok to keep doing reclaim and keep doing compaction, as long as compaction succeeds.
What is causing the higher latency is the fact we try all zones from all remote nodes even if there's a failure caused by full fragmentation of all remote zone, and we're unable to skip (skip with exponential backoff) only those zones where compaction cannot succeed because of fragmentation.
Once we achieve the above deleting __GFP_COMPACT_ONLY will be a trivial patch.
I think the second issue of faulting remote thp by removing __GFP_THISNODE needs supporting evidence that shows some platforms benefit from this (and not with numa=fake on the command line :).
That is needed to compare the current one liner fix with __GFP_COMPACT_ONLY, but I don't think it's needed to compare v4.18 with the current fix. The badness of v4.18 was too bad keep, getting local PAGE_SIZEd memory or remote THPs is a secondary issue.
In fact the main reason for __GFP_COMPACT_ONLY is not anymore such tradeoff, but not to spend too much CPU in compaction when all nodes are fragmented to avoid increasing the allocation latency too much.
Thanks, Andrea