diff mbox series

[v2,4/7] fuse: Use READ_ONCE in fuse_uring_send_in_task

Message ID 20250125-optimize-fuse-uring-req-timeouts-v2-4-7771a2300343@ddn.com (mailing list archive)
State New
Headers show
Series fuse: {io-uring} Ensure fuse requests are set/read with locks | expand

Commit Message

Bernd Schubert Jan. 25, 2025, 5:43 p.m. UTC 
The value is read from another task without, while the task that
had set the value was holding queue->lock. Better use READ_ONCE
to ensure the compiler cannot optimize the read.

Fixes: 284985711dc5 ("fuse: Allow to queue fg requests through io-uring")
Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dev_uring.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Miklos Szeredi Jan. 27, 2025, 1:16 p.m. UTC | #1 
On Sat, 25 Jan 2025 at 18:45, Bernd Schubert <bschubert@ddn.com> wrote:
>
> The value is read from another task without, while the task that
> had set the value was holding queue->lock. Better use READ_ONCE
> to ensure the compiler cannot optimize the read.

I do not think this is necessary.  The ordering should be ensured by
the io_uring infrastructure, no?

Thanks,
Miklos
Bernd Schubert Jan. 27, 2025, 1:41 p.m. UTC | #2 
On 1/27/25 14:16, Miklos Szeredi wrote:
> On Sat, 25 Jan 2025 at 18:45, Bernd Schubert <bschubert@ddn.com> wrote:
>>
>> The value is read from another task without, while the task that
>> had set the value was holding queue->lock. Better use READ_ONCE
>> to ensure the compiler cannot optimize the read.
> 
> I do not think this is necessary.  The ordering should be ensured by
> the io_uring infrastructure, no?

Yes, definitely ordered, as it is called only when ent->fuse_req was set
before. So yeah, you are right, we can skip this patch.


Thanks,
Bernd
diff mbox series

Patch

diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 62a063fda3951d29c27f95c1941a06f38f7b8248..80bb7396a8410022bbef1efa0522974bda77c81a 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -1204,10 +1204,12 @@  static void fuse_uring_send_in_task(struct io_uring_cmd *cmd,
 {
 	struct fuse_ring_ent *ent = uring_cmd_to_ring_ent(cmd);
 	struct fuse_ring_queue *queue = ent->queue;
+	struct fuse_req *req;
 	int err;
 
 	if (!(issue_flags & IO_URING_F_TASK_DEAD)) {
-		err = fuse_uring_prepare_send(ent, ent->fuse_req);
+		req = READ_ONCE(ent->fuse_req);
+		err = fuse_uring_prepare_send(ent, req);
 		if (err) {
 			fuse_uring_next_fuse_req(ent, queue, issue_flags);
 			return;