Futex #1907
Futex #1907
Conversation
JonathanWoollett-Light
force-pushed
the
futex
branch
2 times, most recently
from
46236db to
53a2673
Compare
|
Does https://github.com/m-ou-se/linux-futex meet your needs? |
This is a useful reference, thank you. I do think this functionality should be included in nix. |
I don't disagree in principle, but I'd like to see nix's support provide some value-add compared to prior art. For example, Mara's crate only (appears to) supports Linux futexes. It'd be nice if nix supported the BSD variants as well. |
JonathanWoollett-Light
force-pushed
the
futex
branch
8 times, most recently
from
4244333 to
c6596f9
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
from
c6596f9 to
9ab4f77
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
4 times, most recently
from
e348afe to
43789b3
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
5 times, most recently
from
0e6f2ff to
e558f2a
Compare
I haven't implemented BSD since it is not personally useful to me. I think if someone needs this they can implement it. This is still a significant value add to nix and I would also argue my implementation is smaller and simpler than linux-futex and this I would argue makes it better. |
There was a problem hiding this comment.
This is still a significant value add to nix and I would also argue my implementation is smaller and simpler than linux-futex and this I would argue makes it better.
I completely agree that it's a simpler/more direct wrapping of the underlying futex operations. However, given that there's an existing implementation which (IMO) provides a nice abstraction over the futex syscall, I'm uncertain that splitting the investment into Rust futex support is actually better outcome.
As I mentioned, I don't object to including futex support in the nix crate if there is a compelling reason to do so. What is the use case where the linux-futex crate is insufficient? Missing functionality?
| /// then, when another process or thread executes a FUTEX_WAKE, the caller will receive the | ||
| /// signal number that was passed in val. | ||
| /// | ||
| /// **Because it was inherently racy, this is unsupported from Linux 2.6.26 onward.** |
There was a problem hiding this comment.
If this is racy and has been removed since 2008, why wrap it?
There was a problem hiding this comment.
It is a part of the public futex API. I have no strong opinion either way on keeping or removing it here.
| pub fn new(val: u32) -> Self { | ||
| Self(UnsafeCell::new(val)) | ||
| } | ||
| /// If the value of the futex: |
There was a problem hiding this comment.
The rest of the nix source puts newlines between structs, impls, functions, etc. I'd appreciate if we matched that style.
There was a problem hiding this comment.
Fixed.
| /// // we expect the timeout will pass. | ||
| /// | ||
| /// let instant = Instant::now(); | ||
| /// assert_eq!(futex.wait(0, Some(TIMEOUT)),Err(Errno::ETIMEDOUT)); |
There was a problem hiding this comment.
My familiarity is mostly with the PI variants of futexes, but my understanding is that proper futex use in general involves trying the operation in userspace via atomic operations before falling back to a syscall.
We should make sure the examples/tests demonstrate this, if only to avoid being incredibly confusing about their proper use.
There was a problem hiding this comment.
My understanding is that a futex is used like a Rust Condvar or when more appropriate than a mutex (so as to avoid additional work).
I'm not sure what you mean by:
proper futex use in general involves trying the operation in userspace via atomic operations before falling back to a syscall.
or how this would look.
Could you maybe provide an example?
There was a problem hiding this comment.
See https://linux.die.net/man/4/futex for the description (note it's futex(4), not the typical futex(2)).
The point of futexes is that syscalls are slow, but we can make the operations faster in uncontended cases by never leaving userspace. This is accomplished by doing a CAS operation and only performing the syscall if it fails.
See https://github.com/rtzoeller/rust-priority-inheriting-lock/blob/6225e90971d9aa72f070145788669e421cf012d4/src/lib.rs#L48 for an example of the userspace operation and falling back to the syscall for LOCK_PI (incidentally using linux-futex).
There was a problem hiding this comment.
I don't think this matter in non-highly contentious cases, not all use cases of a futex are highly contentious and not all are using it to implement a locking mechanism.
To have an example that properly illustrates what you suggest would be seem to need to be including an implementation of a priority-inheriting lock as an example.
Relating to docs, if you have suggestions for additions here please add a suggestion. Perhaps a comment suggesting that if a user is looking for a locking mechanism and does not require handling of priority-inheritance, they should use a mutex?
I'm not sure otherwise what changes you would suggest?
src/sys/mod.rs
Outdated
|
|
||
| /// Fast user-space locking. | ||
| #[cfg(any(target_os = "android", target_os = "linux"))] | ||
| pub mod futex; |
There was a problem hiding this comment.
We should probably put this behind a feature (see #1611).
There was a problem hiding this comment.
Added a futex feature.
JonathanWoollett-Light
force-pushed
the
futex
branch
2 times, most recently
from
32aa8e9 to
b1c13c9
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
from
b1c13c9 to
1225513
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
from
1225513 to
977f3f3
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
from
977f3f3 to
8c0da9a
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
from
8c0da9a to
64045be
Compare
JonathanWoollett-Light
force-pushed
the
futex
branch
from
64045be to
a83b831
Compare
|
I still don't understand what advantage this PR has over the existing linux-futex crate. |
The advantage is having it in Nix and it aligning with the design of Nix. The point of nix is to provide wrappers to Unix-like systems.. Providing bindings to Unix-like systems except for functionality that is already covered by other crates would mean removing like 80% of already implemented functionality. |
The point of Nix is to provide idiomatic zero-cost access to Unix system APIs. But providing access to ALL Unix system APIs isn't a goal. Nor is it productive to duplicate work that others have already done. That wastes developer resources and increases our maintenance burden. If another crate already does a good job, then there's no reason why we should need to, too. |
If this is really the path Nix pursues it should be documented:
I think this sounds absurd. I do not agree with this, I see the reasoning but I think it is shallow. I could for instance post an issue on half the functionality on Nix saying it should be removed since decent alternatives exist. I could also leave this comment on most currently open PRs saying a decent alternative exists that support this. Following this almost completely eliminates the purpose of Nix. A key value is that Nix covers many things, that I don't need 30 different crates to cover each small component. That Nix offers an interface to all this functionality that reasonably neatly fits together. |
Add futex interface.