Re: [PATCH v2 3/3] fuse: publish io-uring queues with release semantics
Joanne Koong <[email protected]> Thu, 16 Jul 2026 11:21:10 -0700
| Newsgroups | dev.linux.lists.fuse-devel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAJnrk1YCSmK=7UwCsm8KF5kvndk+zM655whcHt85oKbpse8XYw@mail.gmail.com> |
On Wed, Jul 15, 2026 at 3:54=E2=80=AFPM Bernd Schubert <[email protected]> = wrote: > > > > On 7/15/26 19:43, Joanne Koong wrote: > > fuse_uring_create_queue() initializes a fuse_ring_queue and then > > publishes the pointer into ring->queues[qid] with WRITE_ONCE() under th= e > > fch->lock. There are several readers that may concurrently be fetching > > that pointer locklessly and then deferencing it. > > > > WRITE_ONCE() doesn't ensure ordering of the queue's field > > initialization before the ring->queues[qid] pointer assignment. The > > queue must be published with smp_store_release() so the field > > initialization is guaranteed to happen before. > > > > Readers in paths where the read may happen concurrently with the store > > need to use READ_ONCE() because any race involving a plain access is > > undefined. > > > > Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands"= ) > > Cc: [email protected] > > Signed-off-by: Joanne Koong <[email protected]> > > --- > > fs/fuse/dev_uring.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c > > index 51f985154aa1..bf9d51f2a508 100644 > > --- a/fs/fuse/dev_uring.c > > +++ b/fs/fuse/dev_uring.c > > @@ -321,9 +321,11 @@ static struct fuse_ring_queue *fuse_uring_create_q= ueue(struct fuse_ring *ring, > > } > > > > /* > > - * write_once and lock as the caller mostly doesn't take the lock= at all > > + * fch->lock serializes concurrent creators for this qid. > > + * smp_store_release() are for the lockless readers who must see = a > > + * fully initialized queue after &ring->queues[qid] is set > > */ > > - WRITE_ONCE(ring->queues[qid], queue); > > + smp_store_release(&ring->queues[qid], queue); > > spin_unlock(&fch->lock); > > > > return queue; > > @@ -434,7 +436,7 @@ static void fuse_uring_log_ent_state(struct fuse_ri= ng *ring) > > struct fuse_ring_ent *ent; > > > > for (qid =3D 0; qid < ring->nr_queues; qid++) { > > - struct fuse_ring_queue *queue =3D ring->queues[qid]; > > + struct fuse_ring_queue *queue =3D READ_ONCE(ring->queues[= qid]); > > > > if (!queue) > > continue; > > @@ -967,7 +969,7 @@ static int fuse_uring_commit_fetch(struct io_uring_= cmd *cmd, int issue_flags, > > if (qid >=3D ring->nr_queues) > > return -EINVAL; > > > > - queue =3D ring->queues[qid]; > > + queue =3D READ_ONCE(ring->queues[qid]); > > if (!queue) > > return err; > > fpq =3D &queue->fpq; > > @@ -1035,7 +1037,7 @@ static bool is_ring_ready(struct fuse_ring *ring,= int current_qid) > > if (current_qid =3D=3D qid) > > continue; > > > > - queue =3D ring->queues[qid]; > > + queue =3D READ_ONCE(ring->queues[qid]); > > if (!queue) { > > ready =3D false; > > break; > > @@ -1191,7 +1193,7 @@ static int fuse_uring_register(struct io_uring_cm= d *cmd, > > return -EINVAL; > > } > > > > - queue =3D ring->queues[qid]; > > + queue =3D READ_ONCE(ring->queues[qid]); > > if (!queue) { > > queue =3D fuse_uring_create_queue(ring, qid); > > if (!queue) > > > For consistency, maybe READ_ONCE/WRITE_ONCE in fuse_uring_destruct? I didn't add it because I don't think it's necessary (because fuse_uring_destruct can only run after the queue has been created), but I can add it to make things more uniform, if you have a preference for one way or the other. I'll send out v3 with this change. Thanks, Joanne