]> www.infradead.org Git - nvme.git/commitdiff
nvme/ioctl: move fixed buffer lookup to nvme_uring_cmd_io()
authorCaleb Sander Mateos <csander@purestorage.com>
Fri, 28 Mar 2025 15:46:47 +0000 (09:46 -0600)
committerKeith Busch <kbusch@kernel.org>
Mon, 31 Mar 2025 15:48:25 +0000 (08:48 -0700)
nvme_map_user_request() is called from both nvme_submit_user_cmd() and
nvme_uring_cmd_io(). But the ioucmd branch is only applicable to
nvme_uring_cmd_io(). Move it to nvme_uring_cmd_io() and just pass the
resulting iov_iter to nvme_map_user_request().

For NVMe passthru operations with fixed buffers, the fixed buffer lookup
happens in io_uring_cmd_import_fixed(). But nvme_uring_cmd_io() can
return -EAGAIN first from nvme_alloc_user_request() 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 NVMe passthru operation is followed by UBLK_U_IO_UNREGISTER_IO_BUF
to unregister the fixed buffer and the NVMe passthru goes async, the
fixed buffer lookup will fail because it happens after the unregister.

Userspace should not depend on the order in which io_uring issues SQEs
submitted in parallel, but it may try submitting the SQEs together and
fall back on a slow path if the fixed buffer lookup fails. To make the
fast path more likely, do the import before nvme_alloc_user_request().

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
drivers/nvme/host/ioctl.c

index 809910da8c6eb46e2a91c60cd3448e8fb130d2bd..ca86d3bf7ea49d0ec812640a6c0267a5aad40b79 100644 (file)
@@ -114,8 +114,7 @@ static struct request *nvme_alloc_user_request(struct request_queue *q,
 
 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 iov_iter *iter, unsigned int flags)
 {
        struct request_queue *q = req->q;
        struct nvme_ns *ns = q->queuedata;
@@ -137,24 +136,12 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
                                      "using unchecked metadata buffer\n");
        }
 
-       if (ioucmd && (ioucmd->flags & IORING_URING_CMD_FIXED)) {
-               struct iov_iter iter;
-
-               /* fixedbufs is only for non-vectored io */
-               if (flags & NVME_IOCTL_VEC)
-                       return -EINVAL;
-
-               ret = io_uring_cmd_import_fixed(ubuffer, bufflen,
-                               rq_data_dir(req), &iter, ioucmd,
-                               iou_issue_flags);
-               if (ret < 0)
-                       return ret;
-               ret = blk_rq_map_user_iov(q, req, NULL, &iter, GFP_KERNEL);
-       } else {
+       if (iter)
+               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),
                                bufflen, GFP_KERNEL, flags & NVME_IOCTL_VEC, 0,
                                0, rq_data_dir(req));
-       }
 
        if (ret)
                return ret;
@@ -196,7 +183,7 @@ static int nvme_submit_user_cmd(struct request_queue *q,
        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)
                        goto out_free_req;
        }
@@ -468,6 +455,8 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
        struct request_queue *q = ns ? ns->queue : ctrl->admin_q;
        struct nvme_uring_data d;
        struct nvme_command c;
+       struct iov_iter iter;
+       struct iov_iter *map_iter = NULL;
        struct request *req;
        blk_opf_t rq_flags = REQ_ALLOC_CACHE;
        blk_mq_req_flags_t blk_flags = 0;
@@ -503,6 +492,20 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
        d.metadata_len = READ_ONCE(cmd->metadata_len);
        d.timeout_ms = READ_ONCE(cmd->timeout_ms);
 
+       if (d.data_len && (ioucmd->flags & IORING_URING_CMD_FIXED)) {
+               /* fixedbufs is only for non-vectored io */
+               if (vec)
+                       return -EINVAL;
+
+               ret = io_uring_cmd_import_fixed(d.addr, d.data_len,
+                       nvme_is_write(&c) ? WRITE : READ, &iter, ioucmd,
+                       issue_flags);
+               if (ret < 0)
+                       return ret;
+
+               map_iter = &iter;
+       }
+
        if (issue_flags & IO_URING_F_NONBLOCK) {
                rq_flags |= REQ_NOWAIT;
                blk_flags = BLK_MQ_REQ_NOWAIT;
@@ -516,9 +519,9 @@ 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);
+               ret = nvme_map_user_request(req, d.addr, d.data_len,
+                       nvme_to_user_ptr(d.metadata), d.metadata_len,
+                       map_iter, vec);
                if (ret)
                        goto out_free_req;
        }