[pull] master from libfuse:master#59
Merged
Merged
Conversation
The per-queue uring thread flushed the submission queue from io_uring_submit_and_wait() in its serving loop outside ring_lock, while off-thread repliers flushed it from io_uring_submit() under ring_lock in fuse_uring_commit_sqe(). liburing's SQ is single-producer in userspace, so two concurrent flushes corrupt sqe_head/ktail: when a filesystem replies from a thread other than the per-queue uring thread this shows up as spurious "Failed to get a ring SQEs", a doubled commit/fetch for one commit_id, or an eventual hang. The commit path also keyed locking on pthread_self() != queue->tid, a fragile proxy for lock ownership. This serialisation is required, not just prudent: io_uring_setup_flags.7 documents that sharing a ring between threads "requires additional synchronization". It is orthogonal to IORING_SETUP_SINGLE_ISSUER, which only makes the kernel reject a second submitter task with -EEXIST and does nothing for the userspace SQ bookkeeping; the flush race exists with or without that flag. Make the rule uniform: every submission-side SQ access takes ring_lock, in commit_sqe() and resubmit() alike, replacing the pthread_self() guess. ring_lock can no longer wrap handle_cqes() - an inline commit on the uring thread would self-deadlock on the non-recursive mutex - so it is taken only around the individual SQ accesses. Submission stays batched via the cqe_processing flag, now _Atomic: while the uring thread runs handle_cqes() it is set, so replies fill their SQE under ring_lock but defer the submit; the thread flushes them with one io_uring_submit() when it clears it. A reply that sees the flag clear - the uring thread idle in io_uring_wait_cqe() - submits at once to wake it. CQ access stays lock-free, as only the uring thread reaps. Closes: #1443 Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
Part 1 dropped cqe_processing and made the serving loop wait-only, so the default path now issues one io_uring_submit() per inline reply instead of coalescing a cqe batch into a single submit. Restore the batching: the loop splits submit_and_wait so the flush stays under ring_lock (part 1's race fix), and cqe_processing - now an _Atomic bool set lock-free around handle_cqes() - again gates the per-reply submit. The SQ stays serialised, so a batched reply is always flushed before the next wait, never stranded. Add a conn flag, io_uring_single_issuer, for a filesystem that replies only from the per-queue uring thread: it skips ring_lock entirely and runs the combined io_uring_submit_and_wait() loop, sound because only that thread touches the ring. The flag reuses a reserved padding bit (ABI unchanged; flag 0 keeps the multi-issuer path) and is mirrored into the ring pool after op.init(), before serving starts. A filesystem enables it with fuse_set_conn_flag(conn, FUSE_CONN_FLAG_SINGLE_ISSUER), a new exported setter modelled on fuse_set_feature_flag(); the io_uring_single_issuer bitfield is the internal storage. Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
For single-issuer queues only fuse_uring_thread() ever issues io_uring_enter() on the ring, so the per-thread registered ring-fd index is always valid. Registering it lets io_uring skip the fd grab/put on every enter, which is measurable with libfuse's shared, threaded file table. A ring shared between separate submit and reap threads cannot use this (each issuing thread would need its own index), so it stays gated on io_uring_single_issuer. Registration needs kernel >= 5.18 and is non-fatal: on failure the queue simply keeps using the normal ring fd. Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
These example filesystems always send their reply from the per-queue io-uring worker thread. The ones that run a helper thread use it only for cache-invalidation notifications over /dev/fuse, not ring submissions, so they too satisfy the io_uring_single_issuer contract. Call fuse_set_conn_flag(conn, FUSE_CONN_FLAG_SINGLE_ISSUER) in their init() handlers so they take the lock-free, registered-ring-fd fast path. It is harmless when io-uring is not in use. Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
fuse_set_conn_flag() collects the libfuse-side connection hints that are not negotiated with the kernel. no_interrupt was the one such hint still set only by direct field assignment. Define FUSE_CONN_FLAG_NO_INTERRUPT alongside FUSE_CONN_FLAG_SINGLE_ISSUER and map it onto conn->no_interrupt in the same switch, and convert the example init() handlers to set it through the setter. Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )