[PATCH v6] malloc: Mark pages with MADV_DONTNEED to shrink and grow rather than mremap()

[Wilco Dijkstra]

Hi William,

>> Then if mremap fails/doesn't exist, we still use madvise since that does reduce
>> actual memory usage.
> This works for Linux, since mremap() is supported and it's very unlikely even
> if overcommitting is disabled that mremap() will fail enough times to run out
> of pages. For non-Linux systems this would cause madvise() to be called if we
> use it as a backup since mremap() is Linux-specific. Solaris disables
> overcommit memory by default, and FreeBSD and its derivates allow disabling.
> However, I assume we are not developing support for them since they use their
> own C libraries and I'm not sure if glibc can even be used on these OSs?
 
The goal is for the code to be easy to understand. The extra check is to prefer mremap,
not to block madvise (which we should always use whenever feasible to reduce
memory consumption). This is essentially the same kind of test as the check on the
size, so it belongs with that. 

> I'll fix this so that may_shrink_heap is check for being non-negative, then
> assigned to the function call. A better name for check_can_keep_vma() would be
> check_cannot_overcommit().

Avoid names with "not" in them - !check_cannot_overcommit() becomes very confusing...

>> Florian: I am not sure what the __libc_enable_secure has to do with overcommit - so
>> I am wondering whether we could remove that check altogether?
> I think the intention is to remove access to the pages, so a process running
> with extra privileges doesn't expose data after using a secure exec to a less
> privileged process. MADV_DONTNEED uses zero-fill-on-demand pages for anonymous
> private mappings, so an madvise() works for mmap'd chunks and non-main arenas.
> The main arena uses MORECORE which by default is sbrk, and the process' heap
> is anonymous and private so this should also give zero-fill-on-demand pages.
> However, if MORECORE is defined to something like shm_open() then the memory
> contents would be repopulated from the underlying mapped file. I'm not sure if
> something like that could be supported by malloc though?

Currently this check is only used on heaps, so it is not needed for sbrk.

And yes, MADV_DONTNEED should be sufficient to remove the pages, so it is "secure" enough.

Cheers,
Wilco