On 30/06/2020 17:46, Jens Axboe wrote: > On 6/30/20 8:36 AM, Pavel Begunkov wrote: >> On 30/06/2020 17:04, Jens Axboe wrote: >>> On 6/30/20 6:20 AM, Pavel Begunkov wrote: >>>> Don't call io_commit_cqring() without holding the completion spinlock >>>> in io_iopoll_complete(), it can race, e.g. with async request failing. >>> >>> Can you be more specific? >> >> io_iopoll_complete() >> -> io_req_free_batch() >> -> io_queue_next() >> -> io_req_task_queue() >> -> task_work_add() >> >> if this task_work_add() fails, it will be redirected to io-wq manager task >> to do io_req_task_cancel() -> commit_cqring(). >> >> >> And probably something similar will happen if a request currently in io-wq >> is retried with >> io_rw_should_retry() -> io_async_buf_func() -> task_work_add() > > For the IOPOLL setup, it should be protected by the ring mutex. Adding > the irq disable + lock for polling would be a nasty performance hit. Ok. For what it worth, it would be easier to accidentally screw in the future. I'll try it out. BTW, if you're going to cherry-pick patches from this series, just push them into the branch. I'll check git.kernel.dk later and resend what's left. -- Pavel Begunkov
