On Mon, 2024-08-12 at 14:40 -0600, Jens Axboe wrote:
>
> @@ -174,9 +174,8 @@ static void io_napi_blocking_busy_loop(struct
> io_ring_ctx *ctx,
> do {
> is_stale = __io_napi_do_busy_loop(ctx,
> loop_end_arg);
> } while (!io_napi_busy_loop_should_end(iowq, start_time) &&
> !loop_end_arg);
> - rcu_read_unlock();
> -
> io_napi_remove_stale(ctx, is_stale);
> + rcu_read_unlock();
> }
>
> @@ -309,9 +309,8 @@ int io_napi_sqpoll_busy_poll(struct io_ring_ctx
> *ctx)
>
> rcu_read_lock();
> is_stale = __io_napi_do_busy_loop(ctx, NULL);
> - rcu_read_unlock();
> -
> io_napi_remove_stale(ctx, is_stale);
> + rcu_read_unlock();
> return 1;
> }
>
>
Jens,
I have big doubts that moving the rcu_read_unlock() call is correct.
The read-only list access if performed by the busy loops block.
io_napi_remove_stale() is then modifying the list after having acquired
the spinlock. IMHO, you should not hold the RCU read lock when you are
updating the data. I even wonder is this could not be a possible
livelock cause...
