Re: [PATCH for-next v3 4/7] io_uring: add IORING_SETUP_DEFER_TASKRUN

[Hao Xu <hao.xu@linux.dev>]

On 8/30/22 15:23, Dylan Yudaken wrote:
> On Mon, 2022-08-29 at 14:32 +0800, Hao Xu wrote:
>>>> @@ -3289,17 +3409,29 @@ static __cold int
>>>> io_uring_create(unsigned
>>>> entries, struct io_uring_params *p,
>>>>        if (ctx->flags & IORING_SETUP_SQPOLL) {
>>>>            /* IPI related flags don't make sense with SQPOLL */
>>>>            if (ctx->flags & (IORING_SETUP_COOP_TASKRUN |
>>>> -                  IORING_SETUP_TASKRUN_FLAG))
>>>> +                  IORING_SETUP_TASKRUN_FLAG |
>>>> +                  IORING_SETUP_DEFER_TASKRUN))
>>>
>>> Sounds like we should also fail if SQPOLL is set, especially with
>>> the task check on the waiting side.
>>
>> sqpoll as a natural single issuer case, shouldn't we support this
>> feature for it? And surely, in that case, don't do local task work
>> check
>> in cqring wait time and be careful in other places like
>> io_uring_register
> 
> I think there is definitely scope for that - but it's less obvious how
> to do it.
> i.e. in it's current form DEFER_TASKRUN requires the GETEVENTS to be
> submitted on the same task as the initial submission, but with SQPOLL
> the submission task is a kernel thread so would have to have some
> difference in the API.

Yea, just like what I said, in sqpoll mode, we shouldn't do the tw
handle in the io_uring_enter.

> 
> As an idea for a later patch set - perhaps the semantics should be to
> keep task work local but only run it once submissions have been
> processed for a ctx. I suspect that will require some care to ensure
> the wakeup flag is set correctly and that it cleans up properly.
> 

Yea, it should be a separate follow-up patchset, currently there is a
io_run_task_work() after submitted sqes for all ctxes and before going
to sleep, that may be a good place for it. I haven't think about it in
detail, but there should be a viable way.

> Dylan
>