On 3/16/24 23:58, Jens Axboe wrote:
On 3/16/24 11:42 AM, Pavel Begunkov wrote:
On 3/16/24 17:01, Jens Axboe wrote:
On 3/16/24 10:57 AM, Pavel Begunkov wrote:
On 3/16/24 16:51, Jens Axboe wrote:
On 3/16/24 10:46 AM, Pavel Begunkov wrote:
On 3/16/24 16:42, Jens Axboe wrote:
On 3/16/24 10:36 AM, Pavel Begunkov wrote:
On 3/16/24 16:36, Jens Axboe wrote:
...
It should only set REQ_F_EARLY_FAIL if we fail
_before_ prep is called
I did try both ways, fails if we just have:
Ok, but the point is that the sendzc's ->fail doesn't
need to be called unless you've done ->prep first.
But it fails, not sure how else to say it.
liburing tests? Which test case? If so, it should be another
Like I mentioned earlier, it's send zc and it's failing the test case
for that. test/send-zerocopy.t.
bug. REQ_F_NEED_CLEANUP is only set by opcodes, if a request is
terminated before ->prep is called, it means it never entered
any of the opdef callbacks and have never seen any of net.c
code, so there should be no REQ_F_NEED_CLEANUP, and so
io_sendrecv_fail() wouldn't try to set F_MORE. I don't know
what's wrong.
Feel free to take a look! I do like the simplicity of the early error
flag.
./send-zerocopy.t works fine
Huh, I wonder what I messed up. But:
My blind guess would be it called ->prep(), which is assumingly
failed, but then there was no ->fail() following it. BTW, we might
want to loose up that case, similar to mshots sendzc might decide
not to post notifications for any reason and the user should always
check F_MORE first.
@@ -2250,7 +2249,13 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
int ret;
ret = io_init_req(ctx, req, sqe);
- if (unlikely(ret))
+ if (unlikely(ret)) {
+ req->flags |= REQ_F_UNPREPPED_FAIL;
+ return io_submit_fail_init(sqe, req, ret);
+ }
+
+ ret = def->prep(req, sqe);
+ if (ret)
return io_submit_fail_init(sqe, req, ret);
this obviously won't compile, assuming this is not the one you ran.
Urgh, yeah, some left unstaged
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index ea7e5488b3be..de3a2c67c4a7 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -478,6 +478,7 @@ enum {
REQ_F_CAN_POLL_BIT,
REQ_F_BL_EMPTY_BIT,
REQ_F_BL_NO_RECYCLE_BIT,
+ REQ_F_UNPREPPED_FAIL_BIT,
/* not a real bit, just to check we're not overflowing the space */
__REQ_F_LAST_BIT,
@@ -556,6 +557,8 @@ enum {
REQ_F_BL_EMPTY = IO_REQ_FLAG(REQ_F_BL_EMPTY_BIT),
/* don't recycle provided buffers for this request */
REQ_F_BL_NO_RECYCLE = IO_REQ_FLAG(REQ_F_BL_NO_RECYCLE_BIT),
+
+ REQ_F_UNPREPPED_FAIL = IO_REQ_FLAG(REQ_F_UNPREPPED_FAIL_BIT),
};
typedef void (*io_req_tw_func_t)(struct io_kiocb *req, struct io_tw_state *ts);
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 846d67a9c72e..1231f8c53014 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -993,7 +993,7 @@ void io_req_defer_failed(struct io_kiocb *req, s32 res)
req_set_fail(req);
io_req_set_res(req, res, io_put_kbuf(req, IO_URING_F_UNLOCKED));
- if (def->fail)
+ if (!(req->flags & REQ_F_UNPREPPED_FAIL) && def->fail)
def->fail(req);
io_req_complete_defer(req);
}
@@ -2201,8 +2201,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
}
req->flags |= REQ_F_CREDS;
}
-
- return def->prep(req, sqe);
+ return 0;
}
static __cold int io_submit_fail_init(const struct io_uring_sqe *sqe,
@@ -2250,7 +2249,13 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
int ret;
ret = io_init_req(ctx, req, sqe);
- if (unlikely(ret))
+ if (unlikely(ret)) {
+ req->flags |= REQ_F_UNPREPPED_FAIL;
+ return io_submit_fail_init(sqe, req, ret);
+ }
+
+ ret = io_issue_defs[req->opcode].prep(req, sqe);
+ if (ret)
return io_submit_fail_init(sqe, req, ret);
trace_io_uring_submit_req(req);
--
Pavel Begunkov
