[PATCH] Revert "malloc: Do not call madvise if heap's oldsize >= THP size"

[Dev Jain]

This reverts commit 6e8f32d39a57aa1f31bf15375810aab79a0f5f4b.

First off, apologies for my misunderstanding on how madvise(MADV_HUGEPAGE)
works. I had the misconception that doing madvise(p, 1, MADV_HUGEPAGE) will set
VM_HUGEPAGE on the entire VMA - it does not, it will align the size to
PAGE_SIZE (4k) and then *split* the VMA. Only the first page-length of the
virtual space will VM_HUGEPAGE'd, the rest of it will stay the same.

The above is the semantics for all madvise() calls - which makes sense from a
UABI perspective. madvise() should do the proposed thing to only the length
(page-aligned) which it was asked to do, doing any more than that is not
something the user is expecting.

Commit 6e8f32d39a57 tries to optimize around the madvise() call by determining
whether the VMA got madvise'd before. This will work for most cases except
the following: if check_may_shrink_heap() is true, shrink_heap() re-maps the
shrunk portion, giving us a new VMA altogether. That VMA won't have the
VM_HUGEPAGE flag.

Reverting this commit, we will again mark the new VMA with VM_HUGEPAGE, and
the kernel will merge the two into a single VMA marked with VM_HUGEPAGE.

This may be the only case where we lose VM_HUGEPAGE, and we could micro-optimize
by extending the current if-condition with !check_may_shrink_heap. But let us
not do this - this is very difficult to reason about, and I am soon going
to propose mmap(MAP_HUGEPAGE) in Linux to do away with all these workarounds.

---
No regression observed on SPEC. I examined my other patch doing a similar
optimization in mremap_chunk() and that looks fine to me - mremap() does not
drop VMA flags.

 malloc/arena.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/malloc/arena.c b/malloc/arena.c
index 75f2f32b5f..03a812f54d 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -486,10 +486,7 @@ grow_heap (heap_info *h, long diff)
       h->mprotect_size = new_size;
     }
 
-  /* mprotect preserves MADV_HUGEPAGE semantics - this means that if the old
-     region was marked with MADV_HUGEPAGE, the new region will retain that.  */
-  if (h->size < mp_.thp_pagesize)
-    madvise_thp (h, new_size);
+  madvise_thp (h, new_size);
 
   h->size = new_size;
   LIBC_PROBE (memory_heap_more, 2, h, h->size);
-- 
2.43.0