Skip to content

MemoryManager: coalesce initial adjacent compatible regions#2699

Merged
sporksmith merged 2 commits intoshadow:mainfrom
sporksmith:mm-multiple-heaps
Jan 26, 2023
Merged

MemoryManager: coalesce initial adjacent compatible regions#2699
sporksmith merged 2 commits intoshadow:mainfrom
sporksmith:mm-multiple-heaps

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

This is to work around an odd behavior in Linux kernel 6.1.6 on Fedora 37 - it sometimes reports two heap regions with identical properties.

Rather than further-complicating the heap mapping code, it's probably better to just normalize the initial region map to collapse the heaps into a single region.

Verified that this fixes the tor-minimal test in a Fedora 37 VM.

Fixes #2692

@sporksmith sporksmith self-assigned this Jan 25, 2023
@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Jan 25, 2023
This is to work around an odd behavior in Linux kernel 6.1.6 on Fedora
37 - it sometimes reports two heap regions with identical properties.

Rather than further-complicating the heap mapping code, it's probably
better to just normalize the initial region map to collapse the heaps
into a single region.
We get confusing behavior later if there are multiple heaps; better to
bail out earlier. e.g. when debugging
shadow#2686 it would have been
clearer what was happening if we failed here and didn't try to keep
running.
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 26, 2023

Codecov Report

Base: 68.06% // Head: 67.66% // Decreases project coverage by -0.41% ⚠️

Coverage data is based on head (e2c6b5b) compared to base (10c97fc).
Patch coverage: 72.72% of modified lines in pull request are covered.

❗ Current head e2c6b5b differs from pull request most recent head 5155d97. Consider uploading reports for the commit 5155d97 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2699      +/-   ##
==========================================
- Coverage   68.06%   67.66%   -0.41%     
==========================================
  Files         202      202              
  Lines       30407    30343      -64     
  Branches     5938     5932       -6     
==========================================
- Hits        20698    20533     -165     
- Misses       5012     5148     +136     
+ Partials     4697     4662      -35     
Flag Coverage Δ
tests 67.66% <72.72%> (-0.41%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/host/memory_manager/memory_mapper.rs 71.40% <72.72%> (-0.03%) ⬇️
src/main/host/syscall/handler/eventfd.rs 0.00% <0.00%> (-68.58%) ⬇️
src/main/host/descriptor/eventfd.rs 8.08% <0.00%> (-57.58%) ⬇️
src/main/host/syscall/handler/fcntl.rs 50.00% <0.00%> (-9.16%) ⬇️
src/main/host/syscall/handler/mod.rs 77.77% <0.00%> (-5.98%) ⬇️
src/main/host/syscall/handler/unistd.rs 66.00% <0.00%> (-4.21%) ⬇️
src/main/host/syscall/handler/sched.rs 41.79% <0.00%> (-3.53%) ⬇️
src/main/host/syscall/handler/file.rs 80.00% <0.00%> (-3.34%) ⬇️
src/main/host/syscall/handler/mman.rs 78.26% <0.00%> (-3.23%) ⬇️
src/main/host/syscall/handler/time.rs 65.11% <0.00%> (-3.07%) ⬇️
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sporksmith sporksmith merged commit 29901d6 into shadow:main Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MemoryMapper (--use-memory-manager=true) broken on Fedora 37 (kernel 6.1.6)

2 participants