This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: pthread wastes memory with mlockall(MCL_FUTURE)
- From: Rich Felker <dalias at libc dot org>
- To: Balazs Kezes <rlblaster at gmail dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Sun, 20 Sep 2015 19:32:21 -0400
- Subject: Re: pthread wastes memory with mlockall(MCL_FUTURE)
- Authentication-results: sourceware.org; auth=none
- References: <20150918102734 dot GA27881 at eper> <20150918143824 dot GB17773 at brightrain dot aerifal dot cx> <20150918163842 dot GB27881 at eper> <20150918170853 dot GC17773 at brightrain dot aerifal dot cx> <20150918192952 dot GC27881 at eper> <20150918194521 dot GD17773 at brightrain dot aerifal dot cx> <20150918201101 dot GD27881 at eper> <20150918232246 dot GF17773 at brightrain dot aerifal dot cx> <20150920132712 dot GA7569 at eper>
On Sun, Sep 20, 2015 at 02:27:12PM +0100, Balazs Kezes wrote:
> On 2015-09-18 19:22 -0400, Rich Felker wrote:
> > If this works, I think it's only due to a kernel bug of failing to
> > apply the lock after mprotect. It's also going to be considerably
> > slower, I think. What I had in mind was switching around the existing
> > mmap/mprotect order, not adding an extra mprotect.
>
> Here's a better patch. It will readd the permissions in two steps. First
> it will only add for this pthread structure at the end of the
> allocation. Then at a later step it will readd it for the rest of the
> stack (this part I didn't touch so that's why this patch is short).
>
> I don't really like this two-step mprotect call but unless the stack
> grows down, you can't avoid it (and I didn't special case that option).
>
> But review carefully because I don't know how to test it better than
> with my little test application and make xcheck.
>
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index 753da61..2959816 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -499,11 +499,11 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
> #if MULTI_PAGE_ALIASING != 0
> if ((size % MULTI_PAGE_ALIASING) == 0)
> size += pagesize_m1 + 1;
> #endif
>
> - mem = mmap (NULL, size, prot,
> + mem = mmap (NULL, size, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
>
> if (__glibc_unlikely (mem == MAP_FAILED))
> return errno;
>
> @@ -542,13 +542,25 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
> - __static_tls_size)
> & ~__static_tls_align_m1)
> - TLS_PRE_TCB_SIZE);
> #endif
>
> + /* We allocated the memory without permissions in order for the kernel
> + to not allocate the guard pages in case of mlockall(MCL_FUTURE).
> + Now readd the permissions for pd's page. */
This comment misses the real point: avoiding committing memory.
MCL_FUTURE is just a particular side effect of this.
> + void *pdpage = (void*) ((uintptr_t) pd & ~pagesize_m1);
> + size_t pdsize = mem + size - pdpage;
> + if (__glibc_unlikely (mprotect (pdpage, pdsize, prot) != 0))
> + {
> + (void) munmap (mem, size);
> + return errno;
> + }
> +
You're still adding a new mprotect rather than just inverting the one
that's already there (somewhere else) adding the PROT_NONE for the
guard page. Is there a reason this is hard to do right on glibc?
Rich