Re: [PATCH v2 1/2] target/rd: reduce code duplication in rd_execute_rw()

2015-04-07 Thread Nicholas A. Bellinger 
On Sun, 2015-04-05 at 23:59 +0900, Akinobu Mita wrote:
 Factor out code duplication in rd_execute_rw() into a helper function
 rd_do_prot_rw().  This change is required to minimize the forthcoming
 fix in rd_do_prot_rw().
 
 Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
 Cc: Nicholas Bellinger n...@linux-iscsi.org
 Cc: Sagi Grimberg sa...@dev.mellanox.co.il
 Cc: Martin K. Petersen martin.peter...@oracle.com
 Cc: Christoph Hellwig h...@lst.de
 Cc: James E.J. Bottomley james.bottom...@hansenpartnership.com
 Cc: target-de...@vger.kernel.org
 Cc: linux-scsi@vger.kernel.org
 ---
 * v2
 - Pass dif_verify() function pointer to helper function instead of is_write,
   suggested by Sagi Grimberg.
 
  drivers/target/target_core_rd.c | 66 
 -
  1 file changed, 32 insertions(+), 34 deletions(-)

Nice patch.  Applied to for-next with a bit of fuzz due to the
WRITE_STRIP + READ_INSERT related changes already queued up..

Thanks Akinobu!

--nab

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] target/rd: reduce code duplication in rd_execute_rw()

2015-04-06 Thread Sagi Grimberg 

On 4/5/2015 5:59 PM, Akinobu Mita wrote:

Factor out code duplication in rd_execute_rw() into a helper function
rd_do_prot_rw().  This change is required to minimize the forthcoming
fix in rd_do_prot_rw().

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Cc: Sagi Grimberg sa...@dev.mellanox.co.il
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: Christoph Hellwig h...@lst.de
Cc: James E.J. Bottomley james.bottom...@hansenpartnership.com
Cc: target-de...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
* v2
- Pass dif_verify() function pointer to helper function instead of is_write,
   suggested by Sagi Grimberg.

  drivers/target/target_core_rd.c | 66 -
  1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 98e83ac..ac5e8d2 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -382,6 +382,36 @@ static struct rd_dev_sg_table *rd_get_prot_table(struct 
rd_dev *rd_dev, u32 page
return NULL;
  }

+typedef sense_reason_t (*dif_verify)(struct se_cmd *, sector_t, unsigned int,
+unsigned int, struct scatterlist *, int);
+
+static sense_reason_t rd_do_prot_rw(struct se_cmd *cmd, dif_verify dif_verify)
+{
+   struct se_device *se_dev = cmd-se_dev;
+   struct rd_dev *dev = RD_DEV(se_dev);
+   struct rd_dev_sg_table *prot_table;
+   struct scatterlist *prot_sg;
+   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
+   u32 prot_offset, prot_page;
+   u64 tmp;
+   sense_reason_t rc;
+
+   tmp = cmd-t_task_lba * se_dev-prot_length;
+   prot_offset = do_div(tmp, PAGE_SIZE);
+   prot_page = tmp;
+
+   prot_table = rd_get_prot_table(dev, prot_page);
+   if (!prot_table)
+   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+
+   prot_sg = prot_table-sg_table[prot_page -
+   prot_table-page_start_offset];
+
+   rc = dif_verify(cmd, cmd-t_task_lba, sectors, 0, prot_sg, prot_offset);
+
+   return rc;


Nit, Given there is no action on the returned rc, this can be reduced to:
return dif_verify();


+}
+
  static sense_reason_t
  rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
  enum dma_data_direction data_direction)
@@ -420,23 +450,7 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, 
u32 sgl_nents,
cmd-t_task_lba, rd_size, rd_page, rd_offset);

if (cmd-prot_type  data_direction == DMA_TO_DEVICE) {
-   struct rd_dev_sg_table *prot_table;
-   struct scatterlist *prot_sg;
-   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
-   u32 prot_offset, prot_page;
-
-   tmp = cmd-t_task_lba * se_dev-prot_length;
-   prot_offset = do_div(tmp, PAGE_SIZE);
-   prot_page = tmp;
-
-   prot_table = rd_get_prot_table(dev, prot_page);
-   if (!prot_table)
-   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-
-   prot_sg = prot_table-sg_table[prot_page - 
prot_table-page_start_offset];
-
-   rc = sbc_dif_verify_write(cmd, cmd-t_task_lba, sectors, 0,
- prot_sg, prot_offset);
+   rc = rd_do_prot_rw(cmd, sbc_dif_verify_write);
if (rc)
return rc;
}
@@ -503,23 +517,7 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, 
u32 sgl_nents,
sg_miter_stop(m);

