Re: io_uring: submission error handling

[Jens Axboe <axboe@kernel.dk>]

On 4/27/19 9:50 AM, Stefan Bühler wrote:
> Hi,
> 
> On 24.04.19 00:07, Jens Axboe wrote:
>> On 4/23/19 2:31 PM, Jens Axboe wrote:
>>>> 1. An error for a submission should be returned as completion for that
>>>> submission.  Please don't break my main event loop with strange error
>>>> codes just because a single operation is broken/not supported/...
>>>
>>> So that's the case I was referring to above. We can just make that change,
>>> there's absolutely no reason to have errors passed back through a different
>>> channel.
>>
>> Thinking about this a bit more, and I think the current approach is the
>> best one. The issue is that only submission side events tied to an sqe
>> can return an cqe, the rest have to be returned through the system call
>> value. So I think it's cleaner to keep it as-is, honestly.
> 
> Not sure we're talking about the same.
> 
> I'm talking about the errors returned by io_submit_sqe: io_submit_sqes
> (called by the SQ thread) calls io_cqring_add_event if there was an
> error, but io_ring_submit (called by io_uring_enter) doesn't: instead,
> if there were successfully submitted entries before, it will just return
> those (and "undo" the current SQE), otherwise it will return the error,
> which will then be returned by io_uring_enter.
> 
> But if I get an error from io_uring_enter I have no idea whether it was
> some generic error (say EINVAL for broken flags or EBADF for a
> non-io-uring filedescriptor) or an error related to a single submission.
> 
> I think io_ring_submit should call io_cqring_add_event on errors too
> (like io_submit_sqes), and not stop handling submissions (and never
> return an error).
> 
> Maybe io_cqring_add_event could then even be moved to io_submit_sqe and
> just return whether the job is already done or not (io_submit_sqes
> returns the new "inflight" jobs, and io_ring_submit the total number of
> submitted jobs).

I think we are talking about the same thing, actually. See below patch.
This changes it so that any error that occurs on behalf of a specific
sqe WILL trigger a completion event, instead of returning it through
io_uring_enter(). io_uring_enter() can still return -ERROR for errors
that aren't specific to an sqe.

I think this is what you had in mind?

Totally untested, will do so now.

-- 
Jens Axboe