Re: [PATCH for-next v2 11/12] io_uring: do msg_ring in target task via tw

[Jens Axboe <axboe@kernel.dk>]

On 12/7/22 2:18 PM, Pavel Begunkov wrote:
> On 12/7/22 15:51, Jens Axboe wrote:
>> On 12/7/22 8:31 AM, Jens Axboe wrote:
>>> On 12/6/22 8:53?PM, Pavel Begunkov wrote:
>>>> @@ -43,6 +61,15 @@ static int io_msg_ring_data(struct io_kiocb *req)
>>>>       if (msg->src_fd || msg->dst_fd || msg->flags)
>>>>           return -EINVAL;
>>>>   +    if (target_ctx->task_complete && current != target_ctx->submitter_task) {
>>>> +        init_task_work(&msg->tw, io_msg_tw_complete);
>>>> +        if (task_work_add(target_ctx->submitter_task, &msg->tw,
>>>> +                  TWA_SIGNAL))
>>>> +            return -EOWNERDEAD;
>>>> +
>>>> +        return IOU_ISSUE_SKIP_COMPLETE;
>>>> +    }
>>>> +
>>>
>>> We should probably be able to get by with TWA_SIGNAL_NO_IPI here, no?
>>
>> Considering we didn't even wake before, I'd say that's a solid yes.
> 
> I'm not so sure. It'll work, but a naive approach would also lack
> IORING_SETUP_TASKRUN_FLAG and so mess with latencies when it's not
> desirable.
> 
> option 1)
> 
> method = TWA_SIGNAL;
> if (flags & IORING_SETUP_TASKRUN_FLAG)
>     method = NO_IPI;
> 
> option 2)
> 
> task_work_add(NO_IPI);
> atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);
> 
> 
> Might be better to have one of those.

I like option 2, which should be fine.

-- 
Jens Axboe