On 6/29/20 10:32 AM, Pavel Begunkov wrote: > On 29/06/2020 18:52, Jens Axboe wrote: >> On 6/29/20 4:21 AM, Pavel Begunkov wrote: >>> On 28/06/2020 17:46, Pavel Begunkov wrote: >>>> On 28/06/2020 16:49, Jens Axboe wrote: >>>>> On 6/27/20 5:04 AM, Pavel Begunkov wrote: >>>>>> All but [3/5] are different segfault fixes for >>>>>> c40f63790ec9 ("io_uring: use task_work for links if possible") >>>>> >>>>> Looks reasonable, too bad about the task_work moving out of the >>>>> union, but I agree there's no other nice way to avoid this. BTW, >>>>> fwiw, I've moved that to the head of the series. >>>> >>>> I think I'll move it back, but that would need more work to be >>>> done. I've described the idea in the other thread. >>> >>> BTW, do you know any way to do grab_files() from task_work context? >>> The problem is that nobody sets ctx->ring_{fd,file} there. Using stale >>> values won't do, as ring_fd can be of another process at that point. >> >> We probably have to have them grabbed up-front. Which should be easy >> enough to do now, since task_work and work are no longer in a union. > > Yep, and it's how it's done. Just looking how to handle req.work better. > e.g. if we can grab_files() from task_work, then it's one step from > moving back req.work into union + totally removing memcpy(work, apoll) > from io_arm_poll_handler(). Indeed, and both of those are very worthy goals fwiw. If at all possible, it'd be nicer to get rid of the restriction of having to check ring_fd and file, but that doesn't seem possible without making the general io_ring_enter() system call more expensive. -- Jens Axboe