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

Add {Advise, UncheckedAdvice}::is_supported() #122

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

xzfc
Copy link

@xzfc xzfc commented Sep 12, 2024

Some Linux-specific advices are supported only on recent kernels, e.g. MADV_POPULATE_READ is available starting from Linux 5.14. Some long-term supported distros (e.g. RHEL 8 and Ubuntu 20.04) still have older kernels. So it would be useful to provide a way to perform a runtime check to see whether the specified advice is supported.

This PR adds two similar methods Advise::is_supported() and UncheckedAdvice::is_supported(). They're marked as Linux-only as:

  1. I guess the support for the common (non-Linux) advices should be universal, so there is no need to check.
  2. Only Linux have a documented way to check it1, see madvise(2).

Footnotes

  1. Tho, as for not officially documented way, I've found this approach for BSDs: https://github.com/jart/cosmopolitan/blob/3.8.0/libc/calls/madvise.c#L38-L39

@RazrFalcon
Copy link
Owner

Can you also add a link to the docs. So people would know what the code is doing.

@xzfc xzfc force-pushed the madvise-is-supported branch from cf8952b to 9665299 Compare September 12, 2024 10:46
@xzfc
Copy link
Author

xzfc commented Sep 12, 2024

Added.

@xzfc xzfc force-pushed the madvise-is-supported branch 2 times, most recently from 43e57a1 to 4112b84 Compare September 12, 2024 11:06
@RazrFalcon
Copy link
Owner

Looks like cargo format doesn't like extra spaces.

@xzfc xzfc force-pushed the madvise-is-supported branch from 4112b84 to d3b4f3a Compare September 12, 2024 14:11
@RazrFalcon RazrFalcon merged commit 34776c1 into RazrFalcon:master Sep 12, 2024
13 checks passed
@RazrFalcon
Copy link
Owner

Thanks!

@timvisee
Copy link

@RazrFalcon Would you mind to push an update to crates.io that includes this? 😄

Thanks!

@RazrFalcon
Copy link
Owner

Done.

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