Re: [PATCH 4/5] nvme: split out metadata vs non metadata end_io uring_cmd completions
From: Jens Axboe <axboe@kernel.dk>
Date: 2022-09-23 20:53:03
Also in:
linux-nvme, linux-scsi
Subsystem:
nvm express driver, the rest · Maintainers:
Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Linus Torvalds
On 9/23/22 9:21 AM, Christoph Hellwig wrote:
quoted
+ union { + struct { + void *meta; /* kernel-resident buffer */ + void __user *meta_buffer; + }; + struct { + u32 nvme_flags; + u32 nvme_status; + u64 result; + }; + };Without naming the arms of the union this is becoming a bit too much of a mess..quoted
+static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd) +{ + struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd); + int status; + + if (pdu->nvme_flags & NVME_REQ_CANCELLED) + status = -EINTR; + else + status = pdu->nvme_status;If you add a signed int field you only need one field instead of two in the pdu for this (the nvme status is only 15 bits anyway).
For both of these, how about we just simplify like below? I think at that point it's useless to name them anyway.
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 25f2f6df1602..6f955984ca14 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c@@ -350,16 +350,13 @@ struct nvme_uring_cmd_pdu { struct request *req; }; u32 meta_len; + u32 nvme_status; union { struct { void *meta; /* kernel-resident buffer */ void __user *meta_buffer; }; - struct { - u32 nvme_flags; - u32 nvme_status; - u64 result; - }; + u64 result; }; };
@@ -396,17 +393,11 @@ static void nvme_uring_task_meta_cb(struct io_uring_cmd *ioucmd) static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd) { struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd); - int status; - - if (pdu->nvme_flags & NVME_REQ_CANCELLED) - status = -EINTR; - else - status = pdu->nvme_status; if (pdu->bio) blk_rq_unmap_user(pdu->bio); - io_uring_cmd_done(ioucmd, status, pdu->result); + io_uring_cmd_done(ioucmd, pdu->nvme_status, pdu->result); } static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
@@ -417,8 +408,10 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req, void *cookie = READ_ONCE(ioucmd->cookie); req->bio = pdu->bio; - pdu->nvme_flags = nvme_req(req)->flags; - pdu->nvme_status = nvme_req(req)->status; + if (nvme_req(req)->flags & NVME_REQ_CANCELLED) + pdu->nvme_status = -EINTR; + else + pdu->nvme_status = nvme_req(req)->status; pdu->result = le64_to_cpu(nvme_req(req)->result.u64); /*
--
Jens Axboe