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: Balazs Kezes <rlblaster at gmail dot com>
- To: Rich Felker <dalias at libc dot org>
- Cc: libc-alpha at sourceware dot org
- Date: Fri, 18 Sep 2015 21:11:01 +0100
- 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>
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. :(
--
Balazs