Re: [PATCH v1 8/9] util/mmap-alloc: support RAM_NORESERVE via MAP_NORESERVE

[Peter Xu <peterx@redhat.com>]

On Tue, Mar 02, 2021 at 08:01:11PM +0100, David Hildenbrand wrote:
> On 02.03.21 18:51, Peter Xu wrote:
> > On Tue, Feb 09, 2021 at 02:49:38PM +0100, David Hildenbrand wrote:
> > > +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory"
> > > +static bool map_noreserve_effective(int fd, bool shared)
> > > +{
> > > +#if defined(__linux__)
> > > +    gchar *content = NULL;
> > > +    const char *endptr;
> > > +    unsigned int tmp;
> > > +
> > > +    /* hugetlbfs behaves differently */
> > > +    if (qemu_fd_getpagesize(fd) != qemu_real_host_page_size) {
> > > +        return true;
> > > +    }
> > > +
> > > +    /* only private shared mappings are accounted (ignoring /dev/zero) */
> > > +    if (fd != -1 && shared) {
> > > +        return true;
> > > +    }

[1]

> > > +
> > > +    if (g_file_get_contents(OVERCOMMIT_MEMORY_PATH, &content, NULL, NULL) &&
> > > +        !qemu_strtoui(content, &endptr, 0, &tmp) &&
> > > +        (!endptr || *endptr == '\n')) {
> > > +        if (tmp == 2) {
> > > +            error_report("Skipping reservation of swap space is not supported: "
> > > +                         " \"" OVERCOMMIT_MEMORY_PATH "\" is \"2\"");
> > > +            return false;
> > > +        }
> > > +        return true;
> > > +    }
> > > +    /* this interface has been around since Linux 2.6 */
> > > +    error_report("Skipping reservation of swap space is not supported: "
> > > +                 " Could not read: \"" OVERCOMMIT_MEMORY_PATH "\"");
> > > +    return false;
> > > +#else
> > > +    return true;
> > > +#endif
> > > +}
> > 
> > I feel like this helper wants to fail gracefully for some conditions.  Could
> > you elaborate one example and attach to the commit log?
> 
> Sure. The case is "/proc/sys/vm/overcommit_memory == 2" (never overcommit)
> 
> MAP_NORESERVE is without effect and sparse memory regions are somewhat
> impossible.
> 
> > 
> > I'm also wondering whether it would worth to check the global value.  Even if
> > overcommit is globally disabled, do we (as an application process) need to care
> > about it?  I think the MAP_NORESERVE would simply be silently ignored by the
> > kernel and that seems to be design of it, otherwise would all apps who uses > MAP_NORESERVE would need to do similar things too?
> 
> Right, I want to catch the "gets silently ignored" part, because someone
> requested "reserved=off" (!default) but does not actually get what he asked
> for.
> 
> As one example, glibc manages heaps via:
> 
> a) Creating a new heap: mmap(PROT_NONE, MAP_NORESERVE) the maximum size,
> then mprotect(PROT_READ|PROT_WRITE) the initial heap size. Even if
> MAP_NORESERVE is ignored, only !PROT_NONE memory ever gets committed
> ("reserve swap space") in Linux.
> 
> b) Growing the heap via mprotect(PROT_READ|PROT_WRITE) within the existing
> mmap. This will commit memory in case MAP_NORESERVE got ignored.
> 
> c) Shrinking the heap ("discard memory") via MADV_DONTNEED *unless*
> "/proc/sys/vm/overcommit_memory == 2" - the only way to undo
> mprotect(PROT_READ|PROT_WRITE) and to un-commit memory is by doing a
> mmap(PROT_NONE, MAP_FIXED) over the problematic region.
> 
> If you're interested, you can take a look at:
> 
> malloc/arena.c
> sysdeps/unix/sysv/linux/malloc-sysdep.h:check_may_shrink_heap()

Thanks for the context.  It's interesting to know libc has such special heap
operations.

Glibc shrinks heap to save memory for the no-over-commit case, however in our
case currently we'd like to fail some users using global_overcommit=2 but
reserve=off - it means even if we don't fail the user, mmap() could also fail
if it's overcommitted. Even if this mmap() didn't fail, it'll fail very easily
later on iiuc, right?

I think it's fine to have that early failure, it just seems less helpful than
what glibc was doing which shrinks active memory for real, meanwhile there
seems to encode some very detailed OS information into this helper, so just
less charming.

Btw above [1] "fd != -1 && shared" looks weird to me.

Firstly it'll bypass overcommit_memory==2 check and return true directly, is
that right?  I thought the global will be meaningful for all memories except
hugetlbfs (in do_mmap() of Linux).

Meanwhile, I don't see why file-backed share memories is so special too..  From
your commit message, I'm not sure whether you wanted to return false instead,
however that's still not the case IIUC, since e.g. /dev/shmem still does
accounting iiuc, while MAP_NORESERVE will skip it.

-- 
Peter Xu