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

Should Mutex and Condvar respect priorities? #128231

Open
joboet opened this issue Jul 26, 2024 · 9 comments
Open

Should Mutex and Condvar respect priorities? #128231

joboet opened this issue Jul 26, 2024 · 9 comments
Labels
A-atomic Area: Atomics, barriers, and sync primitives C-discussion Category: Discussion or questions that doesn't represent real issues. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@joboet
Copy link
Member

joboet commented Jul 26, 2024

std does not have an API for setting the thread priority, and doesn't currently support priority inheritance on the majority of platforms (Linux and Windows in particular). Given this, does it still make sense to provide priority inheritance on any platform, when it is not guaranteed, won't be used by default and cannot be relied upon when using external libraries without inspecting the code? Or wouldn't it make more sense to deal with all the priority stuff in a separate crate? This would then allow reducing the number of synchronization primitive implementations inside std (we could switch to the queue-based variant on quite some platforms), thus simplifying efforts like #128203.

@joboet joboet added T-libs Relevant to the library team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. I-libs-nominated Nominated for discussion during a libs team meeting. A-atomic Area: Atomics, barriers, and sync primitives labels Jul 26, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 26, 2024
@VorpalBlade
Copy link

I would for my day job (where we use real time Linux patches) have a use case for having priority inheritance even on Linux. That would allow using third party crates on some projects. Currently it is a massive pain to use PI since virtually nothing third party (or std) uses it (I think I have seen two crates for PI on Linux on crates.io). You basically have to roll your own everything for synchronisation.

Furthermore I expect that on some targets that are more focused on real-time (QNX for example) not having PI would annoy users. (Of course that is a tire 3 target, and I don't know if it has PI currently.)

Pethaps there could be a way to enable PI globally, like the global allocator attribute? Since Rust doesn't use pthreads mutices any more it is unfortunately not as easy as enabling the flag but would require larger implementation changes in std.

@the8472
Copy link
Member

the8472 commented Jul 26, 2024

std does not have an API for setting the thread priority

yet. rust-lang/libs-team#195

@joboet
Copy link
Member Author

joboet commented Jul 29, 2024

I would for my day job (where we use real time Linux patches) have a use case for having priority inheritance even on Linux. That would allow using third party crates on some projects. Currently it is a massive pain to use PI since virtually nothing third party (or std) uses it (I think I have seen two crates for PI on Linux on crates.io). You basically have to roll your own everything for synchronisation.

Furthermore I expect that on some targets that are more focused on real-time (QNX for example) not having PI would annoy users. (Of course that is a tire 3 target, and I don't know if it has PI currently.)

Pethaps there could be a way to enable PI globally, like the global allocator attribute? Since Rust doesn't use pthreads mutices any more it is unfortunately not as easy as enabling the flag but would require larger implementation changes in std.

That sounds like a good candidate a build-std option. It would definitely increase the maintenance burden, though, but that needn't be a blocker. The question then still remains whether the default should be platform-dependent.

@the8472
Copy link
Member

the8472 commented Jul 29, 2024

Isn't it generally better to have the wake queues in the kernel even when no PI is involved? E.g. on a RWLock a single FUTEX_WAKE can wake up all the waiters.

@joboet
Copy link
Member Author

joboet commented Jul 29, 2024

Definitely! I just wondered whether we really have to bother on the platforms were PI is difficult to maintain.

@joshtriplett
Copy link
Member

We definitely want to keep using futexes on Linux, and in general we probably don't want to switch to a userspace queue on any target that has a more efficient OS mechanism. I don't think we need to guarantee PI, but we should attempt to give the kernel enough information to handle priority on targets that have built-in support for it, including Linux.

@joshtriplett
Copy link
Member

cc @m-ou-se as the resident expert on our synchronization code.

@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 5, 2024
@Amanieu
Copy link
Member

Amanieu commented Aug 23, 2024

The conclusion of the libs meeting was that In general we don't make any guarantees about thread priority since this is very OS-specific and not supported on all targets. However we should make a best effort to support PI where possible as long as this doesn't impact performance, but this is purely QoI and not an API guarantee.

@the8472
Copy link
Member

the8472 commented Aug 24, 2024

One data point of someone wanting priority inheritance for game development: https://users.rust-lang.org/t/mutex-priority-inversion/114584?u=the8472

@m-ou-se m-ou-se removed the I-libs-nominated Nominated for discussion during a libs team meeting. label Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-atomic Area: Atomics, barriers, and sync primitives C-discussion Category: Discussion or questions that doesn't represent real issues. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants