-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
std: linux uring kernel interfaces #2525
Conversation
std/os.zig
Outdated
// kernel has the matching store barrier for the update. The | ||
// kernel also ensures that previous stores to CQEs are ordered | ||
// with the tail update. | ||
@fence(.SeqCst); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure what sort of fence these should be. The upstream examples use these definitions: http://git.kernel.dk/cgit/liburing/tree/src/barrier.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has acquire semantics for loads and release semantics for stores, see here.
On x86 that means you don't need synchronization due to its memory model but on e.g. arm64 you'd insert ldar and stlr instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After seeing some commits
2019-04-30 | io_uring: remove unnecessary barrier after unsetting IORING_SQ_NEED_WAKEUP | Stefan Bühler
2019-04-30 | io_uring: remove unnecessary barrier after incrementing dropped counter | Stefan Bühler
2019-04-30 | io_uring: remove unnecessary barrier before reading SQ tail | Stefan Bühler
2019-04-30 | io_uring: remove unnecessary barrier after updating SQ head | Stefan Bühler
2019-04-30 | io_uring: remove unnecessary barrier before reading cq head | Stefan Bühler
2019-04-30 | io_uring: remove unnecessary barrier before wq_has_sleeper | Stefan Bühler
I'm not sure if that comment is still true? @stbuehler ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. The standard way would be using acquire on cq.khead
here, which would order the load on cq.khead
before all other loads following, especially those reading the CQE.
A SeqCst
fence is basically a full memory barrier afaik, and should work of course; an Acquire
fence should work too, or a simple read memory barrier.
I didn't bother fixing liburing, because I'm not that crazy about C, and liburing doesn't have atomic/fences - it only knows the barriers you linked (it probably should use C11 atomics instead, and then it should be easy).
You can see my proof-of-concept code for rust here (for now): https://github.com/stbuehler/rust-io-uring/blob/wip/io-uring/src/lib.rs
0574a6c
to
162af88
Compare
I've taken the wrapper out of this PR, I'll send it as another PR soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to change, everything else looks good.
This branch contains the start of a uring based command buffer structure.Currently only implemented for linux (and needs kernel 5.1+); I have plans to modify/extend the interface to other operating systems and older linux versions.
Early commits on the branch are fixes for other areas, please consider cherry-picking them now even if the final commit is too much to review right away!