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: Fri, 18 Sep 2015 19:22:46 -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>
On Fri, Sep 18, 2015 at 09:11:01PM +0100, Balazs Kezes wrote:
> On 2015-09-18 15:45 -0400, Rich Felker wrote:
> > On Fri, Sep 18, 2015 at 08:29:52PM +0100, Balazs Kezes wrote:
> > > So here's what I think pthreads should do: First mmap with PROT_NONE
> > > and only then should mprotect read/write the stack pages.
> > >
> > > Does that sound reasonable?
> >
> > Yes.
>
> So here's the simplest patch I could come up with:
>
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index 753da61..c6065dc 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -501,12 +501,21 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
> size += pagesize_m1 + 1;
> #endif
>
> - mem = mmap (NULL, size, prot,
> + /* Map with PROT_NONE first and only then mprotect the pages to avoid
> + the kernel unnecessary reserving the pages in the case of
> + mlockall(MCL_FUTURE). */
> + mem = mmap (NULL, size, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
>
> if (__glibc_unlikely (mem == MAP_FAILED))
> return errno;
>
> + if (__glibc_unlikely (mprotect (mem, size, prot) != 0))
> + {
> + munmap(mem, size);
> + return errno;
> + }
> +
> /* SIZE is guaranteed to be greater than zero.
> So we can never get a null pointer back from mmap. */
> assert (mem != NULL);
>
>
> I've verified in my pthreads example that pthreads doesn't waste memory
> with this patch applied. That's not really a nice fix though but this
> allocatestack() function looks too scary to me. :(
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.
Rich