Skip to content

Add MAP_SHARED_VALIDATE & MAP_SYNC #2499

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

Merged
merged 8 commits into from
Sep 15, 2024
Merged

Add MAP_SHARED_VALIDATE & MAP_SYNC #2499

merged 8 commits into from
Sep 15, 2024

Conversation

sectordistrict
Copy link
Contributor

@sectordistrict sectordistrict commented Sep 13, 2024

This pull adds two flags: MAP_SHARED_VALIDATE & MAP_SYNC implemented on https://github.com/rust-lang/libc

The MAP_SHARED_VALIDATE flag was added to linux since 4.15, its a flag that does two things: provide backwards compatibility with old mmap implementations that don't check for unknown flags, and allow new flags to be added.

MAP_SYNC, on the other hand, was added to allow mmap to utilize Direct Accses (DAX) on hardware that support it (non-volatile memory devices) or in general: any ram-shaped filesystem.

both flags are available on both linux and android. (Edit: The android kernel does not support the flags)

Notice:
mmap always returns ENOTSUPP when MAP_SYNC is used on filesystems that are not mounted with DAX enabled (normal everyday filesystems), this means that this kind of test would not be possible on github CI.

What does this PR do

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

This pull adds two flags: MAP_SHARED_VALIDATE & MAP_SYNC implemented on
https://github.com/rust-lang/libc

The MAP_SHARED_VALIDATE flag was added to linux since 4.15, its a flag
that does two things: provide backwards compatibility with old mmap
implementations that don't check for flags, and allow new flags to be
added.

MAP_SYNC, on the other hand, was added to allow mmap to utilize Direct
Accses (DAX) on hardware that support it (non-volatile memory devices)
or in general: any ram-shaped filesystem.

both flags are available on both linux and android.
@SteveLauC SteveLauC self-requested a review September 14, 2024 02:18
@sectordistrict
Copy link
Contributor Author

I was using a local fork of nix that had these updates. Didn't realize I didn't create a branch for this pull request and it's very cluttered, @SteveLauC, I'm thinking we can close this PR and I'll create a new one?

src/sys/mman.rs Outdated
Comment on lines 44 to 45
/// Force mmap to check and fail on unknown flags. This also enables `MAP_SYNC`.
#[cfg(all(any(target_arch = "x86"), not(linux_android), not(target_os = "hurd"), not(target_env = "uclibc"), not(freebsdlike)))]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Force mmap to check and fail on unknown flags. This also enables `MAP_SYNC`.
#[cfg(all(any(target_arch = "x86"), not(linux_android), not(target_os = "hurd"), not(target_env = "uclibc"), not(freebsdlike)))]
/// Force mmap to check and fail on unknown flags. This also enables `MAP_SYNC`.
#[cfg(target_os = "linux")]

This flag is available on all the Linux targets.

src/sys/mman.rs Outdated
Comment on lines 148 to 149
/// Do not write through the page caches, write directly to the file. Used for Direct Access (DAX) enabled file systems.
#[cfg(all(any(target_arch = "x86"), not(linux_android), not(target_os = "hurd"), not(target_env = "uclibc"), not(freebsdlike)))]
Copy link
Member

Choose a reason for hiding this comment

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

This flag is not that lucky, uClibc does not have it, for the glibc and musl, seems that they all lack the support for the MIPS target, so this should work for now:

Suggested change
/// Do not write through the page caches, write directly to the file. Used for Direct Access (DAX) enabled file systems.
#[cfg(all(any(target_arch = "x86"), not(linux_android), not(target_os = "hurd"), not(target_env = "uclibc"), not(freebsdlike)))]
/// Do not write through the page caches, write directly to the file. Used for Direct Access (DAX) enabled file systems.
// Available on Linux glibc and musl, MIPS* target excluded.
#[cfg(all(target_os = "linux", not(any(target_arch = "mips", target_arch = "mips64", target_arch = "mipsel", target_arch = "mips64el")), not(target_env = "uclibc")))]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's great, thanks for the suggestions and apologies again for all the clutter

@@ -0,0 +1 @@
Added `MAP_SHARED_VALIDATE` & `MAP_SYNC` for x86.
Copy link
Member

Choose a reason for hiding this comment

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

When you apply the suggested changes, changelog should also be updated:)

@SteveLauC
Copy link
Member

I'm thinking we can close this PR and I'll create a new one?

You can, of course. But the current PR looks not bad👀

@sectordistrict
Copy link
Contributor Author

They've all passed! again thanks for the CI magic! @SteveLauC

Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for correcting the MIPS target_arch value and the patch!

@SteveLauC SteveLauC added this pull request to the merge queue Sep 15, 2024
Merged via the queue into nix-rust:master with commit 7d75bbc Sep 15, 2024
38 checks passed
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.

2 participants