[PATCH] io_uring/bpf-ops: wake up the loop task on eject

[PATCH] io_uring/bpf-ops: wake up the loop task on eject

[Sidong Yang]

io_eject_bpf() clears ctx->loop_step while a loop may be sleeping in
io_loop_wait(), which releases ->uring_lock before schedule(). Nothing
wakes the submitter task after the BPF ops are unregistered through link
destruction or ring teardown, so the task stays blocked in
io_uring_enter() until an unrelated CQE event or signal arrives.

Since BPF ops require IORING_SETUP_DEFER_TASKRUN, only
ctx->submitter_task can run the loop, so wake it directly. The loop
rechecks loop_step after waking up and exits with -EFAULT. A spurious
wakeup is harmless because io_loop_wait() rechecks the wait condition
before sleeping again.

Fixes: 98f37634b12b ("io_uring/bpf-ops: implement bpf ops registration")
Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
 io_uring/bpf-ops.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/io_uring/bpf-ops.c b/io_uring/bpf-ops.c
index 5a50f0675fe5..87148dbd3b3e 100644
--- a/io_uring/bpf-ops.c
+++ b/io_uring/bpf-ops.c
@@ -210,6 +210,13 @@ static void io_eject_bpf(struct io_ring_ctx *ctx)
 	ops->priv = NULL;
 	ctx->bpf_ops = NULL;
 	ctx->loop_step = NULL;
+	/*
+	 * A loop may be sleeping in io_loop_wait() with ->uring_lock
+	 * released. It'll see loop_step == NULL after waking up, but
+	 * nothing wakes it otherwise.
+	 */
+	if (ctx->submitter_task)
+		wake_up_state(ctx->submitter_task, TASK_INTERRUPTIBLE);
 }
 
 static void bpf_io_unreg(void *kdata, struct bpf_link *link)
-- 
2.53.0

Re: [PATCH] io_uring/bpf-ops: wake up the loop task on eject

[Gabriel Krisman Bertazi]

Sidong Yang <sidong.yang@furiosa.ai> writes:

> io_eject_bpf() clears ctx->loop_step while a loop may be sleeping in
> io_loop_wait(), which releases ->uring_lock before schedule(). Nothing
> wakes the submitter task after the BPF ops are unregistered through link
> destruction or ring teardown, so the task stays blocked in
> io_uring_enter() until an unrelated CQE event or signal arrives.
>
> Since BPF ops require IORING_SETUP_DEFER_TASKRUN, only
> ctx->submitter_task can run the loop, so wake it directly. The loop
> rechecks loop_step after waking up and exits with -EFAULT. A spurious
> wakeup is harmless because io_loop_wait() rechecks the wait condition
> before sleeping again.
>
> Fixes: 98f37634b12b ("io_uring/bpf-ops: implement bpf ops registration")
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>

Makes sense, feel free to add:

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

> ---
>  io_uring/bpf-ops.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/io_uring/bpf-ops.c b/io_uring/bpf-ops.c
> index 5a50f0675fe5..87148dbd3b3e 100644
> --- a/io_uring/bpf-ops.c
> +++ b/io_uring/bpf-ops.c
> @@ -210,6 +210,13 @@ static void io_eject_bpf(struct io_ring_ctx *ctx)
>  	ops->priv = NULL;
>  	ctx->bpf_ops = NULL;
>  	ctx->loop_step = NULL;
> +	/*
> +	 * A loop may be sleeping in io_loop_wait() with ->uring_lock
> +	 * released. It'll see loop_step == NULL after waking up, but
> +	 * nothing wakes it otherwise.
> +	 */
> +	if (ctx->submitter_task)
> +		wake_up_state(ctx->submitter_task, TASK_INTERRUPTIBLE);
>  }
>  
>  static void bpf_io_unreg(void *kdata, struct bpf_link *link)
> -- 
> 2.53.0
>

-- 
Gabriel Krisman Bertazi