Skip to content

Provide the same sched.h constants for musl as gnu#3534

Closed
alyssais wants to merge 1 commit intorust-lang:mainfrom
alyssais:musl-sched
Closed

Provide the same sched.h constants for musl as gnu#3534
alyssais wants to merge 1 commit intorust-lang:mainfrom
alyssais:musl-sched

Conversation

@alyssais
Copy link

I don't understand why these constants from Linux headers aren't just defined in linux_like/mod.rs. Surely they'd be the same for every libc?

@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 11, 2024

📌 Commit 2670fb1 has been approved by JohnTitor

It is now in the queue for this repository.

bors added a commit that referenced this pull request Jan 11, 2024
Provide the same sched.h constants for musl as gnu

I don't understand why these constants from Linux headers aren't just defined in linux_like/mod.rs.  Surely they'd be the same for every libc?
@bors
Copy link
Contributor

bors commented Jan 11, 2024

⌛ Testing commit 2670fb1 with merge 0cad2c2...

@bors
Copy link
Contributor

bors commented Jan 11, 2024

💔 Test failed - checks-actions

@JohnTitor
Copy link
Member

CI fails: https://github.com/rust-lang/libc/actions/runs/7487138095/job/20379076829
It'd also be great if you could rebase onto main to trigger full CI.

@tgross35
Copy link
Contributor

@alyssais are you able to update this to get it over the line? If you need help figuring out the CI, let me know.

@tgross35
Copy link
Contributor

@alyssais ping, are you able to update this?

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Dec 7, 2024
@tgross35 tgross35 force-pushed the musl-sched branch 2 times, most recently from 0e177e5 to 1adff9a Compare December 7, 2024 07:40
@rustbot rustbot added the O-gnu label Dec 7, 2024
@tgross35 tgross35 added this pull request to the merge queue Apr 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 25, 2025
@tgross35 tgross35 force-pushed the musl-sched branch 2 times, most recently from f2838cc to 4286e0e Compare April 25, 2025 04:51
@tgross35 tgross35 enabled auto-merge April 25, 2025 04:51
Comment on lines +5636 to +5639
pub const CLONE_NEWTIME: c_int = 0x80;
pub const CLONE_CLEAR_SIGHAND: c_ulonglong = 0x100000000;
pub const CLONE_INTO_CGROUP: c_ulonglong = 0x200000000;

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually CLONE_NEWTIME is defined at https://github.com/kraj/musl/blob/1b06420abdf46f7d06ab4067e7c51b8b63731852/include/sched.h#L52, but CLONE_CLEAR_SIGHAND and CLONE_INTO_CGROUP do not seem to be defined in Musl

Copy link
Author

Choose a reason for hiding this comment

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

That's true, but given the values are passed straight to the kernel anyway, does it matter?

Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't really be peeking into what the libcs do with constants, i.e. whether the values go right to the kernel or if they get intercepted. Also it is a CI failure.

Copy link
Author

Choose a reason for hiding this comment

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

As far as I can tell Glibc doesn't define CLONE_CLEAR_SIGHAND in a public header either — only the linux/sched.h kernel header and the internal linux/clone3.h header do. What's the difference?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe glibc actually reexports the kernel headers directly, while musl redefines a specific subset that is more cross-platform compatible. Does that seem to match up?

Copy link
Author

Choose a reason for hiding this comment

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

On Alpine at least, musl doesn't install any headers under linux/ — you're expected to separately install the kernel headers. So it's really just a packaging difference, as I understand it. The API is the same.

Copy link
Author

Choose a reason for hiding this comment

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

@tgross35 so what do we do here? I'm noticing a lot of other constants that are also defined in kernel headers yet only available for Glibc in libc. It would be good to have a path forward for these.

Copy link
Contributor

@tgross35 tgross35 Dec 4, 2025

Choose a reason for hiding this comment

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

I think this is okay since we are already including linux/sched.h in tests. In general we are trying to move away from providing linux/ exports and instead suggesting https://docs.rs/linux-raw-sys/latest/linux_raw_sys/, but if this is expected to work well on musl, I don't think there is too much harm in smoothing the API across platforms a bit.

Could you rebase?

@tgross35
Copy link
Contributor

tgross35 commented Sep 1, 2025

@rustbot author

@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@alyssais
Copy link
Author

alyssais commented Dec 4, 2025

Oh, looks like this has already been done in 3ba96df without me noticing :)

@alyssais alyssais closed this Dec 4, 2025
@tgross35
Copy link
Contributor

tgross35 commented Dec 4, 2025

Well sorry about that, I didn't realize there was overlap.

That PR can't actually be backported due to the type changes there. So if you are interested, feel free to prepare a version of what you have here that targets the libc-0.2 branch.

@tgross35 tgross35 removed the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants