Dynamic loading: Fill pages with halts only when they are needed

Dynamic loading: Fill pages with halts only when they are needed

This will save memory and improve startup time.

Pages from the dyncode shared memory segment are mapped as
unreadable/non-executable to start with.  When a page is needed, we
fill it with halt instructions and then make it accessible (readable
and executable) by untrusted code.

We maintain a bitmap of pages that have been allocated in this way.
We track allocations at 64k granularity, even though we could use 4k
on Unix, so that the observable behaviour on Linux/Mac is the same as
on Windows.  Also it makes the bitmap a bit smaller (512 bytes for a
256MB dynamic code area).  Although we could use a list of ranges,
using a bitmap is simpler.

Specific changes:

 * nacl_desc_imc_shm.c: Move the check for PROT_NONE into the
   Windows-specific code (win/nacl_shm.cc).  We need PROT_NONE on
   Unix, and it works fine there.

 * sel_ldr_standard.c: Re-order the setup so that NaClAppLoadFile()'s
   NaCl_mprotect() call does not undo the PROT_NONE/PAGE_NOACCESS
   permissions we carefully set up in nacl_text.c.  Similarly, remove
   the NaCl_mprotect() call in sel_addrspace.c that was undoing the
   damage that NaClAppLoadFile() did.

 * Add a test that checks that unallocated dynamic code pages are
   inaccessible.

Note that on Windows, this change only saves RAM, not swap space.
Since we do not yet pass SEC_RESERVE to CreateFileMapping() in
nacl_shm.cc, we are still reserving swap space for the whole dynamic
code area.  This can be fixed but it involves some trickier code on
Windows.

BUG=http://code.google.com/p/nativeclient/issues/detail?id=503
TEST=run_dynamic_load_test

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=5481

[Roland McGrath, 2011-05-25 20:44:31]

Is there a real benefit to maintaining a bitmap?
It could just be a maximum address, and be simpler.

This mprotect's a (64k) page at a time.  On Unix, it can do a single mprotect
call for the whole contiguous region, and there's no reason not to.

This addresses the resource exhaustion problem on Unix iff ftruncate makes a
sparse file.  It used to be (BSD) that ftruncate allocated space (non-sparse),
though it does not in Linux and I'm not sure about current MacOS.  If the file
is sparse, then we are subject to resource exhaustion later when trying to fill
the pages, and we cannot recover well from that.  It would be better to leave
the shm file at zero length initially, and then use posix_fallocate on it before
filling portions later.  posix_fallocate will fail recoverably if there is no
filesystem space.

[Mark Seaborn, 2011-05-25 20:58:15]

On 25 May 2011 21:44, <mcgrathr@chromium.org> wrote:

> Is there a real benefit to maintaining a bitmap?

It could just be a maximum address, and be simpler.
>

There's a benefit because we load the IRT library at a high address,
currently at 0x08000000 (128MB).  If we only maintain a maximum address then
we will waste about 128MB of memory to HLTs instead of the current 256MB.
128MB is still pretty bad.

Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[bsy, 2011-05-25 21:56:35]

i generally like the suggestion to use posix_fallocate to ensure sparse shm
files on linux, though posix_fallocate doesn't appear to be available on
OSX.  it would be bad to have random NaCl modules abort akin to death by OOM
killer when we are running on the edge of available resource, but as far as
i can tell it's probably okay in the "typical" case.  in normal use where
one tab at a time comes up, the nexe in the last tab would just fail to run
in an eager allocation world, but in the lazy allocation world what would
happen?  for dso loading that occurs at startup, the dynamic code memory
pressure increases early, so nexes would also come up or not at all; it
would only be in nexes that want to JIT that would already be running but
fail to get memory for JITted code.  hopefully it is rare and the runtime
could fall back to interpretation when dynamic code memory is not available.

in any case, we need to be explicit about this trade-off....  also, not for
this CL.

-bsy

On Wed, May 25, 2011 at 1:58 PM, Mark Seaborn <mseaborn@chromium.org> wrote:

> On 25 May 2011 21:44, <mcgrathr@chromium.org> wrote:
>
>> Is there a real benefit to maintaining a bitmap?
>
> It could just be a maximum address, and be simpler.
>>
>
> There's a benefit because we load the IRT library at a high address,
> currently at 0x08000000 (128MB).  If we only maintain a maximum address then
> we will waste about 128MB of memory to HLTs instead of the current 256MB.
> 128MB is still pretty bad.
>
> Mark
>
>


-- 
bennet s yee
i usually don't capitalize due to mild tendonitis

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[bsy, 2011-05-25 21:56:48]

small nits.  plz fix.  o/w lgtm.

http://codereview.chromium.org/7068021/diff/1/src/native_client/src/trusted/s...
File src/native_client/src/trusted/service_runtime/nacl_text.c (right):

http://codereview.chromium.org/7068021/diff/1/src/native_client/src/trusted/s...
src/native_client/src/trusted/service_runtime/nacl_text.c:240:
NaClLog(LOG_FATAL, "VirtualProtect() failed\n");
see below

