Re: [PATCH v2] io_uring: check kthread parked flag before sqthread goes to sleep

[Pavel Begunkov <asml.silence@gmail.com>]

On 29/01/2021 17:53, Hao Xu wrote:
> 
> So check if sqthread gets park flag right before schedule().
> since ctx_list is always empty when this problem happens, here I put
> kthread_should_park() before setting the wakeup flag(ctx_list is empty
> so this for loop is fast), where is close enough to schedule(). The
> problem doesn't show again in my repro testing after this fix.

Looks good, and I believe I saw syzbot reporting similar thing before.
Two nits below

> 
> Reported-by: Abaci <abaci@linux.alibaba.com>
> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
> ---
>  fs/io_uring.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index c07913ec0cca..444dc993157e 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7132,6 +7132,9 @@ static int io_sq_thread(void *data)
>  			}
>  		}

How about killing btw a kthread_should_park() check few lines
above before prepare_to_wait? Parking is fairly rare, so we
don't need fast path for it.

>  
> +		if (kthread_should_park())
> +			needs_sched = false;
> +
>  		if (needs_sched) {

if (needs_sched && !kthread_should_park())

Looks cleaner to me

>  			list_for_each_entry(ctx, &sqd->ctx_list, sqd_list)
>  				io_ring_set_wakeup_flag(ctx);
> 

-- 
Pavel Begunkov