New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
erts: Decouple use of single-mapped memory from perf
support
#6340
erts: Decouple use of single-mapped memory from perf
support
#6340
Conversation
CT Test Results 3 files 132 suites 44m 35s ⏱️ For more details on these failures, see this check. Results for commit 8da7a93. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
@frej The current QEMU bug discussion indicated it wasn't fixable at that level (i.e., it is related to the Linux kernel instead of QEMU). So, the |
What is the downside / tradeoff of enabling the flag? It could be useful to mention in the doc. This looks really useful! |
@okeuday My take away from the bug discussion was that the current translation strategy in QEMU for user mode emulation doesn't consider the case when a physical page is mapped into more than one place in the emulated process' address space. When the JIT updates a page using the RW-mapping, QEMU fails to update the executable mapping and when we then branch into the new code, we crash. As the memory is (at least on Linux) allocated by acquiring an fd to an anonymous file (or POSIX SHM) and then
That depends, if the virtualization implementation does dynamic translation of the JIT:ed code (and doesn't consider multiple maps), it will be needed. If it does not and the virtualization implementation doesn't screw up multiple |
The downside of enabling the flag is that you are using RWX pages, but the pro is that BEAM runs without segfaulting. The case for not using RWX pages is too complicated to cover in the OTP documentation, look to what SpiderMonkey says about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but I suggest you add a test case with a smoke test of the new option (that is, that the emulator can be started when the option is given).
@@ -149,7 +155,6 @@ static JitAllocator *pick_allocator() { | |||
"memory. Either allow this or disable the " | |||
"'+JPperf' option."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the +JMsingle
option be mentioned as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, will fix.
@bjorng I will add a smoke test, but probably not until the weekend as this is fallout from a hobby project. |
6affacd
to
18b5c6f
Compare
The review suggestions are now incorporated. |
Thanks! Added to our daily builds. |
The test case fails on an Apple Silicon Mac because the emulator crashes with Apparently, on Apple Macs it is not allowed to have memory that is both writable and executable, but that is not detected when initializing the allocator. The crash occurs later when attempting to write into the allocated memory that does not appear to be writable. I have pushed a suggestion for a fix as a fixup commit. |
erts/emulator/test/jit_SUITE.erl
Outdated
@@ -180,6 +180,48 @@ annotate(Config) -> | |||
[Symbol, Anno]) | |||
end. | |||
|
|||
run_jmsingle_test(Param, ExpectSuccess, ErrorMsg) -> | |||
Cmd = "erl +JMsingle " ++ Param ++ " -noshell " ++ | |||
"-eval 'io:format(\"All is well~n\"),erlang:halt(0).'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will most likely not work on Windows. As far as I know, single quotes are not supported on Windows. The following is more likely to work:
"-eval 'io:format(\"All is well~n\"),erlang:halt(0).'", | |
"-eval erlang:display(all_is_well),erlang:halt(0).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have pushed a fixup for this, I will wait for a clarification from @garazdawi before dealing with the netbsd issue.
erts/emulator/test/jit_SUITE.erl
Outdated
%% +JMsingle true does not work on macOS running | ||
%% on Apple Silicon computers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os:type() == {unix,netbsd}
has the same issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the allocator error out at creation time or do we have to check for the os in JitAllocator::create_allocator()
just like on the mac?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It errors out at creation time. So the only thing we need to fix is the testcase.
@garazdawi , @bjorng: I have pushed a fixup for the NetBSD issue, should I wait for results from more platforms, or is it time to squash the fixups (if the CI passes)? |
You can squash the commits now. Tests have been run successfully on all platforms. |
Running the JIT with single-mapped RWX memory for JIT:ed machine code is needed for `perf` support. But as it is also useful for running under QEMU user mode emulation [1] this patch adds a new flag `+JMsingle bool` which controls the use of single-mapped RWX memory without triggering the output of perf-related metadata. The naming of the flag is intended to follow the (perceived) pattern introduced by the `+JPperf`-flag, that is, `J` is a JIT-related option and `M` stands for memory related things. Thanks to Björn Gustavsson <bjorn@erlang.org> for suggesting how to deal with Apple-silicon aarch64 and how to make the smoke test work on Windows. Personally I use the additional patch below in order to use the same file system image for development on an amd64 build host and for deployment to an aarch64-target board. The patch detects when the VM is running using QEMU and if so, enables single-mapped memory. As the patch only works on Linux and doesn't try to identify the version of QEMU (if the bug gets fixed), I don't think it belongs on OTP master. But as it is nevertheless useful, I include it here: ``` diff --git a/erts/emulator/beam/erl_init.c b/erts/emulator/beam/erl_init.c index 42216da0b2..6f11c7586b 100644 --- a/erts/emulator/beam/erl_init.c +++ b/erts/emulator/beam/erl_init.c @@ -2408,6 +2408,40 @@ erl_start(int argc, char **argv) erts_usage(); } + +#if defined(HAVE_LINUX_PERF_SUPPORT) + /* We want to detect if we are running under QEMU user mode + emulation in order to activate single-mapped RWX memory for the + JIT:ed code. This only works on Linux, so we use + HAVE_LINUX_PERF_SUPPORT to conditionally compile this piece of + detection code. + + To detect QEMU user mode we make use of the fact that our + parent process will always be QEMU, we detect that by looking + up our parent process and then checking if the final path + component of the symlink /proc/<parent>/exe starts with + "qemu-". + */ + { + char symlink_buf[21 + 11]; /* space for any 64 bit integer + + /proc/%d/exe and terminator */ + char target_buf[MAXPATHLEN]; + ssize_t l; + erts_snprintf(symlink_buf, sizeof(symlink_buf), + "/proc/%d/exe", getppid()); + l = readlink(symlink_buf, target_buf, sizeof(target_buf)); + if (l > 0 && l != sizeof(target_buf)) { + char *last_path_separator; + + target_buf[l] = 0; + last_path_separator = memrchr(target_buf, '/', l); + if (last_path_separator && strncmp(last_path_separator + 1, + "qemu-", 5) == 0) + erts_jit_single_map = 1; + } + } +#endif + /* Output format on windows for sprintf defaults to three exponents. * We use two-exponent to mimic normal sprintf behaviour. */ ``` [1] There is a bug in QEMU, see https://gitlab.com/qemu-project/qemu/-/issues/1034
e7e5c44
to
8da7a93
Compare
Thanks! |
Running the JIT with single-mapped RWX memory for JIT:ed machine code is needed for
perf
support. But as it is also useful for running under QEMU user mode emulation [1] this patch adds a new flag+JMsingle bool
which controls the use of single-mapped RWX memory without triggering the output of perf-related metadata.The naming of the flag is intended to follow the (perceived) pattern introduced by the
+JPperf
-flag, that is,J
is a JIT-related option andM
stands for memory related things.Personally I use the additional patch below in order to use the same file system image for development on an amd64 build host and for deployment to an aarch64-target board. The patch detects when the VM is running using QEMU and if so, enables single-mapped memory. As the patch only works on Linux and doesn't try to identify the version of QEMU (if the bug gets fixed), I don't think it belongs to OTP master. But as it is nevertheless useful, I include it here:
[1] There is a bug in QEMU,
see https://gitlab.com/qemu-project/qemu/-/issues/1034