On 2011/07/15 12:43:22, William Hesse wrote:
http://codereview.chromium.org/7377008/diff/1/src/platform-linux.cc
File src/platform-linux.cc (left):
http://codereview.chromium.org/7377008/diff/1/src/platform-linux.cc#oldcode374
src/platform-linux.cc:374: if (mbase == MAP_FAILED) {
I think you need to add the MAP_VARIABLE flag to the flags, once you start
supplying a non-null address to mmap. Otherwise, the function can fail
due to
conflict and return MAP_FAILED, even when memory is available. It may be
that
MAP_VARIABLE is actually a 0 flag, which is why it is working currently.
Linux doesn't have MAP_VARIABLE; the contract is the presence vs. absence of
MAP_FIXED.
(Quick test to demo safety):
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7f16b46be000
mmap(0x7f16b46be000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
-1,
0) = 0x7f16b46bd000
http://codereview.chromium.org/7377008/diff/1/src/platform-linux.cc
File src/platform-linux.cc (right):
http://codereview.chromium.org/7377008/diff/1/src/platform-linux.cc#newcode89
src/platform-linux.cc:89: uint64_t raw_addr =
(static_cast<uint64_t>(rnd1) <<
33) |
All of the masking can be replaced by
raw_addr = (static_cast<uint64_t>(rnd1) << 14) ^
(static_cast<uint64_t>(rnd2) << 12)
Thanks all for the comments on this part. I've replaced it with something
much
simpler.
This has 0s in the upper 18 and lower 12 bits.
You should not combine random numbers with OR unless they
are non-overlapping bits. A random bit OR another random bit is 1 with
probability 3/4. Use XOR instead.
http://codereview.chromium.org/7377008/diff/1/src/platform-linux.cc#newcode92
src/platform-linux.cc:92: // permit the use of a 64-bit constant).
There are macros in globals.h to enter 64-bit constants into code (on both
32-bit and 64-bit platforms).
http://codereview.chromium.org/7377008/diff/1/src/platform-linux.cc#newcode108
src/platform-linux.cc:108: // call this setup code within the same
millisecond.
Use XOR, not OR, to combine the overlapping seed sources.
Oh, I'm embarrassed about that one. Thanks.
I'll upload a patch will all review comments from everyone actioned,
shortly.
http://codereview.chromium.org/7377008/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
