On 8/30/24 14:33, Jens Axboe wrote:
On 8/30/24 7:28 AM, Bernd Schubert wrote:
On 8/30/24 15:12, Jens Axboe wrote:
On 8/29/24 4:32 PM, Bernd Schubert wrote:
We probably need to call iov_iter_get_pages2() immediately
on submitting the buffer from fuse server and not only when needed.
I had planned to do that as optimization later on, I think
it is also needed to avoid io_uring_cmd_complete_in_task().
I think you do, but it's not really what's wrong here - fallback work is
being invoked as the ring is being torn down, either directly or because
the task is exiting. Your task_work should check if this is the case,
and just do -ECANCELED for this case rather than attempt to execute the
work. Most task_work doesn't do much outside of post a completion, but
yours seems complex in that attempts to map pages as well, for example.
In any case, regardless of whether you move the gup to the actual issue
side of things (which I think you should), then you'd want something
ala:
if (req->task != current)
don't issue, -ECANCELED
in your task_work.nvme_uring_task_cb
Thanks a lot for your help Jens! I'm a bit confused, doesn't this belong
into __io_uring_cmd_do_in_task then? Because my task_work_cb function
(passed to io_uring_cmd_complete_in_task) doesn't even have the request.
Yeah it probably does, the uring_cmd case is a bit special is that it's
a set of helpers around task_work that can be consumed by eg fuse and
ublk. The existing users don't really do anything complicated on that
side, hence there's no real need to check. But since the ring/task is
going away, we should be able to generically do it in the helpers like
you did below.
That won't work, we should give commands an opportunity to clean up
after themselves. I'm pretty sure it will break existing users.
For now we can pass a flag to the callback, fuse would need to
check it and fail. Compile tested only
commit a5b382f150b44476ccfa84cefdb22ce2ceeb12f1
Author: Pavel Begunkov <asml.silence@xxxxxxxxx>
Date: Fri Aug 30 15:43:32 2024 +0100
io_uring/cmd: let cmds tw know about dying task
When the taks that submitted a request is dying, a task work for that
request might get run by a kernel thread or even worse by a half
dismantled task. We can't just cancel the task work without running the
callback as the cmd might need to do some clean up, so pass a flag
instead. If set, it's not safe to access any task resources and the
callback is expected to cancel the cmd ASAP.
Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index ace7ac056d51..a89abec98832 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -37,6 +37,7 @@ enum io_uring_cmd_flags {
/* set when uring wants to cancel a previously issued command */
IO_URING_F_CANCEL = (1 << 11),
IO_URING_F_COMPAT = (1 << 12),
+ IO_URING_F_TASK_DEAD = (1 << 13),
};
struct io_zcrx_ifq;
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 8391c7c7c1ec..55bdcb4b63b3 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -119,9 +119,13 @@ EXPORT_SYMBOL_GPL(io_uring_cmd_mark_cancelable);
static void io_uring_cmd_work(struct io_kiocb *req, struct io_tw_state *ts)
{
struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
+ unsigned flags = IO_URING_F_COMPLETE_DEFER;
+
+ if (req->task->flags & PF_EXITING)
+ flags |= IO_URING_F_TASK_DEAD;
/* task_work executor checks the deffered list completion */
- ioucmd->task_work_cb(ioucmd, IO_URING_F_COMPLETE_DEFER);
+ ioucmd->task_work_cb(ioucmd, flags);
}
void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
--
Pavel Begunkov
