Skip to content

Add MADV_SOFT_OFFLINE definition for RISC-V musl targets #4447

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

Conversation

mnissler-rivos
Copy link
Contributor

@mnissler-rivos mnissler-rivos commented May 6, 2025

Description

This adds missing MADV_SOFT_OFFLINE definitions for RISC-V musl targets. It is analogous to #2391 where the same definitions were added for RISC-V glibc targets. The patch addresses a rustix build issue: bytecodealliance/rustix#1462

Sources

https://github.com/bminor/musl/blob/c47ad25ea3b484e10326f933e927c0bc8cded3da/include/sys/mman.h#L99

Checklist

  • Relevant tests in libc-test/semver have been updated
    -- I have added MADV_SOFT_OFFLINE to libc-test/semver/linux-riscv64gc.txt for good measure
  • No placeholder or unstable values like *LAST or *MAX are
    included (see rust-lang/libc#3131)
  • Tested locally (cd libc-test && cargo test --target riscv64gc-unknown-linux-{gnu,musl});
    especially relevant for platforms that may not be checked in CI
    -- I've tested the riscv64gc-unknown-linux-musl target against a riscv-gnu-toolchain musl build - there are actually a bunch of other issues with struct layout, field name, and value inconsistencies, but these are unrelated to my change.

@rustbot label +stable-nominated

@rustbot rustbot added O-linux O-musl O-riscv O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels May 6, 2025
@mnissler-rivos
Copy link
Contributor Author

There's one consideration I want to bring up: Instead of adding the missing definitions to the RISC-V musl targets, we could also opt for a single definition at linux_like level, with cfg guards to exempt mips platforms. The definition was actually more centralized, but 1cbc523 changed it to the approach that is currently used, and I don't know what the rationale/preference is. Let me know if you'd prefer a conditionally compiled definition in linux_like and I'll happily update the pull request.

@tgross35
Copy link
Contributor

tgross35 commented May 9, 2025

There's one consideration I want to bring up: Instead of adding the missing definitions to the RISC-V musl targets, we could also opt for a single definition at linux_like level, with cfg guards to exempt mips platforms. The definition was actually more centralized, but 1cbc523 changed it to the approach that is currently used, and I don't know what the rationale/preference is. Let me know if you'd prefer a conditionally compiled definition in linux_like and I'll happily update the pull request.

I don't like this repo's layout :) we'll be reorganizing things soon, don't worry about it for now.

@tgross35 tgross35 added this pull request to the merge queue May 9, 2025
Merged via the queue into rust-lang:main with commit 181b043 May 9, 2025
49 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-linux O-musl O-riscv O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants