Skip to content

Mmap refactoring #296

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

Merged
merged 14 commits into from
May 3, 2021
Merged

Mmap refactoring #296

merged 14 commits into from
May 3, 2021

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Apr 22, 2021

This PR refactors our mmap implementation and uses. It addresses #244, and it works towards #257 (though more work is needed for #257).

This PR:

  • Replaces most of our uses of dzmmap() with dzmmap_noreplace().
  • Changes mmap flags in mmap_noreserve() from read/write to PROT_NONE so it serves the purpose of reserving address range. The memory is not usable before another mmap or mprotect.
  • Marks dzmmap() unsafe, as it may overwrite existing mappings. The only place it is now used is in side metadata where we reserve address range with mmap_noreserve() and then use dzmmap() to map it.
  • Renames check_is_mapped() to panic_if_unmapped(). The function is more than just checking mapping - it may actually mmap memory as a side effect. We should simply panic if we mmap any memory in the function.
  • Refactors util::memory so they deal with errors in the same way.
  • Adds unit tests for util::memory.
  • Refactors unit tests that uses mmap so 1. they run in serial, 2. they use a memory range that is not in our heap, and 3. they will properly clean up the mapping they create.

@qinsoon qinsoon added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label Apr 22, 2021
@qinsoon qinsoon marked this pull request as ready for review April 23, 2021 06:05
@qinsoon qinsoon requested a review from javadamiri April 23, 2021 06:05
Copy link
Contributor

@javadamiri javadamiri left a comment

Choose a reason for hiding this comment

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

LGTM

@qinsoon qinsoon merged commit 4e8d32c into master May 3, 2021
@qinsoon qinsoon deleted the mmap-reserve branch May 3, 2021 23:06
@qinsoon qinsoon mentioned this pull request May 5, 2021
k-sareen pushed a commit to k-sareen/mmtk-core that referenced this pull request Jun 10, 2021
* Add unit tests for mmap and some refactoring
* Replace dzmmap with dzmmap_noreplace in almost all use cases except the
onces in side metadata
* Make dzmmap unsafe. Add some comments.
* mmap_noreserve maps memory with PROT_NONE. We have to do dzmmap before
actually use it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants