Re: [PATCH v5 25/26] nvme: remove redundant NvmeCmd pointer parameter

2020-02-12 Thread Maxim Levitsky 
On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote:
> The command struct is available in the NvmeRequest that we generally
> pass around anyway.
> 
> Signed-off-by: Klaus Jensen 
> ---
>  hw/block/nvme.c | 198 
>  1 file changed, 98 insertions(+), 100 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index bdef53a590b0..5fe2e2fe1fa9 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -566,16 +566,18 @@ unmap:
>  }
>  
>  static uint16_t nvme_dma(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
> -NvmeCmd *cmd, DMADirection dir, NvmeRequest *req)
> +DMADirection dir, NvmeRequest *req)
>  {
>  uint16_t status = NVME_SUCCESS;
>  size_t bytes;
> +uint64_t prp1, prp2;
>  
> -switch (NVME_CMD_FLAGS_PSDT(cmd->flags)) {
> +switch (NVME_CMD_FLAGS_PSDT(req->cmd.flags)) {
>  case PSDT_PRP:
> -status = nvme_map_prp(n, >qsg, >iov,
> -le64_to_cpu(cmd->dptr.prp.prp1), le64_to_cpu(cmd->dptr.prp.prp2),
> -len, req);
> +prp1 = le64_to_cpu(req->cmd.dptr.prp.prp1);
> +prp2 = le64_to_cpu(req->cmd.dptr.prp.prp2);
> +
> +status = nvme_map_prp(n, >qsg, >iov, prp1, prp2, len, req);
>  if (status) {
>  return status;
>  }
> @@ -589,7 +591,7 @@ static uint16_t nvme_dma(NvmeCtrl *n, uint8_t *ptr, 
> uint32_t len,
>  return NVME_INVALID_FIELD;
>  }
>  
> -status = nvme_map_sgl(n, >qsg, >iov, cmd->dptr.sgl, len,
> +status = nvme_map_sgl(n, >qsg, >iov, req->cmd.dptr.sgl, 
> len,
>  req);
>  if (status) {
>  return status;
> @@ -632,20 +634,21 @@ static uint16_t nvme_dma(NvmeCtrl *n, uint8_t *ptr, 
> uint32_t len,
>  return status;
>  }
>  
> -static uint16_t nvme_map(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> +static uint16_t nvme_map(NvmeCtrl *n, NvmeRequest *req)
>  {
>  uint32_t len = req->nlb << nvme_ns_lbads(req->ns);
>  uint64_t prp1, prp2;
>  
> -switch (NVME_CMD_FLAGS_PSDT(cmd->flags)) {
> +switch (NVME_CMD_FLAGS_PSDT(req->cmd.flags)) {
>  case PSDT_PRP:
> -prp1 = le64_to_cpu(cmd->dptr.prp.prp1);
> -prp2 = le64_to_cpu(cmd->dptr.prp.prp2);
> +prp1 = le64_to_cpu(req->cmd.dptr.prp.prp1);
> +prp2 = le64_to_cpu(req->cmd.dptr.prp.prp2);
>  
>  return nvme_map_prp(n, >qsg, >iov, prp1, prp2, len, req);
>  case PSDT_SGL_MPTR_CONTIGUOUS:
>  case PSDT_SGL_MPTR_SGL:
> -return nvme_map_sgl(n, >qsg, >iov, cmd->dptr.sgl, len, 
> req);
> +return nvme_map_sgl(n, >qsg, >iov, req->cmd.dptr.sgl, len,
> +req);
>  default:
>  return NVME_INVALID_FIELD;
>  }
> @@ -1024,7 +1027,7 @@ static void nvme_aio_cb(void *opaque, int ret)
>  nvme_aio_destroy(aio);
>  }
>  
> -static uint16_t nvme_flush(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> +static uint16_t nvme_flush(NvmeCtrl *n, NvmeRequest *req)
>  {
>  NvmeNamespace *ns = req->ns;
>  NvmeAIO *aio = g_new0(NvmeAIO, 1);
> @@ -1040,12 +1043,12 @@ static uint16_t nvme_flush(NvmeCtrl *n, NvmeCmd *cmd, 
> NvmeRequest *req)
>  return NVME_NO_COMPLETE;
>  }
>  
> -static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> +static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeRequest *req)
>  {
>  NvmeAIO *aio;
>  
>  NvmeNamespace *ns = req->ns;
> -NvmeRwCmd *rw = (NvmeRwCmd *) cmd;
> +NvmeRwCmd *rw = (NvmeRwCmd *) >cmd;
>  
>  int64_t offset;
>  size_t count;
> @@ -1081,9 +1084,9 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeCmd 
> *cmd, NvmeRequest *req)
>  return NVME_NO_COMPLETE;
>  }
>  
> -static uint16_t nvme_rw(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> +static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req)
>  {
> -NvmeRwCmd *rw = (NvmeRwCmd *) cmd;
> +NvmeRwCmd *rw = (NvmeRwCmd *) >cmd;
>  NvmeNamespace *ns = req->ns;
>  int status;
>  
> @@ -1103,7 +1106,7 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeCmd *cmd, 
> NvmeRequest *req)
>  return status;
>  }
>  
> -status = nvme_map(n, cmd, req);
> +status = nvme_map(n, req);
>  if (status) {
>  block_acct_invalid(blk_get_stats(ns->blk), acct);
>  return status;
> @@ -1115,12 +1118,12 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeCmd *cmd, 
> NvmeRequest *req)
>  return NVME_NO_COMPLETE;
>  }
>  
> -static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> +static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest *req)
>  {
> -uint32_t nsid = le32_to_cpu(cmd->nsid);
> +uint32_t nsid = le32_to_cpu(req->cmd.nsid);
>  
>  trace_nvme_dev_io_cmd(nvme_cid(req), nsid, le16_to_cpu(req->sq->sqid),
> -cmd->opcode);
> +req->cmd.opcode);
>  
>  req->ns = nvme_ns(n, nsid);
>  
> @@ -1128,16 +1131,16 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd 
> *cmd, NvmeRequest *req)
>  return nvme_nsid_err(n, nsid);
>  }

[PATCH v5 25/26] nvme: remove redundant NvmeCmd pointer parameter

2020-02-04 Thread Klaus Jensen 
The command struct is available in the NvmeRequest that we generally
pass around anyway.

Signed-off-by: Klaus Jensen 
---
 hw/block/nvme.c | 198 
 1 file changed, 98 insertions(+), 100 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index bdef53a590b0..5fe2e2fe1fa9 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -566,16 +566,18 @@ unmap:
 }
 
 static uint16_t nvme_dma(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
-NvmeCmd *cmd, DMADirection dir, NvmeRequest *req)
+DMADirection dir, NvmeRequest *req)
 {
 uint16_t status = NVME_SUCCESS;
 size_t bytes;
+uint64_t prp1, prp2;
 
-switch (NVME_CMD_FLAGS_PSDT(cmd->flags)) {
+switch (NVME_CMD_FLAGS_PSDT(req->cmd.flags)) {
 case PSDT_PRP:
-status = nvme_map_prp(n, >qsg, >iov,
-le64_to_cpu(cmd->dptr.prp.prp1), le64_to_cpu(cmd->dptr.prp.prp2),
-len, req);
+prp1 = le64_to_cpu(req->cmd.dptr.prp.prp1);
+prp2 = le64_to_cpu(req->cmd.dptr.prp.prp2);
+
+status = nvme_map_prp(n, >qsg, >iov, prp1, prp2, len, req);
 if (status) {
 return status;
 }
@@ -589,7 +591,7 @@ static uint16_t nvme_dma(NvmeCtrl *n, uint8_t *ptr, 
uint32_t len,
 return NVME_INVALID_FIELD;
 }
 
-status = nvme_map_sgl(n, >qsg, >iov, cmd->dptr.sgl, len,
+status = nvme_map_sgl(n, >qsg, >iov, req->cmd.dptr.sgl, len,
 req);
 if (status) {
 return status;
@@ -632,20 +634,21 @@ static uint16_t nvme_dma(NvmeCtrl *n, uint8_t *ptr, 
uint32_t len,
 return status;
 }
 
-static uint16_t nvme_map(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
+static uint16_t nvme_map(NvmeCtrl *n, NvmeRequest *req)
 {
 uint32_t len = req->nlb << nvme_ns_lbads(req->ns);
 uint64_t prp1, prp2;
 
-switch (NVME_CMD_FLAGS_PSDT(cmd->flags)) {
+switch (NVME_CMD_FLAGS_PSDT(req->cmd.flags)) {
 case PSDT_PRP:
-prp1 = le64_to_cpu(cmd->dptr.prp.prp1);
-prp2 = le64_to_cpu(cmd->dptr.prp.prp2);
+prp1 = le64_to_cpu(req->cmd.dptr.prp.prp1);
+prp2 = le64_to_cpu(req->cmd.dptr.prp.prp2);
 
 return nvme_map_prp(n, >qsg, >iov, prp1, prp2, len, req);
 case PSDT_SGL_MPTR_CONTIGUOUS:
 case PSDT_SGL_MPTR_SGL:
-return nvme_map_sgl(n, >qsg, >iov, cmd->dptr.sgl, len, req);
+return nvme_map_sgl(n, >qsg, >iov, req->cmd.dptr.sgl, len,
+req);
 default:
 return NVME_INVALID_FIELD;
 }
@@ -1024,7 +1027,7 @@ static void nvme_aio_cb(void *opaque, int ret)
 nvme_aio_destroy(aio);
 }
 
-static uint16_t nvme_flush(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
+static uint16_t nvme_flush(NvmeCtrl *n, NvmeRequest *req)
 {
 NvmeNamespace *ns = req->ns;
 NvmeAIO *aio = g_new0(NvmeAIO, 1);
@@ -1040,12 +1043,12 @@ static uint16_t nvme_flush(NvmeCtrl *n, NvmeCmd *cmd, 
NvmeRequest *req)
 return NVME_NO_COMPLETE;
 }
 
-static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
+static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeRequest *req)
 {
 NvmeAIO *aio;
 
 NvmeNamespace *ns = req->ns;
-NvmeRwCmd *rw = (NvmeRwCmd *) cmd;
+NvmeRwCmd *rw = (NvmeRwCmd *) >cmd;
 
 int64_t offset;
 size_t count;
@@ -1081,9 +1084,9 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeCmd 
*cmd, NvmeRequest *req)
 return NVME_NO_COMPLETE;
 }
 
-static uint16_t nvme_rw(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
+static uint16_t nvme_rw(NvmeCtrl *n, NvmeRequest *req)
 {
-NvmeRwCmd *rw = (NvmeRwCmd *) cmd;
+NvmeRwCmd *rw = (NvmeRwCmd *) >cmd;
 NvmeNamespace *ns = req->ns;
 int status;
 
@@ -1103,7 +1106,7 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeCmd *cmd, 
NvmeRequest *req)
 return status;
 }
 
-status = nvme_map(n, cmd, req);
+status = nvme_map(n, req);
 if (status) {
 block_acct_invalid(blk_get_stats(ns->blk), acct);
 return status;
@@ -1115,12 +1118,12 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeCmd *cmd, 
NvmeRequest *req)
 return NVME_NO_COMPLETE;
 }
 
-static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
+static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest *req)
 {
-uint32_t nsid = le32_to_cpu(cmd->nsid);
+uint32_t nsid = le32_to_cpu(req->cmd.nsid);
 
 trace_nvme_dev_io_cmd(nvme_cid(req), nsid, le16_to_cpu(req->sq->sqid),
-cmd->opcode);
+req->cmd.opcode);
 
 req->ns = nvme_ns(n, nsid);
 
@@ -1128,16 +1131,16 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, 
NvmeRequest *req)
 return nvme_nsid_err(n, nsid);
 }
 
-switch (cmd->opcode) {
+switch (req->cmd.opcode) {
 case NVME_CMD_FLUSH:
-return nvme_flush(n, cmd, req);
+return nvme_flush(n, req);
 case NVME_CMD_WRITE_ZEROS:
-return nvme_write_zeros(n, cmd, req);
+return nvme_write_zeros(n, req);
 case NVME_CMD_WRITE:
 case NVME_CMD_READ:
-