move mmap into sysdeps

[froydnj]

The patch is relatively easy; what's not clear to me is how something like this really wants to be exported. Should sysdeps export a MappedRegion that would be backed by VirtualAlloc on Windows? Should sysdeps just export everything with their platform names and let lucet-runtime use conditionals to choose what to export?

This patch punts on those questions for now; we're just trying to move the obvious system-dependent code into sysdeps.

[fst-crenshaw]

Hello! Thanks for this work!

I know that you requested that @acfoltzer review this PR. He asked me to review the PR so that you could get a timely response. My intention is to continue the conversation you've started around this valuable reorganization of system-dependent Region implementations.

At a high-level, I appreciate the sensibility of putting system-dependent code into a directory called sysdeps. I saw that this was done recently for host_page_size and that was a very worthwhile refactor.

System-dependent Region code requires a less-straightforward refactor. As you can see from the merge conflicts on this PR, other folks have also been working on Regions in the lucet-runtime portion of this repo. Most notably, a recent PR introduced another system-dependent implementation of Regions, this time using the Linux Kernel’s userfaultfd mechanism. See it here: https://github.com/bytecodealliance/lucet/pull/492

What I like about the timing of your PR alongside the above-mentioned PR is that the repo now has a concrete example of what it looks like when there are side-by-side system-dependent implementations of Region in the repo. Here are some highlights:

First, there is some work to conditionally include the uffd version: https://github.com/bytecodealliance/lucet/blob/master/lucet-runtime/lucet-runtime-internals/src/region.rs#L3

Second, there are some fiddly things that take place to run the tests — once for mmap and once for userfaultfd: https://github.com/bytecodealliance/lucet/blob/master/lucet-runtime/tests/entrypoint.rs

Finally, the repo has taken on a new CI (CircleCI) so that it has Linux containers that are privileged enough to use the userfaultfd mechanism.

Certainly, this concrete example isn't ideal. But it is something to look at and may spark insights.

*

You asked really good questions. To paraphrase, “How does something like this really want to be exported?”

Here’s one way that’s done now in a Fastly repo:

use lucet_runtime::{MmapRegion as LucetRegion, Region}; I don’t offer this as a good example of how something like this really wants to be exported, but rather as a concrete example of what is done today.

Now that the userfaultfd work has been folded into the repo, now that we have a concrete example of side-by-side Region implementations, I'd love to hear any reactions you have about eventually adding a Windows-dependent version into the mix.

[pchickey]

This PR was closed as a byproduct of deleting the branch named master. If this is still an active PR, re-open as a new PR against main.