On 6/30/20 9:00 AM, Pavel Begunkov wrote: > 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. Yep done, I'll push it out. -- Jens Axboe
