On 5/17/23 11:33, Kanchan Joshi wrote:
On Tue, May 16, 2023 at 07:52:23PM +0100, Pavel Begunkov wrote:
On 5/16/23 11:00, Kanchan Joshi wrote:
On Mon, May 15, 2023 at 01:54:42PM +0100, Pavel Begunkov wrote:
We want to use IOU_F_TWQ_LAZY_WAKE in commands. First, introduce a new
cmd tw helper accepting TWQ flags, and then add
io_uring_cmd_do_in_task_laz() that will pass IOU_F_TWQ_LAZY_WAKE and
imply the "lazy" semantics, i.e. it posts no more than 1 CQE and
delaying execution of this tw should not prevent forward progress.
Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
---
[...]
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 5e32db48696d..476c7877ce58 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -20,16 +20,24 @@ static void io_uring_cmd_work(struct io_kiocb *req, struct io_tw_state *ts)
ioucmd->task_work_cb(ioucmd, issue_flags);
}
-void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
- void (*task_work_cb)(struct io_uring_cmd *, unsigned))
+void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
+ void (*task_work_cb)(struct io_uring_cmd *, unsigned),
+ unsigned flags)
{
struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
ioucmd->task_work_cb = task_work_cb;
req->io_task_work.func = io_uring_cmd_work;
- io_req_task_work_add(req);
+ __io_req_task_work_add(req, flags);
+}
+EXPORT_SYMBOL_GPL(__io_uring_cmd_do_in_task);
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
+static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
+ void (*task_work_cb)(struct io_uring_cmd *, unsigned))
+{
+ __io_uring_cmd_do_in_task(ioucmd, task_work_cb, 0);
+}
That should fail for nvme unless exported.
But it does not. Give it a try.
Took the first patch, killed the export and compiled ublk
and nvme as modules:
ERROR: modpost: "__io_uring_cmd_do_in_task" [drivers/block/ublk_drv.ko] undefined!
ERROR: modpost: "__io_uring_cmd_do_in_task" [drivers/nvme/host/nvme-core.ko] undefined!
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 476c7877ce58..3bb43122a683 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -30,7 +30,6 @@ void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
req->io_task_work.func = io_uring_cmd_work;
__io_req_task_work_add(req, flags);
}
-EXPORT_SYMBOL_GPL(__io_uring_cmd_do_in_task);
Any reason to export this? No one is using this at the moment.
+void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
+ void (*task_work_cb)(struct io_uring_cmd *, unsigned))
+{
+ __io_uring_cmd_do_in_task(ioucmd, task_work_cb, IOU_F_TWQ_LAZY_WAKE);
}
-EXPORT_SYMBOL_GPL(io_uring_cmd_complete_in_task);
+EXPORT_SYMBOL_GPL(io_uring_cmd_do_in_task_lazy);
Seems you did not want callers to pass the the new flag (LAZY_WAKE) and
therefore this helper.
Yep, I wouldn't mind exposing just *LAZY_WAKE but don't want
to let it use whatever flags there might be in the future.
Initially I wanted to just make io_uring_cmd_complete_in_task and
io_uring_cmd_do_in_task_lazy static inline, but that would need
some code shuffling to make it clean.
And if you did not want callers to know about this flag (internal
details of io_uring), it would be better to have two exported helpers
io_uring_cmd_do_in_task_lazy() and io_uring_cmd_complete_in_task().
Both will use the internal helper __io_uring_cmd_do_in_task with
different flag.
That's how it should be in this patch
Nah, in this patch __io_uring_cmd_do_in_task is exported helper. And
io_uring_cmd_complete_in_task has been changed too (explicit export to
header based one). Seems like bit more shuffling than what is necessary.
With the end goal to turn them into inline helpers later on
I think it's fine.
--
Pavel Begunkov
