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

Hugepage: Don't check for system setting #260

Merged
merged 1 commit into from
Jan 1, 2022

Conversation

mkroening
Copy link
Member

madvise still works if hugepages are disabled system wide. It just does not have an effect then.

madvise still works if hugepages are disabled system wide. It just does not have an effect then.
@codecov
Copy link

codecov bot commented Jan 1, 2022

Codecov Report

Merging #260 (2c453a3) into master (fd9d2fd) will increase coverage by 0.66%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
+ Coverage   62.58%   63.25%   +0.66%     
==========================================
  Files          16       16              
  Lines        2475     2449      -26     
==========================================
  Hits         1549     1549              
+ Misses        926      900      -26     
Impacted Files Coverage Δ
src/bin/uhyve.rs 0.45% <0.00%> (ø)
src/utils.rs 100.00% <ø> (+49.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd9d2fd...2c453a3. Read the comment docs.

@stlankes
Copy link
Collaborator

stlankes commented Jan 1, 2022

@mkroening
Copy link
Member Author

mkroening commented Jan 1, 2022

We do: https://github.com/hermitcore/uhyve/blob/fd9d2fd54c4561f51d5262a43b91f071af643b87/src/linux/uhyve.rs#L422-L427

But madvise only gives an advise to the kernel. We do not have to check weather the user has disabled transparent hugepages. If the user did explicitly disable it on the system, madvise is ignored, as far as I know (at my system at least).

This merely removes the check and defaults to on. Users can still disable huge pages either system wide or via the CLI flag.

@stlankes
Copy link
Collaborator

stlankes commented Jan 1, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 1, 2022

@bors bors bot merged commit 36c04bd into hermit-os:master Jan 1, 2022
@mkroening mkroening deleted the huge-page-check branch January 1, 2022 20:08
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.

None yet

2 participants