http://codereview.chromium.org/7068021/diff/1/src/native_client/src/trusted/s...
src/native_client/src/trusted/service_runtime/nacl_text.c:249:
NaClLog(LOG_FATAL, "malloc failed\n");
please be more descriptive so that *which* malloc failed is more obvious.  it's
nice to see how far things got before resource exhaustion caused the system to
abort, and if all such messages were just "malloc failed", we'd have no idea
except by hopefully looking at the backtrace.  if we got a core at all.

[Mark Seaborn, 2011-05-25 22:37:56]

On 25 May 2011 21:44, <mcgrathr@chromium.org> wrote:

> Is there a real benefit to maintaining a bitmap?
> It could just be a maximum address, and be simpler.
>
> This mprotect's a (64k) page at a time.  On Unix, it can do a single
> mprotect
> call for the whole contiguous region, and there's no reason not to.
>
> This addresses the resource exhaustion problem on Unix iff ftruncate makes
> a
> sparse file.  It used to be (BSD) that ftruncate allocated space
> (non-sparse),
> though it does not in Linux and I'm not sure about current MacOS.  If the
> file
> is sparse, then we are subject to resource exhaustion later when trying to
> fill
> the pages, and we cannot recover well from that.  It would be better to
> leave
> the shm file at zero length initially, and then use posix_fallocate on it
> before
> filling portions later.  posix_fallocate will fail recoverably if there is
> no
> filesystem space.
>

On Linux:

The idea of using posix_fallocate() seems like a good point.

However, I do wonder if graceful failure is mostly a lost cause on Linux, at
least where overcommit is enabled.  If mmap() does not commit pages, won't
it always succeed?  We can make the dyncode syscalls fail gracefully in
out-of-memory situations -- assuming, I think, that /dev/shm has a limit
that is lower than swap space.  However, all other allocations by the NaCl
process will be subject to non-graceful failure via the OOM killer.

Having said that, I think Chrome OS disables overcommit.

Also, my change does not introduce the problem of hitting OOM via
HLT-filling the dynamic code area.  This would already be present, albeit
confined to sel_ldr's startup phase.  So I suppose triggering OOM from
dyncode_create() while the user process is doing something useful could be
worse than triggering it during startup (since it could lose the user's
work), but I think I can leave introducing posix_fallocate() to a later
change.

On Mac OS X:

The question of whether ftruncate() creates a sparse file on Mac is a good
point.  (Although actually we are using seek()+write() in Chromium -- see
nacl_launcher_thread.cc.)

It turns out that OS X's HFS+ does not support sparse files anyway. :-(
e.g. see http://en.wikipedia.org/wiki/Sparse_file  (I haven't tested whether
shm_open() supports sparse files but it doesn't make much difference since
shm_open() FDs don't allow PROT_EXEC mappings.)

So it looks like this change will not improve performance on Mac OS X.  A
quick test shows that ftruncate()'ing an empty file to 1GB is as slow as
write()ing 1GB of zeroes to it.

Your suggestion of using ftruncate() might then be a good partial solution
for fixing startup performance on Mac, but on its own it will only halve the
time given the current IRT load address and the fact that the IRT is loaded
via dyncode_create().

I'll raise bugs for these two issues.

Cheers,
Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[Mark Seaborn, 2011-05-25 22:58:25]

On 25 May 2011 22:56, <bsy@google.com> wrote:

> small nits.  plz fix.  o/w lgtm.
>
>
>
>
http://codereview.chromium.org/7068021/diff/1/src/native_client/src/trusted/s...
> File src/native_client/src/trusted/service_runtime/nacl_text.c (right):
>
>
>
http://codereview.chromium.org/7068021/diff/1/src/native_client/src/trusted/s...
> src/native_client/src/trusted/service_runtime/nacl_text.c:240:
> NaClLog(LOG_FATAL, "VirtualProtect() failed\n");
> see below
>
>
>
http://codereview.chromium.org/7068021/diff/1/src/native_client/src/trusted/s...
> src/native_client/src/trusted/service_runtime/nacl_text.c:249:
> NaClLog(LOG_FATAL, "malloc failed\n");
> please be more descriptive so that *which* malloc failed is more
> obvious.  it's nice to see how far things got before resource exhaustion
> caused the system to abort, and if all such messages were just "malloc
> failed", we'd have no idea except by hopefully looking at the backtrace.
>  if we got a core at all.
>

Fixed.  Thanks for catching these.

Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[Roland McGrath, 2011-05-25 22:58:37]

Indeed, a non-sparse file is fairly pessimal for resource use.
The posix_fallocate trick only works because it can de-sparse part of a file
discontiguously.

The only way to win on Mac (i.e. with non-sparse files) is not to use a 1:1
mapping of the backing file to the address space.  That would require a lot more
bookkeeping, to allocate pages from the backing file linearly from the beginning
to be mapped with subsequent mmap calls to whatever portion of the dynamic text
region is actually filled.