CHERI: Use another pagemap for arena pointers #578
Merged
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
|
Awesome, LGTM. I think we can simplify FrontendMetadata and BackendMetadata, and push all needs for custom meta-data to use a different pagemap. That works for this use case, and Monza. I guess we should think about
Those are the use cases where I have considered a custom meta-data. |
mjp41
approved these changes
Dec 13, 2022
mjp41
reviewed
Dec 13, 2022
| @@ -7,6 +7,9 @@ namespace snmalloc | |||
| { | |||
| /** | |||
| * Protect the ParentRange with a spin lock. | |||
| * | |||
| * Accesses via the ancestor() mechanism will bypass the lock and so | |||
| * should be used only where the resulting data races are acceptable. | |||
There was a problem hiding this comment.
I wonder if there is a way to add some API that requires something to be explicitly set to pass a lock range with an ancestor call.
nwf-msr
force-pushed
the
202212-authmap
branch
from
f690087
to
6ad3499
Compare
These functions depend only on the PAL and so can live lower down the stack.
Separate out the pagemap datastructure (ds/pagemap.h) from the backend adapter shim (backend_helpers/pagemap.h).
More directly ensure that a "basic" pagemap's type matches its "concrete" pagemap parameter's entry type. Absent this check, getting this wrong won't be detected until even further along in template code generation (when considering a method that sees the mismatch).
Instead, take a template parameter for the no-args init() method, so that randomization can be disabled on StrictProvenance architectures (CHERI), where we don't expect it to be useful, even when snmalloc is being built to be otherwise paranoid. Catch callsites up.
nwf-msr
force-pushed
the
202212-authmap
branch
from
6ad3499
to
b2eb295
Compare
nwf-msr
commented
Dec 14, 2022
To date, we've had exactly one kind of Pagemap and it held exactly one type of thing, a descendant of class MetaEntryBase. PagemapRegisterRange tacitly assumed that the Pagemap (adapter) it interacted would therefore store entries that could have .set_boundary() called on them. But in general there's no requirement that this be true; Pagemaps are generic data structures. To enable reuse of the PagemapRegisterRange machinery more generally, change the type of Pagemap::register_range() to take a pointer (rather than an address) and move the MetaEntryBase-specific functionality to the backend_helpers/pagemap adapter.
This will serve as the granularity with which we store authority pointers in the (forthcoming) authmap, so 4K is almost surely too small. 16M is, admittedly, chosen out of a hat.
No use of them, yet, though.
nwf-msr
force-pushed
the
202212-authmap
branch
from
b2eb295
to
5e130c6
Compare
rmn30
reviewed
Dec 14, 2022
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.
Holding the arena pointers in slab metadata invites tricky problems in the presence of misbehaving clients and the current mix-in approach means that the metadata isn't a standard layout type. Here's a first stab at using another
Pagemapfor the arena pointers in modernsnmalloc. We'd tried this approach before, but it feels much less bolted-on now, thanks to the "build-yourself-a-malloc kit" design thatsnmallochas grown into.This is, unfortunately, very slightly (0.2%) slower when used to run SPEC CPU2006
471.omnetpp(more or less a malloc performance benchmark) on the Morello cluster nodes, at the moment. There may be a straightforward optimization I've missed; can investigate.For review, probably best to proceed commit-by-commit rather than all at once, and you can ignore the #577 at the bottom. As is not uncommon for my PRs, we may decide to pull some things off the bottom of this one to merge while the rest bakes a bit more.