jemalloc: disable transparent hugepages by default on ARMv6/7 #33260
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
|
I think enabling THP on 32-bit ARM is not possible by default, given that it depends on LPAE (and if LPAE is enabled, the kernel won't run on CPUs that lack it): |
dezgeg
reviewed
Jan 1, 2018
| # jemalloc is unable to correctly detect transparent hugepage support on | ||
| # ARM, and the default kernel ARMv6/7 kernel does not enable it, so we | ||
| # explicitly disable support | ||
| thpSupport ? stdenv.system != "armv7l-linux" && |
There was a problem hiding this comment.
I'd also add a link to jemalloc/jemalloc#526 in the comments, so we know when we can strip this.
The default NixOS kernels for ARMv7 (and probably ARMv6) do not have support for transparent huge pages, but jemalloc is unable to detect this. This is a known bug and the current solution is to pass --disable-thp to ./configure.
lopsided98
force-pushed
the
jemalloc-thp
branch
from
c1daa60
to
98dd418
Compare
Mic92
reviewed
Jan 2, 2018
| # jemalloc is unable to correctly detect transparent hugepage support on | ||
| # ARM (https://github.com/jemalloc/jemalloc/issues/526), and the default | ||
| # kernel ARMv6/7 kernel does not enable it, so we explicitly disable support | ||
| thpSupport ? !stdenv.isArm }: |
There was a problem hiding this comment.
I would not expose this as an option.
The only common use case so far seems ARMv6/ARMv7 support. The way this option is exposed might collide with a package with the same name. Also the option naming on its own is not self-descriptive without context.
kierdavis
added a commit
to kierdavis/config
that referenced
this pull request
Feb 17, 2018
This overlays NixOS/nixpkgs#33260 on top of the current release channel.
10 tasks
kierdavis
added a commit
to kierdavis/config
that referenced
this pull request
Sep 27, 2022
This overlays NixOS/nixpkgs#33260 on top of the current release channel.
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.
Motivation for this change
This PR disables transparent hugepage support in jemalloc by default on ARMv6/7 (adding a parameter that allows it to be enabled if desired). The NixOS ARMv6/7 kernels do not enable transparent hugepage support, but jemalloc is not able to detect this (see jemalloc/jemalloc#526).
Even though the
--disable-thpflag is available in jemalloc 4.5.0, it does not work correctly in this version, so this PR also updates jemalloc to 5.0.1 (which requires a large rebuild with the possibility of breakage).Alternatively, this problem could be solved by enabling THP in the default kernel config, but it still might be good to have an option to disable it in jemalloc.
Also, is
stdenv.systemthe correct way to do platform detection nowadays (with the upcoming better cross compiling support)?Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)