Skip to content
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

Pagemap Rounding #558

Merged
merged 12 commits into from
Sep 17, 2022
Merged

Pagemap Rounding #558

merged 12 commits into from
Sep 17, 2022

Conversation

mjp41
Copy link
Member

@mjp41 mjp41 commented Sep 16, 2022

This PR ensures that if a pagemap is used in a fixed region that has very large granularity it does require undue additional alignment.

@mjp41 mjp41 requested a review from ihaller September 16, 2022 10:27
src/test/func/pagemap/pagemap.cc Outdated Show resolved Hide resolved
auto end = pointer_align_down(
// Calculate range in pagemap that is associated to this space.
// Over calculate to cover any unaligned parts at either end.
auto b_align = pointer_align_down(b, bits::one_at_bit(GRANULARITY_BITS));
Copy link
Contributor

Choose a reason for hiding this comment

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

While this might take b_align below b, and so logically start our PageMap out of bounds, that's OK because we never access the MetaEntry-s associated with the PageMap itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is b_align only used in address and size computations? If yes, then would it be clearer and safer to get the address of b and align the address down? That way no wild pointer exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

So I have tried to make this capability less code as it is just addresses like you suggest:
9542fbe

src/snmalloc/backend_helpers/pagemap.h Show resolved Hide resolved
src/test/func/pagemap/pagemap.cc Outdated Show resolved Hide resolved
@ihaller
Copy link
Contributor

ihaller commented Sep 16, 2022

I am very happy with this change now.

Check for possible overlap between heap and pagemap, but writing and
reading the heap.
This commit allows the pagemap to return unaligned range of memory. This
means that bump allocation of multiple pagemaps doesn't
waste as much space.
@mjp41 mjp41 merged commit 2f8f376 into microsoft:main Sep 17, 2022
@mjp41 mjp41 deleted the pagemaprounding branch September 17, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants