Hi,
On 24.04.19 18:09, Jens Axboe wrote:
> On 4/23/19 4:07 PM, Jens Axboe wrote:
>>>> (Also RWF_NOWAIT doesn't work in io_uring right now: IOCB_NOWAIT is
>>>> always removed in the workqueue context, and I don't see an early EAGAIN
>>>> completion).
>>>
>>> That's a case I didn't consider, that you'd want to see EAGAIN after
>>> it's been punted. Once punted, we're not going to return EAGAIN since
>>> we can now block. Not sure how you'd want to handle that any better...
>>
>> I think I grok this one too now - what you're saying is that if the
>> caller has RWF_NOWAIT set, then the EAGAIN should be returned instead of
>> being punted to the workqueue? I totally agree with that, that's a bug.
>
> This should do it for the EAGAIN part, if the user has set RWF_NOWAIT
> in the sqe, then we don't do the automatic punt to workqueue. We just
> return the EAGAIN instead.
>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 58ec6e449fd8..6c0d49c3736b 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -822,7 +822,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct sqe_submit *s,
> ret = kiocb_set_rw_flags(kiocb, READ_ONCE(sqe->rw_flags));
> if (unlikely(ret))
> return ret;
> - if (force_nonblock) {
> + if (force_nonblock && !(kiocb->ki_flags & IOCB_NOWAIT)) {
> kiocb->ki_flags |= IOCB_NOWAIT;
> req->flags |= REQ_F_FORCE_NONBLOCK;
> }
> @@ -1828,7 +1828,7 @@ static int io_submit_sqe(struct io_ring_ctx *ctx, struct sqe_submit *s,
> }
>
> ret = __io_submit_sqe(ctx, req, s, true);
> - if (ret == -EAGAIN) {
> + if (ret == -EAGAIN && (req->flags & REQ_F_FORCE_NONBLOCK)) {
> struct io_uring_sqe *sqe_copy;
>
> sqe_copy = kmalloc(sizeof(*sqe_copy), GFP_KERNEL);
>
I think this breaks other request types.
Only io_prep_rw ever sets REQ_F_FORCE_NONBLOCK, but e.g. IORING_OP_FSYNC
always wants to punt.
Given that REQ_F_FORCE_NONBLOCK wasn't actually used before (never read
afaict), maybe remove it and insert a new flag REQ_F_NOWAIT that will
prevent the punt (inverted semantics!), e.g:
> - if (ret == -EAGAIN) {
> + if (ret == -EAGAIN && !(req->flags & REQ_F_NOWAIT)) {
And set this flag in io_prep_rw if RWF_NOWAIT is set.
cheers,
Stefan