if (cmd-prot_type  data_direction == DMA_FROM_DEVICE) {
-   struct rd_dev_sg_table *prot_table;
-   struct scatterlist *prot_sg;
-   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
-   u32 prot_offset, prot_page;
-
-   tmp = cmd-t_task_lba * se_dev-prot_length;
-   prot_offset = do_div(tmp, PAGE_SIZE);
-   prot_page = tmp;
-
-   prot_table = rd_get_prot_table(dev, prot_page);
-   if (!prot_table)
-   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-
-   prot_sg = prot_table-sg_table[prot_page - 
prot_table-page_start_offset];
-
-   rc = sbc_dif_verify_read(cmd, cmd-t_task_lba, sectors, 0,
-prot_sg, prot_offset);
+   rc = rd_do_prot_rw(cmd, sbc_dif_verify_read);
if (rc)
return rc;
}



--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] target/rd: reduce code duplication in rd_execute_rw()

2015-04-06 Thread Akinobu Mita 
2015-04-06 16:43 GMT+09:00 Sagi Grimberg sa...@dev.mellanox.co.il:
 On 4/5/2015 5:59 PM, Akinobu Mita wrote:

 Factor out code duplication in rd_execute_rw() into a helper function
 rd_do_prot_rw().  This change is required to minimize the forthcoming
 fix in rd_do_prot_rw().

 Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
 Cc: Nicholas Bellinger n...@linux-iscsi.org
 Cc: Sagi Grimberg sa...@dev.mellanox.co.il
 Cc: Martin K. Petersen martin.peter...@oracle.com
 Cc: Christoph Hellwig h...@lst.de
 Cc: James E.J. Bottomley james.bottom...@hansenpartnership.com
 Cc: target-de...@vger.kernel.org
 Cc: linux-scsi@vger.kernel.org
 ---
 * v2
 - Pass dif_verify() function pointer to helper function instead of
 is_write,
suggested by Sagi Grimberg.

   drivers/target/target_core_rd.c | 66
 -
   1 file changed, 32 insertions(+), 34 deletions(-)

 diff --git a/drivers/target/target_core_rd.c
 b/drivers/target/target_core_rd.c
 index 98e83ac..ac5e8d2 100644
 --- a/drivers/target/target_core_rd.c
 +++ b/drivers/target/target_core_rd.c
 @@ -382,6 +382,36 @@ static struct rd_dev_sg_table
 *rd_get_prot_table(struct rd_dev *rd_dev, u32 page
 return NULL;
   }

 +typedef sense_reason_t (*dif_verify)(struct se_cmd *, sector_t, unsigned
 int,
 +unsigned int, struct scatterlist *,
 int);
 +
 +static sense_reason_t rd_do_prot_rw(struct se_cmd *cmd, dif_verify
 dif_verify)
 +{
 +   struct se_device *se_dev = cmd-se_dev;
 +   struct rd_dev *dev = RD_DEV(se_dev);
 +   struct rd_dev_sg_table *prot_table;
 +   struct scatterlist *prot_sg;
 +   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
 +   u32 prot_offset, prot_page;
 +   u64 tmp;
 +   sense_reason_t rc;
 +
 +   tmp = cmd-t_task_lba * se_dev-prot_length;
 +   prot_offset = do_div(tmp, PAGE_SIZE);
 +   prot_page = tmp;
 +
 +   prot_table = rd_get_prot_table(dev, prot_page);
 +   if (!prot_table)
 +   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 +
 +   prot_sg = prot_table-sg_table[prot_page -
 +   prot_table-page_start_offset];
 +
 +   rc = dif_verify(cmd, cmd-t_task_lba, sectors, 0, prot_sg,
 prot_offset);
 +
 +   return rc;


 Nit, Given there is no action on the returned rc, this can be reduced to:
 return dif_verify();

You are right,  but the patch 2/2 inserts the lines to release
temporary prot_sg before return statement, so assignment and return
statements will be both required in the end.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] target/rd: reduce code duplication in rd_execute_rw()

