From: Caleb Sander Mateos <[email protected]>
To: Jens Axboe <[email protected]>,
Pavel Begunkov <[email protected]>,
Ming Lei <[email protected]>, Keith Busch <[email protected]>,
Christoph Hellwig <[email protected]>, Sagi Grimberg <[email protected]>
Cc: Xinyu Zhang <[email protected]>,
[email protected], [email protected],
[email protected],
Caleb Sander Mateos <[email protected]>
Subject: [PATCH 3/3] io_uring/uring_cmd: import fixed buffer before going async
Date: Fri, 21 Mar 2025 12:48:19 -0600 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
For uring_cmd operations with fixed buffers, the fixed buffer lookup
happens in io_uring_cmd_import_fixed(), called from the ->uring_cmd()
implementation. A ->uring_cmd() implementation could return -EAGAIN on
the initial issue for any reason before io_uring_cmd_import_fixed().
For example, nvme_uring_cmd_io() calls nvme_alloc_user_request() first,
which can return -EAGAIN if all tags in the tag set are in use.
This ordering difference is observable when using
UBLK_U_IO_{,UN}REGISTER_IO_BUF SQEs to modify the fixed buffer table.
If the uring_cmd is followed by a UBLK_U_IO_UNREGISTER_IO_BUF operation
that unregisters the fixed buffer, the uring_cmd going async will cause
the fixed buffer lookup to fail because it happens after the unregister.
Move the fixed buffer lookup out of io_uring_cmd_import_fixed() and
instead perform it in io_uring_cmd() before calling ->uring_cmd().
io_uring_cmd_import_fixed() now only initializes an iov_iter from the
existing fixed buffer node. This division of responsibilities makes
sense as the fixed buffer lookup is an io_uring implementation detail
and independent of the ->uring_cmd() implementation. It also cuts down
on the need to pass around the io_uring issue_flags.
Signed-off-by: Caleb Sander Mateos <[email protected]>
Fixes: 27cb27b6d5ea ("io_uring: add support for kernel registered bvecs")
---
drivers/nvme/host/ioctl.c | 10 ++++------
include/linux/io_uring/cmd.h | 6 ++----
io_uring/rsrc.c | 6 ++++++
io_uring/rsrc.h | 2 ++
io_uring/uring_cmd.c | 10 +++++++---
5 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index fe9fb80c6a14..3fad74563b9e 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -112,12 +112,11 @@ static struct request *nvme_alloc_user_request(struct request_queue *q,
return req;
}
static int nvme_map_user_request(struct request *req, u64 ubuffer,
unsigned bufflen, void __user *meta_buffer, unsigned meta_len,
- struct io_uring_cmd *ioucmd, unsigned int flags,
- unsigned int iou_issue_flags)
+ struct io_uring_cmd *ioucmd, unsigned int flags)
{
struct request_queue *q = req->q;
struct nvme_ns *ns = q->queuedata;
struct block_device *bdev = ns ? ns->disk->part0 : NULL;
bool supports_metadata = bdev && blk_get_integrity(bdev->bd_disk);
@@ -141,12 +140,11 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
/* fixedbufs is only for non-vectored io */
if (WARN_ON_ONCE(flags & NVME_IOCTL_VEC))
return -EINVAL;
ret = io_uring_cmd_import_fixed(ubuffer, bufflen,
- rq_data_dir(req), &iter, ioucmd,
- iou_issue_flags);
+ rq_data_dir(req), &iter, ioucmd);
if (ret < 0)
goto out;
ret = blk_rq_map_user_iov(q, req, NULL, &iter, GFP_KERNEL);
} else {
ret = blk_rq_map_user_io(req, NULL, nvme_to_user_ptr(ubuffer),
@@ -194,11 +192,11 @@ static int nvme_submit_user_cmd(struct request_queue *q,
return PTR_ERR(req);
req->timeout = timeout;
if (ubuffer && bufflen) {
ret = nvme_map_user_request(req, ubuffer, bufflen, meta_buffer,
- meta_len, NULL, flags, 0);
+ meta_len, NULL, flags);
if (ret)
return ret;
}
bio = req->bio;
@@ -514,11 +512,11 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
req->timeout = d.timeout_ms ? msecs_to_jiffies(d.timeout_ms) : 0;
if (d.data_len) {
ret = nvme_map_user_request(req, d.addr,
d.data_len, nvme_to_user_ptr(d.metadata),
- d.metadata_len, ioucmd, vec, issue_flags);
+ d.metadata_len, ioucmd, vec);
if (ret)
return ret;
}
/* to free bio on completion, as req->bio will be null at that time */
diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
index 598cacda4aa3..ea243bfab2a8 100644
--- a/include/linux/io_uring/cmd.h
+++ b/include/linux/io_uring/cmd.h
@@ -39,12 +39,11 @@ static inline void io_uring_cmd_private_sz_check(size_t cmd_sz)
)
#if defined(CONFIG_IO_URING)
int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
struct iov_iter *iter,
- struct io_uring_cmd *ioucmd,
- unsigned int issue_flags);
+ struct io_uring_cmd *ioucmd);
/*
* Completes the request, i.e. posts an io_uring CQE and deallocates @ioucmd
* and the corresponding io_uring request.
*
@@ -69,12 +68,11 @@ void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd);
#else
static inline int
io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
- struct iov_iter *iter, struct io_uring_cmd *ioucmd,
- unsigned int issue_flags)
+ struct iov_iter *iter, struct io_uring_cmd *ioucmd)
{
return -EOPNOTSUPP;
}
static inline void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret,
u64 ret2, unsigned issue_flags)
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 5fff6ba2b7c0..ad0dfe51acb1 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -1099,10 +1099,16 @@ int io_import_reg_buf(struct io_kiocb *req, struct iov_iter *iter,
if (!node)
return -EFAULT;
return io_import_fixed(ddir, iter, node->buf, buf_addr, len);
}
+int io_import_buf_node(struct io_kiocb *req, struct iov_iter *iter,
+ u64 buf_addr, size_t len, int ddir)
+{
+ return io_import_fixed(ddir, iter, req->buf_node->buf, buf_addr, len);
+}
+
/* Lock two rings at once. The rings must be different! */
static void lock_two_rings(struct io_ring_ctx *ctx1, struct io_ring_ctx *ctx2)
{
if (ctx1 > ctx2)
swap(ctx1, ctx2);
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index f10a1252b3e9..bc0f8f0a2054 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -59,10 +59,12 @@ int io_rsrc_data_alloc(struct io_rsrc_data *data, unsigned nr);
struct io_rsrc_node *io_find_buf_node(struct io_kiocb *req,
unsigned issue_flags);
int io_import_reg_buf(struct io_kiocb *req, struct iov_iter *iter,
u64 buf_addr, size_t len, int ddir,
unsigned issue_flags);
+int io_import_buf_node(struct io_kiocb *req, struct iov_iter *iter,
+ u64 buf_addr, size_t len, int ddir);
int io_register_clone_buffers(struct io_ring_ctx *ctx, void __user *arg);
int io_sqe_buffers_unregister(struct io_ring_ctx *ctx);
int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
unsigned int nr_args, u64 __user *tags);
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index de39b602aa82..15a76fe48fe5 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -232,10 +232,15 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
return -EOPNOTSUPP;
issue_flags |= IO_URING_F_IOPOLL;
req->iopoll_completed = 0;
}
+ if (ioucmd->flags & IORING_URING_CMD_FIXED) {
+ if (!io_find_buf_node(req, issue_flags))
+ return -EFAULT;
+ }
+
ret = file->f_op->uring_cmd(ioucmd, issue_flags);
if (ret == -EAGAIN || ret == -EIOCBQUEUED)
return ret;
if (ret < 0)
req_set_fail(req);
@@ -244,16 +249,15 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
return IOU_OK;
}
int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
struct iov_iter *iter,
- struct io_uring_cmd *ioucmd,
- unsigned int issue_flags)
+ struct io_uring_cmd *ioucmd)
{
struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
- return io_import_reg_buf(req, iter, ubuf, len, rw, issue_flags);
+ return io_import_buf_node(req, iter, ubuf, len, rw);
}
EXPORT_SYMBOL_GPL(io_uring_cmd_import_fixed);
void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd)
{
--
2.45.2
next prev parent reply other threads:[~2025-03-21 18:48 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-21 18:48 [PATCH 0/3] Consistently look up fixed buffers before going async Caleb Sander Mateos
2025-03-21 18:48 ` [PATCH 1/3] io_uring/net: only import send_zc buffer once Caleb Sander Mateos
2025-03-21 20:38 ` Pavel Begunkov
2025-03-21 20:44 ` Caleb Sander Mateos
2025-03-21 18:48 ` [PATCH 2/3] io_uring/net: import send_zc fixed buffer before going async Caleb Sander Mateos
2025-03-21 18:48 ` Caleb Sander Mateos [this message]
2025-03-21 20:35 ` [PATCH 3/3] io_uring/uring_cmd: import " Pavel Begunkov
2025-03-21 21:38 ` Caleb Sander Mateos
2025-03-22 12:18 ` Pavel Begunkov
2025-03-21 19:53 ` [PATCH 0/3] Consistently look up fixed buffers " Jens Axboe
2025-03-21 20:24 ` Pavel Begunkov
2025-03-21 21:24 ` Caleb Sander Mateos
2025-03-22 12:33 ` Pavel Begunkov
2025-03-22 7:42 ` Ming Lei
2025-03-22 7:33 ` Ming Lei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox