[v2,1/2] nvme: move command clear into the various setup helpers

Commit Message

Jens Axboe Oct. 18, 2021, 12:53 p.m. UTC

On 10/18/21 6:49 AM, Jens Axboe wrote:
> We don't have to worry about doing extra memsets by moving it outside
> the protection of RQF_DONTPREP, as nvme doesn't do partial completions.
> 
> This is in preparation for making the read/write fast path not do a full
> memset of the command.

Gah, v2 of this one below, it send out an older one.

commit fb4e29f648e320c94f210c54692c754ad69fb6f6
Author: Jens Axboe <[e-mail]>
Date:   Mon Oct 18 06:45:06 2021 -0600

    nvme: move command clear into the various setup helpers
    
    We don't have to worry about doing extra memsets by moving it outside
    the protection of RQF_DONTPREP, as nvme doesn't do partial completions.
    
    This is in preparation for making the read/write fast path not do a full
    memset of the command.
    
    Signed-off-by: Jens Axboe <[e-mail]>

Comments

Keith Busch Oct. 19, 2021, 6:28 p.m. UTC | #1

On Mon, Oct 18, 2021 at 06:53:02AM -0600, Jens Axboe wrote:
> commit fb4e29f648e320c94f210c54692c754ad69fb6f6
> Author: Jens Axboe <[e-mail]>
> Date:   Mon Oct 18 06:45:06 2021 -0600
> 
>     nvme: move command clear into the various setup helpers
>     
>     We don't have to worry about doing extra memsets by moving it outside
>     the protection of RQF_DONTPREP, as nvme doesn't do partial completions.
>     
>     This is in preparation for making the read/write fast path not do a full
>     memset of the command.
>     
>     Signed-off-by: Jens Axboe <[e-mail]>

Looks good.

Reviewed-by: Keith Busch <[e-mail]>

Patch

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ae15cb714596..de2250c5b057 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -823,6 +823,7 @@  static void nvme_assign_write_stream(struct nvme_ctrl *ctrl,
 static inline void nvme_setup_flush(struct nvme_ns *ns,
 		struct nvme_command *cmnd)
 {
+	memset(cmnd, 0, sizeof(*cmnd));
 	cmnd->common.opcode = nvme_cmd_flush;
 	cmnd->common.nsid = cpu_to_le32(ns->head->ns_id);
 }
@@ -874,6 +875,7 @@  static blk_status_t nvme_setup_discard(struct nvme_ns *ns, struct request *req,
 		return BLK_STS_IOERR;
 	}
 
+	memset(cmnd, 0, sizeof(*cmnd));
 	cmnd->dsm.opcode = nvme_cmd_dsm;
 	cmnd->dsm.nsid = cpu_to_le32(ns->head->ns_id);
 	cmnd->dsm.nr = cpu_to_le32(segments - 1);
@@ -890,6 +892,8 @@  static blk_status_t nvme_setup_discard(struct nvme_ns *ns, struct request *req,
 static inline blk_status_t nvme_setup_write_zeroes(struct nvme_ns *ns,
 		struct request *req, struct nvme_command *cmnd)
 {
+	memset(cmnd, 0, sizeof(*cmnd));
+
 	if (ns->ctrl->quirks & NVME_QUIRK_DEALLOCATE_ZEROES)
 		return nvme_setup_discard(ns, req, cmnd);
 
@@ -914,6 +918,8 @@  static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
 	u16 control = 0;
 	u32 dsmgmt = 0;
 
+	memset(cmnd, 0, sizeof(*cmnd));
+
 	if (req->cmd_flags & REQ_FUA)
 		control |= NVME_RW_FUA;
 	if (req->cmd_flags & (REQ_FAILFAST_DEV | REQ_RAHEAD))
@@ -982,10 +988,8 @@  blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
 	struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
 	blk_status_t ret = BLK_STS_OK;
 
-	if (!(req->rq_flags & RQF_DONTPREP)) {
+	if (!(req->rq_flags & RQF_DONTPREP))
 		nvme_clear_nvme_request(req);
-		memset(cmd, 0, sizeof(*cmd));
-	}
 
 	switch (req_op(req)) {
 	case REQ_OP_DRV_IN:
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index d95010481fce..bfc259e0d7b8 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -233,6 +233,8 @@  int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, struct request *req,
 		struct nvme_command *c, enum nvme_zone_mgmt_action action)
 {
+	memset(c, 0, sizeof(*c));
+
 	c->zms.opcode = nvme_cmd_zone_mgmt_send;
 	c->zms.nsid = cpu_to_le32(ns->head->ns_id);
 	c->zms.slba = cpu_to_le64(nvme_sect_to_lba(ns, blk_rq_pos(req)));