On 4/4/22 08:16, Christoph Hellwig wrote:
[...]
+static void io_uring_cmd_work(struct io_kiocb *req, bool *locked)
+{
+ req->uring_cmd.driver_cb(&req->uring_cmd);
+}
+
+void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
+ void (*driver_cb)(struct io_uring_cmd *))
+{
+ struct io_kiocb *req = container_of(ioucmd, struct io_kiocb, uring_cmd);
+
+ req->uring_cmd.driver_cb = driver_cb;
+ req->io_task_work.func = io_uring_cmd_work;
+ io_req_task_work_add(req, !!(req->ctx->flags & IORING_SETUP_SQPOLL));
+}
+EXPORT_SYMBOL_GPL(io_uring_cmd_complete_in_task);
I'm still not a fund of the double indirect call here. I don't really
have a good idea yet, but I plan to look into it.
I haven't familiarised myself with the series properly, but if it's about
driver_cb, we can expose struct io_kiocb and io_req_task_work_add() so
the lower layers can implement their own io_task_work.func. Hopefully, it
won't be inventively abused...
# io_uring.h
static inline struct io_uring_cmd *io_req_to_ucmd(struct io_kiocb *req)
{
return container_of();
}
typedef void (*io_tw_cb_t)(struct io_kiocb *req, bool *locked);
static inline void io_cmd_tw_add(struct io_uring_cmd *ioucmd, io_tw_cb_t foo)
{
struct io_kiocb *req = container_of(ioucmb...);
req->io_task_work.func = foo;
io_req_task_work_add();
}
static void io_req_task_queue_fail(struct io_kiocb *req, int ret)
Also it would be great to not add it between io_req_task_queue_fail and
the callback set by it.
+void io_uring_cmd_done(struct io_uring_cmd *ioucmd, ssize_t ret)
+{
+ struct io_kiocb *req = container_of(ioucmd, struct io_kiocb, uring_cmd);
+
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_complete(req, ret);
+}
+EXPORT_SYMBOL_GPL(io_uring_cmd_done);
It seems like all callers of io_req_complete actually call req_set_fail
on failure. So maybe it would be nice pre-cleanup to handle the
req_set_fail call from ĩo_req_complete?
Interpretation of the result is different, e.g. io_tee(), that was the
reason it was left in the callers.
[...]
@@ -60,7 +62,10 @@ struct io_uring_sqe {
__s32 splice_fd_in;
__u32 file_index;
};
- __u64 __pad2[2];
+ union {
+ __u64 __pad2[2];
+ __u64 cmd;
+ };
Can someone explain these changes to me a little more?
not required indeed, just
- __u64 __pad2[2];
+ __u64 cmd;
+ __u64 __pad2;
--
Pavel Begunkov
