On Sat, Jan 25, 2025 at 9:44 AM Bernd Schubert <bschubert@xxxxxxx> wrote:
>
> This is set and read by different threads, we better use
> _ONCE.
>
> Fixes: 284985711dc5 ("fuse: Allow to queue fg requests through io-uring")
> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
> ---
> fs/fuse/dev_uring.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 9af5314f63d54cb1158e9372f4472759f5151ac3..257ee375e79a369c18088664781dd29d538078ac 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -36,7 +36,7 @@ static void uring_cmd_set_ring_ent(struct io_uring_cmd *cmd,
> struct fuse_uring_pdu *pdu =
> io_uring_cmd_to_pdu(cmd, struct fuse_uring_pdu);
>
> - pdu->ent = ring_ent;
> + WRITE_ONCE(pdu->ent, ring_ent);
> }
>
> static struct fuse_ring_ent *uring_cmd_to_ring_ent(struct io_uring_cmd *cmd)
> @@ -44,7 +44,7 @@ static struct fuse_ring_ent *uring_cmd_to_ring_ent(struct io_uring_cmd *cmd)
> struct fuse_uring_pdu *pdu =
> io_uring_cmd_to_pdu(cmd, struct fuse_uring_pdu);
>
> - return pdu->ent;
> + return READ_ONCE(pdu->ent);
> }
Not an expert in this so there's a good chance i'm wrong here, but why
do we need _ONCE? from what I understand, we only read pdu->ent when
handling IO_URING_F_CANCEL or when preparing to send the queued
request. It looks like it's always guaranteed that pdu->ent is set
before any reads on pdu->ent for the case of sending a queued request,
and pdu->ent gets read when making a request cancellable (which
happens when we register the cmd and when we do a commit), but it
seems like it would always be guaranteed that making the request
cancellable must happen before any IO_URING_F_CANCELs can occur? Is
there a race condition I'm missing where we need these to be _ONCE?
Thanks,
Joanne
>
> static void fuse_uring_flush_bg(struct fuse_ring_queue *queue)
>
> --
> 2.43.0
>
