Add POSIX AIO support #483
Conversation
asomers
force-pushed
the
aio2
branch
2 times, most recently
from
dd0b6c2 to
372d2b3
Compare
|
All failures are actually in the dependent gcc crate, which no longer compiles with rust 1.7.0. |
|
☔ The latest upstream changes (presumably #478) made this pull request unmergeable. Please resolve the merge conflicts. |
POSIX AIO is a standard for asynchronous file I/O. Read, write, and fsync operations can all take place in the background, with completion notification delivered by a signal, by a new thread, by kqueue, or not at all. This commit supports all standard AIO functions. However, lio_listio is disabled on macos because it doesn't seem to work, even though the syscall is present. The SigEvent class, used for AIO notifications among other things, is also added. Also, impl AsRef for TimeVal and TimeSpec
|
Could I please get some review of this PR? |
| // lio_listio wouldn't work, because that function needs a slice of AioCb, | ||
| // and they must all be the same type. We're basically stuck with using an | ||
| // unsafe function, since aio (as designed in C) is an unsafe API. | ||
| pub unsafe fn from_slice(fd: RawFd, offs: off_t, buf: &[u8], |
There was a problem hiding this comment.
With the version of these functions that take references to buffers, I wonder if it would be reasonable to do something to ensure that the lifetime of the reference is at least as long as the lifetime of the AioCb. Without this, there buffer could go out of scope in the calling context and we would be working with an invalid pointer.
I think this would typically be done with something like PhantomData (when not storing the reference itself).
| Errno::result(unsafe { libc::aio_fsync(mode as ::c_int, p) }).map(drop) | ||
| } | ||
|
|
||
| /// Asynchously reads from a file descriptor into a buffer |
There was a problem hiding this comment.
nit: Typo with "Asynchronously"
|
Hi @asomers. Deep apologies for the delay in getting you a review on this. Having AIO support in nix will be a wonderful addition and this looks like it will work great. I think my main concern right now is with the validity of buffers we are using for doing asynchronous reads/writes and ensuring that the lifetime of the buffer is at least as long as the asynchronous operation -- Without this, I think the kernel could easily end up stomping on memory (which would result in all manner of bad things). Is this concern warranted? I have mixed feelings on the AIO subsystem in general (my experience with Linux only) and am not sure where it is headed. That being said, there are cases where it is beneficial for it to be used today and will be happy to accept this change (once we discuss a bit more). |
|
Point taken, @posborne . I'll try to fix the buffer lifetime issue. Regarding the AIO subsystem: I can understand why you're skeptical, because it's not well implemented on Linux. On Linux, POSIX AIO performs poorly and the nonstandard libaio performs well. However, in the BSD world, POSIX AIO performs well and there is no libaio. Unfortunately, libaio does not have a compatible API at all. The differences are even worse for higher level libraries. mio and tokio-core basically need completely different implementations to use posix AIO vs libaio. But POSIX AIO does exist and work on Linux, and it works well on the BSDs, so I think it's worth including in nix. |
|
Thanks for the summary @asomers and points well taken. I agree that exposing the Posix AIO interface (even if not performant with the glibc userspace implementation on linux) is the right thing to do in nix. Mio or other libraries should be the ones to decide what abstraction to use on which host operating systems. |
|
Excellent, feeling better about this now that we track lifetimes. I think it would be good to consider the following additional improvements in the future. Let me know your thoughts:
@homu r+ |
|
📌 Commit b740156 has been approved by |
Add POSIX AIO support POSIX AIO is a standard for asynchronous file I/O. Read, write, and fsync operations can all take place in the background, with completion notification delivered by a signal, by a new thread, by kqueue, or not at all. The SigEvent class, used for AIO notifications among other things, is also added.
|
⚡ Test exempted - status |
|
Good idea regarding class vs instance methods, @posborne. I'll take a swing at it. I think there's also some runtime safety checks we could add, if we're willing to expand the |
|
@asomers I think the runtime check sounds reasonable. |
POSIX AIO is a standard for asynchronous file I/O. Read, write, and
fsync operations can all take place in the background, with completion
notification delivered by a signal, by a new thread, by kqueue, or not
at all.
The SigEvent class, used for AIO notifications among other things, is
also added.