Re: [PATCH] io_uring/napi: remove duplicate io_napi_entry timeout assignation

[Olivier Langlois <olivier@trillion01.com>]

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...