Re: [PATCH 1/3] io_uring: fix invalid ctx->sq_thread_idle

[Stefan Metzmacher <metze@samba.org>]

Hi Pavel,

> We have to set ctx->sq_thread_idle before adding a ring to an SQ task,
> otherwise sqd races for seeing zero and accounting it as such.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  fs/io_uring.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 896a7845447c..0b39c3818809 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7827,14 +7827,14 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx,
>  
>  		ctx->sq_creds = get_current_cred();
>  		ctx->sq_data = sqd;
> -		io_sq_thread_park(sqd);
> -		list_add(&ctx->sqd_list, &sqd->ctx_new_list);
> -		io_sq_thread_unpark(sqd);
> -
>  		ctx->sq_thread_idle = msecs_to_jiffies(p->sq_thread_idle);
>  		if (!ctx->sq_thread_idle)
>  			ctx->sq_thread_idle = HZ;
>  
> +		io_sq_thread_park(sqd);
> +		list_add(&ctx->sqd_list, &sqd->ctx_new_list);
> +		io_sq_thread_unpark(sqd);

I wondered about the exact same change this morning, while researching
the IORING_SETUP_ATTACH_WQ behavior :-)

It still seems to me that IORING_SETUP_ATTACH_WQ changed over time.
As you introduced that flag, can you summaries it's behavior (and changes)
over time (over the releases).

I'm wondering if ctx->sq_creds is really the only thing we need to take care of.

Do we know about existing users of IORING_SETUP_ATTACH_WQ and their use case?
As mm, files and other things may differ now between sqe producer and the sq_thread.

metze