Skip to content

Commit

Permalink
Trac #22633: Pari default stack size results in huge physical memory …
Browse files Browse the repository at this point in the history
…commit after fork on Cygwin

It turns out #21582 causes big problems for fork on Cygwin.  This is due
to an implementation detail of how Cygwin handles copy-on-write private
mappings when forking.  A fact that might, unfortunately, be difficult
to avoid.  The problem with this on Cygwin is that, while Windows does
not commit pages to physical memory until they are accessed, they ''do''
become committed upon any access (even reads, when they haven't already
been written to).

So when a process with a private mmap is forked, it loops over all pages
in the mmap'd region and copies them into the child's copy of the mmap'd
region.  This results in committing physical memory on both the parent
and the child, even for pages that haven't been written to yet.

TL;DR, if there's a huge private|anonymous mmap in a process, and that
process is forked, it will commit the full size of the mmap to memory in
both the parent and child processes.

This is a big problem in Sage since we set a very large default stack
size for Pari.  This was not a problem prior to #21582, since Pari used
the MAP_NORESERVE flag which circumvents this issue (only dirty pages
need to be copied).  The changes in #21582 make sense for Linux, but for
Cygwin the opposite is true.  Different but unfortunate implementation
details on both platforms.

The best way forward is to use `MAP_NORESERVE` anyway, which helps
Cygwin and won't hurt other platforms.

'''Upstream bug''': https://pari.math.u-bordeaux.fr/cgi-
bin/bugreport.cgi?bug=1912

URL: https://trac.sagemath.org/22633
Reported by: embray
Ticket author(s): Erik Bray, Jeroen Demeyer
Reviewer(s): Erik Bray, Jeroen Demeyer
  • Loading branch information
Release Manager authored and vbraun committed Apr 7, 2017
2 parents 706f802 + 343f241 commit fc8ed14
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
2 changes: 1 addition & 1 deletion build/pkgs/pari/package-version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.9.1.p2
2.9.1.p3
53 changes: 53 additions & 0 deletions build/pkgs/pari/patches/prot_none_4.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
commit c3dc1546580eda3bff6243cf563801c8a26ec67f
Author: Jeroen Demeyer <jdemeyer@cage.ugent.be>
Date: Mon Apr 3 16:11:54 2017 +0200

mmap the PARI stack with MAP_NORESERVE

diff --git a/src/language/init.c b/src/language/init.c
index 34cce31..acebe2f 100644
--- a/src/language/init.c
+++ b/src/language/init.c
@@ -597,12 +597,26 @@ pari_add_defaults_module(entree *ep)
#ifndef MAP_ANONYMOUS
#define MAP_ANONYMOUS MAP_ANON
#endif
+#ifndef MAP_NORESERVE
+#define MAP_NORESERVE 0
+#endif
static void *
pari_mainstack_malloc(size_t size)
{
+ /* Check that the system allows reserving "size" bytes. This is just
+ * a check, we immediately free the memory. */
void *b = mmap(NULL, size, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
- return (b == MAP_FAILED) ? NULL: b;
+ if (b == MAP_FAILED) return NULL;
+ munmap(b, size);
+
+ /* Map again, this time with MAP_NORESERVE. On some operating systems
+ * like Cygwin, this is needed because remapping with PROT_NONE and
+ * MAP_NORESERVE does not work as expected. */
+ b = mmap(NULL, size, PROT_READ|PROT_WRITE,
+ MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0);
+ if (b == MAP_FAILED) return NULL;
+ return b;
}

static void
@@ -628,7 +642,13 @@ static void
pari_mainstack_mreset(pari_sp from, pari_sp to)
{
size_t s = to - from;
- mmap((void*)from, s, PROT_NONE, MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+ void *addr, *res;
+ if (!s) return;
+
+ addr = (void*)from;
+ res = mmap(addr, s, PROT_NONE,
+ MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0);
+ if (res != addr) pari_err(e_MEM);
}

/* Commit (make available) the virtual memory mapped between the

0 comments on commit fc8ed14

Please sign in to comment.