Skip to content

Commit

Permalink
Discontiguous PageProtect GC support (#946)
Browse files Browse the repository at this point in the history
This CL fixes PageProtect's memory unprotect calls. When using
discontiguous space,
* If the allocated pages are not in a new chunk, unprotect the pages as
before.
* If the pages are in a new chunk, there can be two cases:
   * If the new chunk is not mapped, no need to unprotect
* If the new chunk is mapped, this means that the chunk is allocated and
released on previous GC epochs before. The GC should unprotect these
pages.
  • Loading branch information
wenyuzhao authored Sep 7, 2023
1 parent e8b826f commit 62f1dc9
Showing 1 changed file with 22 additions and 13 deletions.
35 changes: 22 additions & 13 deletions src/util/heap/freelistpageresource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,19 +147,28 @@ impl<VM: VMBinding> PageResource<VM> for FreeListPageResource<VM> {
let rtn = self.start + conversions::pages_to_bytes(page_offset as _);
// The meta-data portion of reserved Pages was committed above.
self.commit_pages(reserved_pages, required_pages, tls);
if self.protect_memory_on_release && !new_chunk {
// This check is necessary to prevent us from mprotecting an address that is not yet mapped by mmapper.
// See https://github.com/mmtk/mmtk-core/issues/400.
// It is possible that one thread gets a new chunk, and returns from this function. However, the Space.acquire()
// has not yet call ensure_mapped() for it. So the chunk is not yet mmapped. At this point, if another thread calls
// this function, and get a few more pages from the same chunk, it is no longer seen as 'new_chunk', and we
// will try to munprotect on it. But the chunk may not yet be mapped.
//
// If we want to improve and get rid of this loop, we need to move this munprotect to anywhere after the ensure_mapped() call
// in Space.acquire(). We can either move it the option of 'protect_on_release' to space, or have a call to page resource
// after ensure_mapped(). However, I think this is sufficient given that this option is only used for PageProtect for debugging use.
while !MMAPPER.is_mapped_address(rtn) {}
self.munprotect(rtn, self.free_list.size(page_offset as _) as _)
if self.protect_memory_on_release {
if !new_chunk {
// This check is necessary to prevent us from mprotecting an address that is not yet mapped by mmapper.
// See https://github.com/mmtk/mmtk-core/issues/400.
// It is possible that one thread gets a new chunk, and returns from this function. However, the Space.acquire()
// has not yet call ensure_mapped() for it. So the chunk is not yet mmapped. At this point, if another thread calls
// this function, and get a few more pages from the same chunk, it is no longer seen as 'new_chunk', and we
// will try to munprotect on it. But the chunk may not yet be mapped.
//
// If we want to improve and get rid of this loop, we need to move this munprotect to anywhere after the ensure_mapped() call
// in Space.acquire(). We can either move it the option of 'protect_on_release' to space, or have a call to page resource
// after ensure_mapped(). However, I think this is sufficient given that this option is only used for PageProtect for debugging use.
while !new_chunk && !MMAPPER.is_mapped_address(rtn) {}
self.munprotect(rtn, self.free_list.size(page_offset as _) as _)
} else if !self.common().contiguous && new_chunk {
// Don't unprotect if this is a new unmapped discontiguous chunk
// For a new mapped discontiguous chunk, this should previously be released and protected by us.
// We still need to unprotect it.
if MMAPPER.is_mapped_address(rtn) {
self.munprotect(rtn, self.free_list.size(page_offset as _) as _)
}
}
};
Result::Ok(PRAllocResult {
start: rtn,
Expand Down

0 comments on commit 62f1dc9

Please sign in to comment.