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@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Hao Xu <haoxu@xxxxxxxxxxxxxxxxx>
> ---
> 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
