-
Notifications
You must be signed in to change notification settings - Fork 666
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
Add MapFlags::map_hugetlb_with_size_log2
#2125
Conversation
src/sys/mman.rs
Outdated
/// `Some(MapFlags::MAP_HUGETLB | MAP_HUGE_1GB)`. | ||
#[cfg(target_os = "linux")] | ||
#[cfg_attr(docsrs, doc(cfg(all())))] | ||
pub fn map_hugetlb_with_shift(huge_page_size_log2: u8) -> Option<Self> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't make it const
because I couldn't find a way to convert i32
into InternalBitFlags
.
c2e1d52
to
46a8272
Compare
MapFlags::map_hugetlb_with_size_log2
879460f
to
1ab8175
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget a changelog entry.
/// let f = MapFlags::map_hugetlb_with_size_log2(30).unwrap(); | ||
/// assert_eq!(f, MapFlags::MAP_HUGETLB | MapFlags::MAP_HUGE_1GB); | ||
/// ``` | ||
#[cfg(target_os = "linux")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably include android and fuchsia, too. At least, libc defines MAP_HUGETLB for those platforms. But it doesn't define MAP_HUGE_SHIFT. Is that an oversight in libc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that an oversight in libc?
Maybe. Either way, we would need first introduce the constant to libc
and only then add expand list of targets, so I think it's better to do it in a later PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I filed a PR adding these symbols for Android or Linux: rust-lang/libc#3444
1ab8175
to
d2785bf
Compare
We can merge this PR as-is, but @newpavlov could you please leave a TODO here: support Andorid and Fuchsia when libc 0.2.151 gets released |
Closes #2124