Re: [PATCH 2/2] Revert "mm, thp: restore node-local hugepage allocations"

[Michal Hocko]

On Thu 06-06-19 15:12:40, David Rientjes wrote:
> On Wed, 5 Jun 2019, Michal Hocko wrote:
> 
> > > That's fine, but we also must be mindful of users who have used 
> > > MADV_HUGEPAGE over the past four years based on its hard-coded behavior 
> > > that would now regress as a result.
> > 
> > Absolutely, I am all for helping those usecases. First of all we need to
> > understand what those usecases are though. So far we have only seen very
> > vague claims about artificial worst case examples when a remote access
> > dominates the overall cost but that doesn't seem to be the case in real
> > life in my experience (e.g. numa balancing will correct things or the
> > over aggressive node reclaim tends to cause problems elsewhere etc.).
> > 
> 
> The usecase is a remap of a binary's text segment to transparent hugepages 
> by doing mmap() -> madvise(MADV_HUGEPAGE) -> mremap() and when this 
> happens on a locally fragmented node.  This happens at startup when we 
> aren't concerned about allocation latency: we want to compact.  We are 
> concerned with access latency thereafter as long as the process is 
> running.

You have indicated this previously but no call for a stand alone
reproducer was successful. It is really hard to optimize for such a
specialized workload without anything to play with. Btw. this is exactly
a case where I would expect numa balancing to converge to the optimal
placement. And if numabalancing is not an option than an explicit
mempolicy (e.g. the one suggested here) would be a good fit.

[...]

I will defer the compaction related stuff to Vlastimil and Mel who are
much more familiar with the current code.

> So my proposed change would be:
>  - give the page allocator a consistent indicator that compaction failed
>    because we are low on memory (make COMPACT_SKIPPED really mean this),
>  - if we get this in the page allocator and we are allocating thp, fail,
>    reclaim is unlikely to help here and is much more likely to be
>    disruptive
>      - we could retry compaction if we haven't scanned all memory and
>        were contended,
>  - if the hugepage allocation fails, have thp check watermarks for order-0 
>    pages without any padding,
>  - if watermarks succeed, fail the thp allocation: we can't allocate
>    because of fragmentation and it's better to return node local memory,

Doesn't this lead to the same THP low success rate we have seen with one
of the previous patches though?

Let me remind you of the previous semantic I was proposing
http://lkml.kernel.org/r/20181206091405.GD1286@xxxxxxxxxxxxxx and that
didn't get shot down. Linus had some follow up ideas on how exactly
the fallback order should look like and that is fine. We should just
measure differences between local node cheep base page vs. remote THP on
_real_ workloads. Any microbenchmark which just measures a latency is
inherently misleading.

And really, fundamental problem here is that MADV_HUGEPAGE has gained 
a NUMA semantic without a due scrutiny leading to a broken interface
with side effects that are simply making the interface unusable for a
large part of usecases that the madvise was originaly designed for.
Until we find an agreement on this point we will be looping in a dead
end discussion, I am afraid.

-- 
Michal Hocko
SUSE Labs