Skip to content
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

patches: Prefault page tables used in stacks allocated for threads #73

Closed
wants to merge 1 commit into from

Conversation

eduardvintila
Copy link
Member

@eduardvintila eduardvintila commented Nov 12, 2023

We need to explicitly populate the page tables allocated for stacks (via mmap's MAP_POPULATE flag) in order to avoid infinite page fault loops during runtime. This measure is necessary since Unikraft on AArch64 does not switch stacks when handling CPU exceptions. A page fault occuring on the stack would cause the trap handler to save all the registers on the same stack, thus generating another page fault, ad infinitum.

This PR is part of a larger series which provides support for Golang on ARM64 platforms:

We need to explicitly populate the page tables allocated for stacks
(via mmap's `MAP_POPULATE` flag) in order to avoid infinite page
fault loops during runtime. This measure is necessary since Unikraft
on AArch64 does not switch stacks when handling CPU exceptions.
A page fault occuring on the stack would cause the trap handler to
save all the registers on the same stack, thus generating another
page fault, ad infinitum.

Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Copy link
Member

@mogasergiu mogasergiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maaan, I gotta say I really like the commit message: "ad infinitum" 🧠 ! Very descriptive and nicely written. I really appreciate it when people put in effort in their commits 💪.

Thanks for your work! I agree this fix will work, however at a great cost! Stacks (and especially userspace stacks) can be huge (MB in size). This means that pre-populating them early on has a big impact on boot 👢 times. I am wondering 🤔, would it be possible that #1174 could solve this issue without needing to prefault the stacks (btw I have not tested it individually yet so i'd advise to use #1175 instead)?

@eduardvintila
Copy link
Member Author

@mogasergiu, thank you so much for the nice comment! :)

It's true, I completely agree it has a big impact on boot times - it was more of a quick fix to temporarily get the ARM64 Golang support rolling until we find a proper solution. But now your PR does a wonderful job to handle the problem described in this PR, I've just tested it and all works very smoothly 😎

Thanks a lot for the great work! Now, I hereby officially declare this PR obsolete! 🧑‍⚖️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants