Linux: Converts passthrough syscalls to generator script #3355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sonicadvance1
force-pushed
the
cleanup_passthrough_syscalls
branch
7 times, most recently
from
e4c2abb to
5ae9905
Compare
After adding the past few Linux versions of syscalls I noticed that this could be better. Walks through all of our syscalls and converts them to a json format that is parsed and then generated at compile time. This also fixes some mislabeling of system calls that were getting inlined on accident. This really helps clean up the syscall implementation, all syscalls that can get passed through to the host syscall handler is no longer visible in the implementation.
I guess this was handled by brk things before.
Sonicadvance1
force-pushed
the
cleanup_passthrough_syscalls
branch
from
5ae9905 to
e1c6cb2
Compare
|
I like the prospect of cleaning up the syscalls handling and making it less prone to small bugs. Not sure I'm a fan of the implementation chosen here though. You've added a layer of code generation (Python) on top of another (C macros), which in turn uses a third code-generating mechanism (C++ templates). The syscalls implementation cleanup comes at the cost of increased architectural complexity. Generally I'm not a fan of involving more Python scripts into our build process, since we're already at a point where minor changes trigger almost full rebuilds since everything depends on everything. This has tangible effects on the developer workflow, since I often find myself waiting for lengthy recompiles after rebasing even though I didn't touch any FEXCore code. Since the state captured in SyscallDescription.json is super simple, have you considered any C++-based alternatives? The code we use today seems straightforward enough to turn into a single template with simple (As a side note, it looks like e1c6cb2 accidentally slipped into this PR.) |
|
Thank you for reviewing the code. |
Sonicadvance1
added a commit
to Sonicadvance1/FEX
that referenced
this pull request
Feb 24, 2024
Reimagining of FEX-Emu#3355 without any json generators or new concepts. Fixes some mislabeling of system calls. Some getting inlined when they shouldn't be, a lot not getting inlined when they can be. This really cleans up the syscall implementation, all syscalls that can be passthrough implementations require a very small two line declaration. Additionally cleans up a bit of implementation cruft where some passthrough syscalls were using the glibc syscall handler, and some were using the glibc implementation. We have had multiple issues in the past where the glibc implementation does something subtly different than the raw syscall and breaks things. Now all passthrough handlers do a system call directly, removing at least one indirection and some ambiguity. This makes it significantly easier to add new passthrough syscalls as well. Only need to do a version check and add the three lines per syscall. Which there are new syscalls incoming that we will want to add. Tangible improvements: - Syscalls are lower overhead than ever. - When I'm adding more syscalls I have less chance of mucking it up.
Sonicadvance1
added a commit
to Sonicadvance1/FEX
that referenced
this pull request
Feb 24, 2024
Reimagining of FEX-Emu#3355 without any json generators or new concepts. Fixes some mislabeling of system calls. Some getting inlined when they shouldn't be, a lot not getting inlined when they can be. This really cleans up the syscall implementation, all syscalls that can be passthrough implementations require a very small two line declaration. Additionally cleans up a bit of implementation cruft where some passthrough syscalls were using the glibc syscall handler, and some were using the glibc implementation. We have had multiple issues in the past where the glibc implementation does something subtly different than the raw syscall and breaks things. Now all passthrough handlers do a system call directly, removing at least one indirection and some ambiguity. This makes it significantly easier to add new passthrough syscalls as well. Only need to do a version check and add the three lines per syscall. Which there are new syscalls incoming that we will want to add. Tangible improvements: - Syscalls are lower overhead than ever. - When I'm adding more syscalls I have less chance of mucking it up.
Sonicadvance1
added a commit
to Sonicadvance1/FEX
that referenced
this pull request
Feb 24, 2024
Reimagining of FEX-Emu#3355 without any json generators or new concepts. Fixes some mislabeling of system calls. Some getting inlined when they shouldn't be, a lot not getting inlined when they can be. This really cleans up the syscall implementation, all syscalls that can be passthrough implementations require a very small two line declaration. Additionally cleans up a bit of implementation cruft where some passthrough syscalls were using the glibc syscall handler, and some were using the glibc implementation. We have had multiple issues in the past where the glibc implementation does something subtly different than the raw syscall and breaks things. Now all passthrough handlers do a system call directly, removing at least one indirection and some ambiguity. This makes it significantly easier to add new passthrough syscalls as well. Only need to do a version check and add the three lines per syscall. Which there are new syscalls incoming that we will want to add. Tangible improvements: - Syscalls are lower overhead than ever. - When I'm adding more syscalls I have less chance of mucking it up.
Sonicadvance1
added a commit
to Sonicadvance1/FEX
that referenced
this pull request
Feb 25, 2024
Reimagining of FEX-Emu#3355 without any json generators or new concepts. Fixes some mislabeling of system calls. Some getting inlined when they shouldn't be, a lot not getting inlined when they can be. This really cleans up the syscall implementation, all syscalls that can be passthrough implementations require a very small two line declaration. Additionally cleans up a bit of implementation cruft where some passthrough syscalls were using the glibc syscall handler, and some were using the glibc implementation. We have had multiple issues in the past where the glibc implementation does something subtly different than the raw syscall and breaks things. Now all passthrough handlers do a system call directly, removing at least one indirection and some ambiguity. This makes it significantly easier to add new passthrough syscalls as well. Only need to do a version check and add the three lines per syscall. Which there are new syscalls incoming that we will want to add. Tangible improvements: - Syscalls are lower overhead than ever. - When I'm adding more syscalls I have less chance of mucking it up.
Sonicadvance1
added a commit
to Sonicadvance1/FEX
that referenced
this pull request
Feb 27, 2024
Reimagining of FEX-Emu#3355 without any json generators or new concepts. Fixes some mislabeling of system calls. Some getting inlined when they shouldn't be, a lot not getting inlined when they can be. This really cleans up the syscall implementation, all syscalls that can be passthrough implementations require a very small two line declaration. Additionally cleans up a bit of implementation cruft where some passthrough syscalls were using the glibc syscall handler, and some were using the glibc implementation. We have had multiple issues in the past where the glibc implementation does something subtly different than the raw syscall and breaks things. Now all passthrough handlers do a system call directly, removing at least one indirection and some ambiguity. This makes it significantly easier to add new passthrough syscalls as well. Only need to do a version check and add the three lines per syscall. Which there are new syscalls incoming that we will want to add. Tangible improvements: - Syscalls are lower overhead than ever. - When I'm adding more syscalls I have less chance of mucking it up.
bylaws
pushed a commit
to bylaws/FEX
that referenced
this pull request
Feb 29, 2024
Reimagining of FEX-Emu#3355 without any json generators or new concepts. Fixes some mislabeling of system calls. Some getting inlined when they shouldn't be, a lot not getting inlined when they can be. This really cleans up the syscall implementation, all syscalls that can be passthrough implementations require a very small two line declaration. Additionally cleans up a bit of implementation cruft where some passthrough syscalls were using the glibc syscall handler, and some were using the glibc implementation. We have had multiple issues in the past where the glibc implementation does something subtly different than the raw syscall and breaks things. Now all passthrough handlers do a system call directly, removing at least one indirection and some ambiguity. This makes it significantly easier to add new passthrough syscalls as well. Only need to do a version check and add the three lines per syscall. Which there are new syscalls incoming that we will want to add. Tangible improvements: - Syscalls are lower overhead than ever. - When I'm adding more syscalls I have less chance of mucking it up.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needs #3349 and #3354 merged first.After adding the past few Linux versions of syscalls I noticed that this
could be better.
Walks through all of our syscalls and converts them to a json format
that is parsed and then generated at compile time.
This also fixes some mislabeling of system calls that were getting
inlined on accident.
This really helps clean up the syscall implementation, all syscalls that
can get passed through to the host syscall handler is no longer visible
in the implementation.