Re: [PATCH 0/5] "task_work for links" fixes

[Jens Axboe <axboe@kernel.dk>]

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