2015-04-05 Thread Akinobu Mita 
Factor out code duplication in rd_execute_rw() into a helper function
rd_do_prot_rw().  This change is required to minimize the forthcoming
fix in rd_do_prot_rw().

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Cc: Sagi Grimberg sa...@dev.mellanox.co.il
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: Christoph Hellwig h...@lst.de
Cc: James E.J. Bottomley james.bottom...@hansenpartnership.com
Cc: target-de...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
* v2
- Pass dif_verify() function pointer to helper function instead of is_write,
  suggested by Sagi Grimberg.

 drivers/target/target_core_rd.c | 66 -
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 98e83ac..ac5e8d2 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -382,6 +382,36 @@ static struct rd_dev_sg_table *rd_get_prot_table(struct 
rd_dev *rd_dev, u32 page
return NULL;
 }
 
+typedef sense_reason_t (*dif_verify)(struct se_cmd *, sector_t, unsigned int,
+unsigned int, struct scatterlist *, int);
+
+static sense_reason_t rd_do_prot_rw(struct se_cmd *cmd, dif_verify dif_verify)
+{
+   struct se_device *se_dev = cmd-se_dev;
+   struct rd_dev *dev = RD_DEV(se_dev);
+   struct rd_dev_sg_table *prot_table;
+   struct scatterlist *prot_sg;
+   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
+   u32 prot_offset, prot_page;
+   u64 tmp;
+   sense_reason_t rc;
+
+   tmp = cmd-t_task_lba * se_dev-prot_length;
+   prot_offset = do_div(tmp, PAGE_SIZE);
+   prot_page = tmp;
+
+   prot_table = rd_get_prot_table(dev, prot_page);
+   if (!prot_table)
+   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+
+   prot_sg = prot_table-sg_table[prot_page -
+   prot_table-page_start_offset];
+
+   rc = dif_verify(cmd, cmd-t_task_lba, sectors, 0, prot_sg, prot_offset);
+
+   return rc;
+}
+
 static sense_reason_t
 rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
  enum dma_data_direction data_direction)
@@ -420,23 +450,7 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, 
u32 sgl_nents,
cmd-t_task_lba, rd_size, rd_page, rd_offset);
 
if (cmd-prot_type  data_direction == DMA_TO_DEVICE) {
-   struct rd_dev_sg_table *prot_table;
-   struct scatterlist *prot_sg;
-   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
-   u32 prot_offset, prot_page;
-
-   tmp = cmd-t_task_lba * se_dev-prot_length;
-   prot_offset = do_div(tmp, PAGE_SIZE);
-   prot_page = tmp;
-
-   prot_table = rd_get_prot_table(dev, prot_page);
-   if (!prot_table)
-   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-
-   prot_sg = prot_table-sg_table[prot_page - 
prot_table-page_start_offset];
-
-   rc = sbc_dif_verify_write(cmd, cmd-t_task_lba, sectors, 0,
- prot_sg, prot_offset);
+   rc = rd_do_prot_rw(cmd, sbc_dif_verify_write);
if (rc)
return rc;
}
@@ -503,23 +517,7 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, 
u32 sgl_nents,
sg_miter_stop(m);
 
if (cmd-prot_type  data_direction == DMA_FROM_DEVICE) {
-   struct rd_dev_sg_table *prot_table;
-   struct scatterlist *prot_sg;
-   u32 sectors = cmd-data_length / se_dev-dev_attrib.block_size;
-   u32 prot_offset, prot_page;
-
-   tmp = cmd-t_task_lba * se_dev-prot_length;
-   prot_offset = do_div(tmp, PAGE_SIZE);
-   prot_page = tmp;
-
-   prot_table = rd_get_prot_table(dev, prot_page);
-   if (!prot_table)
-   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-
-   prot_sg = prot_table-sg_table[prot_page - 
prot_table-page_start_offset];
-
-   rc = sbc_dif_verify_read(cmd, cmd-t_task_lba, sectors, 0,
-prot_sg, prot_offset);
+   rc = rd_do_prot_rw(cmd, sbc_dif_verify_read);
if (rc)
return rc;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html