diff mbox series

[v2,6/7] fuse: Access entries with queue lock in fuse_uring_entry_teardown

Message ID 20250125-optimize-fuse-uring-req-timeouts-v2-6-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:44 p.m. UTC 
This ensures that ent->cmd and ent->fuse_req are accessed in
fuse_uring_entry_teardown while holding the queue lock.

Fixes: a4bdb3d786c0 ("fuse: enable fuse-over-io-uring")
Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dev_uring.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

Comments

Joanne Koong Jan. 28, 2025, 12:01 a.m. UTC | #1 
On Sat, Jan 25, 2025 at 9:44 AM Bernd Schubert <bschubert@ddn.com> wrote:
>
> This ensures that ent->cmd and ent->fuse_req are accessed in
> fuse_uring_entry_teardown while holding the queue lock.
>
> Fixes: a4bdb3d786c0 ("fuse: enable fuse-over-io-uring")
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>

Reviewed-by: Joanne Koong <joannelkoong@gmail.com>

> ---
>  fs/fuse/dev_uring.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index e90dd4ae5b2133e427855f1b0e60b73f008f7bc9..9af5314f63d54cb1158e9372f4472759f5151ac3 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -298,13 +298,8 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
>         return queue;
>  }
>
> -static void fuse_uring_stop_fuse_req_end(struct fuse_ring_ent *ent)
> +static void fuse_uring_stop_fuse_req_end(struct fuse_req *req)
>  {
> -       struct fuse_req *req = ent->fuse_req;
> -
> -       /* remove entry from fuse_pqueue->processing */
> -       list_del_init(&req->list);
> -       ent->fuse_req = NULL;
>         clear_bit(FR_SENT, &req->flags);
>         req->out.h.error = -ECONNABORTED;
>         fuse_request_end(req);
> @@ -315,14 +310,20 @@ static void fuse_uring_stop_fuse_req_end(struct fuse_ring_ent *ent)
>   */
>  static void fuse_uring_entry_teardown(struct fuse_ring_ent *ent)
>  {
> -       struct fuse_ring_queue *queue = ent->queue;
> -       if (ent->cmd) {
> -               io_uring_cmd_done(ent->cmd, -ENOTCONN, 0, IO_URING_F_UNLOCKED);
> -               ent->cmd = NULL;
> -       }
> +       struct fuse_req *req;
> +       struct io_uring_cmd *cmd;
>
> -       if (ent->fuse_req)
> -               fuse_uring_stop_fuse_req_end(ent);
> +       struct fuse_ring_queue *queue = ent->queue;
> +
> +       spin_lock(&queue->lock);
> +       cmd = ent->cmd;
> +       ent->cmd = NULL;
> +       req = ent->fuse_req;
> +       ent->fuse_req = NULL;
> +       if (req) {
> +               /* remove entry from queue->fpq->processing */
> +               list_del_init(&req->list);
> +       }
>
>         /*
>          * The entry must not be freed immediately, due to access of direct
> @@ -330,10 +331,15 @@ static void fuse_uring_entry_teardown(struct fuse_ring_ent *ent)
>          * of race between daemon termination (which triggers IO_URING_F_CANCEL
>          * and accesses entries without checking the list state first
>          */
> -       spin_lock(&queue->lock);
>         list_move(&ent->list, &queue->ent_released);
>         ent->state = FRRS_RELEASED;
>         spin_unlock(&queue->lock);
> +
> +       if (cmd)
> +               io_uring_cmd_done(cmd, -ENOTCONN, 0, IO_URING_F_UNLOCKED);
> +
> +       if (req)
> +               fuse_uring_stop_fuse_req_end(req);
>  }
>
>  static void fuse_uring_stop_list_entries(struct list_head *head,
>
> --
> 2.43.0
>
diff mbox series

Patch

diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index e90dd4ae5b2133e427855f1b0e60b73f008f7bc9..9af5314f63d54cb1158e9372f4472759f5151ac3 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -298,13 +298,8 @@  static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
 	return queue;
 }
 
-static void fuse_uring_stop_fuse_req_end(struct fuse_ring_ent *ent)
+static void fuse_uring_stop_fuse_req_end(struct fuse_req *req)
 {
-	struct fuse_req *req = ent->fuse_req;
-
-	/* remove entry from fuse_pqueue->processing */
-	list_del_init(&req->list);
-	ent->fuse_req = NULL;
 	clear_bit(FR_SENT, &req->flags);
 	req->out.h.error = -ECONNABORTED;
 	fuse_request_end(req);
@@ -315,14 +310,20 @@  static void fuse_uring_stop_fuse_req_end(struct fuse_ring_ent *ent)
  */
 static void fuse_uring_entry_teardown(struct fuse_ring_ent *ent)
 {
-	struct fuse_ring_queue *queue = ent->queue;
-	if (ent->cmd) {
-		io_uring_cmd_done(ent->cmd, -ENOTCONN, 0, IO_URING_F_UNLOCKED);
-		ent->cmd = NULL;
-	}
+	struct fuse_req *req;
+	struct io_uring_cmd *cmd;
 
-	if (ent->fuse_req)
-		fuse_uring_stop_fuse_req_end(ent);
+	struct fuse_ring_queue *queue = ent->queue;
+
+	spin_lock(&queue->lock);
+	cmd = ent->cmd;
+	ent->cmd = NULL;
+	req = ent->fuse_req;
+	ent->fuse_req = NULL;
+	if (req) {
+		/* remove entry from queue->fpq->processing */
+		list_del_init(&req->list);
+	}
 
 	/*
 	 * The entry must not be freed immediately, due to access of direct
@@ -330,10 +331,15 @@  static void fuse_uring_entry_teardown(struct fuse_ring_ent *ent)
 	 * of race between daemon termination (which triggers IO_URING_F_CANCEL
 	 * and accesses entries without checking the list state first
 	 */
-	spin_lock(&queue->lock);
 	list_move(&ent->list, &queue->ent_released);
 	ent->state = FRRS_RELEASED;
 	spin_unlock(&queue->lock);
+
+	if (cmd)
+		io_uring_cmd_done(cmd, -ENOTCONN, 0, IO_URING_F_UNLOCKED);
+
+	if (req)
+		fuse_uring_stop_fuse_req_end(req);
 }
 
 static void fuse_uring_stop_list_entries(struct list_head *head,