[PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector()

[PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector()

[Bart Van Assche]

Hi Martin,

This patch series renames scsi_get_lba() into scsi_get_pos(). The name of
scsi_get_lba() is confusing since it does not return an LBA but instead the
start offset divided by 512.

Please consider this patch series for kernel v5.14.

Thanks,

Bart.

Changes compared to v2:
- Renamed scsi_get_pos() into scsi_get_sector() as suggested by Damien.
- Split the lpfc and qla2xxx changes into separate patches.

Changes compared to v1:
- Changed the "iscsi" prefix into "isci" for patch 4/7.
- Added support for READ(16) and WRITE(16) in patch 6/7.

Bart Van Assche (8):
  core: Introduce scsi_get_sector()
  iser: Use scsi_get_sector() instead of scsi_get_lba()
  zfcp: Use scsi_get_sector() instead of scsi_get_lba()
  isci: Use scsi_get_sector() instead of scsi_get_lba()
  lpfc: Use scsi_get_sector() instead of scsi_get_lba()
  qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()
  ufs: Fix the tracing code
  core: Remove scsi_get_lba()

 drivers/infiniband/ulp/iser/iser_verbs.c |  2 +-
 drivers/s390/scsi/zfcp_fsf.c             |  2 +-
 drivers/scsi/isci/request.c              |  4 +--
 drivers/scsi/lpfc/lpfc_scsi.c            | 12 ++++-----
 drivers/scsi/qla2xxx/qla_iocb.c          |  9 +++----
 drivers/scsi/qla2xxx/qla_isr.c           |  8 +++---
 drivers/scsi/ufs/ufshcd.c                | 34 +++++++++++-------------
 include/scsi/scsi_cmnd.h                 |  2 +-
 include/trace/events/ufs.h               | 10 +++----
 9 files changed, 38 insertions(+), 45 deletions(-)

[PATCH v3 1/8] core: Introduce scsi_get_sector()

[Bart Van Assche]

Since scsi_get_lba() returns a sector_t value instead of the LBA, the name
of that function is confusing. Introduce an identical function
scsi_get_sector(). A later patch will remove scsi_get_lba().

Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/scsi/scsi_cmnd.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index bc5535b269c5..7f55faa70301 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -289,6 +289,11 @@ static inline sector_t scsi_get_lba(struct scsi_cmnd *scmd)
 	return blk_rq_pos(scmd->request);
 }
 
+static inline sector_t scsi_get_sector(struct scsi_cmnd *scmd)
+{
+	return blk_rq_pos(scmd->request);
+}
+
 static inline unsigned int scsi_prot_interval(struct scsi_cmnd *scmd)
 {
 	return scmd->device->sector_size;

[PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

Use scsi_get_sector() instead of scsi_get_lba() since the name of the
latter is confusing. This patch does not change any functionality.

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/infiniband/ulp/iser/iser_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 136f6c4492e0..d6bbf1bf428c 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -949,7 +949,7 @@ u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			sector_t sector_off = mr_status.sig_err.sig_err_offset;
 
 			sector_div(sector_off, sector_size + 8);
-			*sector = scsi_get_lba(iser_task->sc) + sector_off;
+			*sector = scsi_get_sector(iser_task->sc) + sector_off;
 
 			iser_err("PI error found type %d at sector %llx "
 			       "expected %x vs actual %x\n",

[PATCH v3 3/8] zfcp: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

Use scsi_get_sector() instead of scsi_get_lba() since the name of the
latter is confusing. Additionally, use lower_32_bits() instead of
open-coding it. This patch does not change any functionality.

Cc: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/s390/scsi/zfcp_fsf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 2e4804ef2fb9..3d9a3dc4975b 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2600,7 +2600,7 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
 
 	if (scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) {
 		io->data_block_length = scsi_cmnd->device->sector_size;
-		io->ref_tag_value = scsi_get_lba(scsi_cmnd) & 0xFFFFFFFF;
+		io->ref_tag_value = lower_32_bits(scsi_get_sector(scsi_cmnd));
 	}
 
 	if (zfcp_fsf_set_data_dir(scsi_cmnd, &io->data_direction))

[PATCH v3 4/8] isci: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

Use scsi_get_sector() instead of scsi_get_lba() since the name of the
latter is confusing. Additionally, use lower_32_bits() instead of
open-coding it. This patch does not change any functionality.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/isci/request.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index e7c6cb4c1556..ad24ddbcaea3 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -341,7 +341,7 @@ static void scu_ssp_ireq_dif_insert(struct isci_request *ireq, u8 type, u8 op)
 	tc->reserved_E8_0 = 0;
 
 	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2))
-		tc->ref_tag_seed_gen = scsi_get_lba(scmd) & 0xffffffff;
+		tc->ref_tag_seed_gen = lower_32_bits(scsi_get_sector(scmd));
 	else if (type & SCSI_PROT_DIF_TYPE3)
 		tc->ref_tag_seed_gen = 0;
 }
@@ -369,7 +369,7 @@ static void scu_ssp_ireq_dif_strip(struct isci_request *ireq, u8 type, u8 op)
 	tc->app_tag_gen = 0;
 
 	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2))
-		tc->ref_tag_seed_verify = scsi_get_lba(scmd) & 0xffffffff;
+		tc->ref_tag_seed_verify = lower_32_bits(scsi_get_sector(scmd));
 	else if (type & SCSI_PROT_DIF_TYPE3)
 		tc->ref_tag_seed_verify = 0;
 

[PATCH v3 5/8] lpfc: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

Use scsi_get_sector() instead of scsi_get_lba() since the name of the
latter is confusing. This patch does not change any functionality.

Cc: James Smart <james.smart@broadcom.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/lpfc/lpfc_scsi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index eefbb9b22798..8c6b25807f12 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -2963,7 +2963,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
 				"9059 BLKGRD: Guard Tag error in cmd"
 				" 0x%x lba 0x%llx blk cnt 0x%x "
 				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
-				(unsigned long long)scsi_get_lba(cmd),
+				(u64)scsi_get_sector(cmd),
 				blk_rq_sectors(cmd->request), bgstat, bghm);
 	}
 
@@ -2980,7 +2980,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
 				"9060 BLKGRD: Ref Tag error in cmd"
 				" 0x%x lba 0x%llx blk cnt 0x%x "
 				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
-				(unsigned long long)scsi_get_lba(cmd),
+				(u64)scsi_get_sector(cmd),
 				blk_rq_sectors(cmd->request), bgstat, bghm);
 	}
 
@@ -2997,7 +2997,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
 				"9062 BLKGRD: App Tag error in cmd"
 				" 0x%x lba 0x%llx blk cnt 0x%x "
 				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
-				(unsigned long long)scsi_get_lba(cmd),
+				(u64)scsi_get_sector(cmd),
 				blk_rq_sectors(cmd->request), bgstat, bghm);
 	}
 
@@ -3028,7 +3028,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
 			break;
 		}
 
-		failing_sector = scsi_get_lba(cmd);
+		failing_sector = scsi_get_sector(cmd);
 		failing_sector += bghm;
 
 		/* Descriptor Information */
@@ -3041,7 +3041,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
 				"9068 BLKGRD: Unknown error in cmd"
 				" 0x%x lba 0x%llx blk cnt 0x%x "
 				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
-				(unsigned long long)scsi_get_lba(cmd),
+				(u64)scsi_get_sector(cmd),
 				blk_rq_sectors(cmd->request), bgstat, bghm);
 
 		/* Calcuate what type of error it was */
@@ -3174,7 +3174,7 @@ lpfc_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
 			break;
 		}
 
-		failing_sector = scsi_get_lba(cmd);
+		failing_sector = scsi_get_sector(cmd);
 		failing_sector += bghm;
 
 		/* Descriptor Information */

[PATCH v3 6/8] qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

Use scsi_get_sector() instead of scsi_get_lba() since the name of the
latter is confusing. Additionally, use lower_32_bits() instead of
open-coding it. This patch does not change any functionality.

Cc: Nilesh Javali <njavali@marvell.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 9 +++------
 drivers/scsi/qla2xxx/qla_isr.c  | 8 ++++----
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 38b5bdde2405..28e30a7e8883 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -778,8 +778,7 @@ qla24xx_set_t10dif_tags(srb_t *sp, struct fw_dif_context *pkt,
 		 * No check for ql2xenablehba_err_chk, as it would be an
 		 * I/O error if hba tag generation is not done.
 		 */
-		pkt->ref_tag = cpu_to_le32((uint32_t)
-		    (0xffffffff & scsi_get_lba(cmd)));
+		pkt->ref_tag = cpu_to_le32(lower_32_bits(scsi_get_sector(cmd)));
 
 		if (!qla2x00_hba_err_chk_enabled(sp))
 			break;
@@ -799,8 +798,7 @@ qla24xx_set_t10dif_tags(srb_t *sp, struct fw_dif_context *pkt,
 		pkt->app_tag_mask[0] = 0x0;
 		pkt->app_tag_mask[1] = 0x0;
 
-		pkt->ref_tag = cpu_to_le32((uint32_t)
-		    (0xffffffff & scsi_get_lba(cmd)));
+		pkt->ref_tag = cpu_to_le32(lower_32_bits(scsi_get_sector(cmd)));
 
 		if (!qla2x00_hba_err_chk_enabled(sp))
 			break;
@@ -824,8 +822,7 @@ qla24xx_set_t10dif_tags(srb_t *sp, struct fw_dif_context *pkt,
 	 * 16 bit app tag.
 	 */
 	case SCSI_PROT_DIF_TYPE1:
-		pkt->ref_tag = cpu_to_le32((uint32_t)
-		    (0xffffffff & scsi_get_lba(cmd)));
+		pkt->ref_tag = cpu_to_le32(lower_32_bits(scsi_get_sector(cmd)));
 		pkt->app_tag = cpu_to_le16(0);
 		pkt->app_tag_mask[0] = 0x0;
 		pkt->app_tag_mask[1] = 0x0;
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 67229af4c142..24d406411f81 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2632,7 +2632,7 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
 	    "DIF ERROR in cmd 0x%x lba 0x%llx act ref"
 	    " tag=0x%x, exp ref_tag=0x%x, act app tag=0x%x, exp app"
 	    " tag=0x%x, act guard=0x%x, exp guard=0x%x.\n",
-	    cmd->cmnd[0], (u64)scsi_get_lba(cmd), a_ref_tag, e_ref_tag,
+	    cmd->cmnd[0], (u64)scsi_get_sector(cmd), a_ref_tag, e_ref_tag,
 	    a_app_tag, e_app_tag, a_guard, e_guard);
 
 	/*
@@ -2644,10 +2644,10 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
 	    (scsi_get_prot_type(cmd) != SCSI_PROT_DIF_TYPE3 ||
 	     a_ref_tag == be32_to_cpu(T10_PI_REF_ESCAPE))) {
 		uint32_t blocks_done, resid;
-		sector_t lba_s = scsi_get_lba(cmd);
+		sector_t sector = scsi_get_sector(cmd);
 
 		/* 2TB boundary case covered automatically with this */
-		blocks_done = e_ref_tag - (uint32_t)lba_s + 1;
+		blocks_done = e_ref_tag - (uint32_t)sector + 1;
 
 		resid = scsi_bufflen(cmd) - (blocks_done *
 		    cmd->device->sector_size);
@@ -2677,7 +2677,7 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
 			if (k != blocks_done) {
 				ql_log(ql_log_warn, vha, 0x302f,
 				    "unexpected tag values tag:lba=%x:%llx)\n",
-				    e_ref_tag, (unsigned long long)lba_s);
+				    e_ref_tag, (u64)sector);
 				return 1;
 			}
 

[PATCH v3 7/8] ufs: Fix the tracing code

[Bart Van Assche]

Use scsi_get_sector() instead of scsi_get_lba() since the name of the
latter is confusing. Use scsi_get_sector() for all SCSI requests since all
SCSI requests have a block layer request attached and hence calling
scsi_get_sector() is allowed. Convert the scsi_get_sector() result
from sector_t into an LBA with sectors_to_logical(). Since READ(10),
WRITE(10), READ(16) and WRITE(16) all have a GROUP NUMBER field, extract
the GROUP NUMBER field for all four SCSI commands. Apply the 0x3f mask to
that field since the upper two bits are reserved. Rename the 'transfer_len'
variable into 'affected_bytes' since it represents the number of bytes
affected on the storage medium instead of the size of the SCSI data buffer.

Cc: Can Guo <cang@codeaurora.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Bean Huo <beanhuo@micron.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c  | 34 +++++++++++++++-------------------
 include/trace/events/ufs.h | 10 +++++-----
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ab707a2c3796..75310ac95b3d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -17,6 +17,8 @@
 #include <linux/blk-pm.h>
 #include <linux/blkdev.h>
 #include <scsi/scsi_driver.h>
+#include <scsi/scsi_cmnd.h>
+#include "../sd.h"
 #include "ufshcd.h"
 #include "ufs_quirks.h"
 #include "unipro.h"
@@ -369,7 +371,7 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 	u32 intr, doorbell;
 	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
 	struct scsi_cmnd *cmd = lrbp->cmd;
-	int transfer_len = -1;
+	int affected_bytes = -1;
 
 	if (!trace_ufshcd_command_enabled()) {
 		/* trace UPIU W/O tracing command */
@@ -381,30 +383,24 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 	if (cmd) { /* data phase exists */
 		/* trace UPIU also */
 		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
-		opcode = cmd->cmnd[0];
-		if ((opcode == READ_10) || (opcode == WRITE_10)) {
-			/*
-			 * Currently we only fully trace read(10) and write(10)
-			 * commands
-			 */
-			if (cmd->request && cmd->request->bio)
-				lba = cmd->request->bio->bi_iter.bi_sector;
-			transfer_len = be32_to_cpu(
-				lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
-			if (opcode == WRITE_10)
-				group_id = lrbp->cmd->cmnd[6];
-		} else if (opcode == UNMAP) {
-			if (cmd->request) {
-				lba = scsi_get_lba(cmd);
-				transfer_len = blk_rq_bytes(cmd->request);
-			}
+		lba = sectors_to_logical(cmd->device, scsi_get_sector(cmd));
+		affected_bytes = blk_rq_bytes(cmd->request);
+		switch (cmd->cmnd[0]) {
+		case READ_10:
+		case WRITE_10:
+			group_id = lrbp->cmd->cmnd[6] & 0x3f;
+			break;
+		case READ_16:
+		case WRITE_16:
+			group_id = lrbp->cmd->cmnd[14] & 0x3f;
+			break;
 		}
 	}
 
 	intr = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
 	doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	trace_ufshcd_command(dev_name(hba->dev), str_t, tag,
-			doorbell, transfer_len, intr, lba, opcode, group_id);
+			doorbell, affected_bytes, intr, lba, opcode, group_id);
 }
 
 static void ufshcd_print_clk_freqs(struct ufs_hba *hba)
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index 599739ee7b20..71055021de91 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -268,10 +268,10 @@ DEFINE_EVENT(ufshcd_template, ufshcd_wl_runtime_resume,
 
 TRACE_EVENT(ufshcd_command,
 	TP_PROTO(const char *dev_name, enum ufs_trace_str_t str_t,
-		 unsigned int tag, u32 doorbell, int transfer_len, u32 intr,
+		 unsigned int tag, u32 doorbell, int affected_bytes, u32 intr,
 		 u64 lba, u8 opcode, u8 group_id),
 
-	TP_ARGS(dev_name, str_t, tag, doorbell, transfer_len,
+	TP_ARGS(dev_name, str_t, tag, doorbell, affected_bytes,
 				intr, lba, opcode, group_id),
 
 	TP_STRUCT__entry(
@@ -279,7 +279,7 @@ TRACE_EVENT(ufshcd_command,
 		__field(enum ufs_trace_str_t, str_t)
 		__field(unsigned int, tag)
 		__field(u32, doorbell)
-		__field(int, transfer_len)
+		__field(int, affected_bytes)
 		__field(u32, intr)
 		__field(u64, lba)
 		__field(u8, opcode)
@@ -291,7 +291,7 @@ TRACE_EVENT(ufshcd_command,
 		__entry->str_t = str_t;
 		__entry->tag = tag;
 		__entry->doorbell = doorbell;
-		__entry->transfer_len = transfer_len;
+		__entry->affected_bytes = affected_bytes;
 		__entry->intr = intr;
 		__entry->lba = lba;
 		__entry->opcode = opcode;
@@ -301,7 +301,7 @@ TRACE_EVENT(ufshcd_command,
 	TP_printk(
 		"%s: %s: tag: %u, DB: 0x%x, size: %d, IS: %u, LBA: %llu, opcode: 0x%x (%s), group_id: 0x%x",
 		show_ufs_cmd_trace_str(__entry->str_t), __get_str(dev_name),
-		__entry->tag, __entry->doorbell, __entry->transfer_len,
+		__entry->tag, __entry->doorbell, __entry->affected_bytes,
 		__entry->intr, __entry->lba, (u32)__entry->opcode,
 		str_opcode(__entry->opcode), (u32)__entry->group_id
 	)

[PATCH v3 8/8] core: Remove scsi_get_lba()

[Bart Van Assche]

Remove scsi_get_lba() since all callers have been converted to
scsi_get_sector().

Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/scsi/scsi_cmnd.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 7f55faa70301..77a0916c8769 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -284,11 +284,6 @@ static inline unsigned char scsi_get_prot_type(struct scsi_cmnd *scmd)
 	return scmd->prot_type;
 }
 
-static inline sector_t scsi_get_lba(struct scsi_cmnd *scmd)
-{
-	return blk_rq_pos(scmd->request);
-}
-
 static inline sector_t scsi_get_sector(struct scsi_cmnd *scmd)
 {
 	return blk_rq_pos(scmd->request);

Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Since scsi_get_lba() returns a sector_t value instead of the LBA, the name
> of that function is confusing. Introduce an identical function
> scsi_get_sector(). A later patch will remove scsi_get_lba().
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  include/scsi/scsi_cmnd.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index bc5535b269c5..7f55faa70301 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -289,6 +289,11 @@ static inline sector_t scsi_get_lba(struct scsi_cmnd *scmd)
>  	return blk_rq_pos(scmd->request);
>  }
>  
> +static inline sector_t scsi_get_sector(struct scsi_cmnd *scmd)
> +{
> +	return blk_rq_pos(scmd->request);
> +}
> +
>  static inline unsigned int scsi_prot_interval(struct scsi_cmnd *scmd)
>  {
>  	return scmd->device->sector_size;
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. This patch does not change any functionality.
> 
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/infiniband/ulp/iser/iser_verbs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
> index 136f6c4492e0..d6bbf1bf428c 100644
> --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
> @@ -949,7 +949,7 @@ u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
>  			sector_t sector_off = mr_status.sig_err.sig_err_offset;
>  
>  			sector_div(sector_off, sector_size + 8);
> -			*sector = scsi_get_lba(iser_task->sc) + sector_off;
> +			*sector = scsi_get_sector(iser_task->sc) + sector_off;
>  
>  			iser_err("PI error found type %d at sector %llx "
>  			       "expected %x vs actual %x\n",
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 3/8] zfcp: Use scsi_get_sector() instead of scsi_get_lba()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. Additionally, use lower_32_bits() instead of
> open-coding it. This patch does not change any functionality.
> 
> Cc: Benjamin Block <bblock@linux.ibm.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/s390/scsi/zfcp_fsf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
> index 2e4804ef2fb9..3d9a3dc4975b 100644
> --- a/drivers/s390/scsi/zfcp_fsf.c
> +++ b/drivers/s390/scsi/zfcp_fsf.c
> @@ -2600,7 +2600,7 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
>  
>  	if (scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) {
>  		io->data_block_length = scsi_cmnd->device->sector_size;
> -		io->ref_tag_value = scsi_get_lba(scsi_cmnd) & 0xFFFFFFFF;
> +		io->ref_tag_value = lower_32_bits(scsi_get_sector(scsi_cmnd));
>  	}
>  
>  	if (zfcp_fsf_set_data_dir(scsi_cmnd, &io->data_direction))
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 4/8] isci: Use scsi_get_sector() instead of scsi_get_lba()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. Additionally, use lower_32_bits() instead of
> open-coding it. This patch does not change any functionality.
> 
> Reviewed-by: Lee Duncan <lduncan@suse.com>
> Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/isci/request.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
> index e7c6cb4c1556..ad24ddbcaea3 100644
> --- a/drivers/scsi/isci/request.c
> +++ b/drivers/scsi/isci/request.c
> @@ -341,7 +341,7 @@ static void scu_ssp_ireq_dif_insert(struct isci_request *ireq, u8 type, u8 op)
>  	tc->reserved_E8_0 = 0;
>  
>  	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2))
> -		tc->ref_tag_seed_gen = scsi_get_lba(scmd) & 0xffffffff;
> +		tc->ref_tag_seed_gen = lower_32_bits(scsi_get_sector(scmd));
>  	else if (type & SCSI_PROT_DIF_TYPE3)
>  		tc->ref_tag_seed_gen = 0;
>  }
> @@ -369,7 +369,7 @@ static void scu_ssp_ireq_dif_strip(struct isci_request *ireq, u8 type, u8 op)
>  	tc->app_tag_gen = 0;
>  
>  	if ((type & SCSI_PROT_DIF_TYPE1) || (type & SCSI_PROT_DIF_TYPE2))
> -		tc->ref_tag_seed_verify = scsi_get_lba(scmd) & 0xffffffff;
> +		tc->ref_tag_seed_verify = lower_32_bits(scsi_get_sector(scmd));
>  	else if (type & SCSI_PROT_DIF_TYPE3)
>  		tc->ref_tag_seed_verify = 0;
>  
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 5/8] lpfc: Use scsi_get_sector() instead of scsi_get_lba()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. This patch does not change any functionality.
> 
> Cc: James Smart <james.smart@broadcom.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/lpfc/lpfc_scsi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index eefbb9b22798..8c6b25807f12 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -2963,7 +2963,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>  				"9059 BLKGRD: Guard Tag error in cmd"
>  				" 0x%x lba 0x%llx blk cnt 0x%x "
>  				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
> -				(unsigned long long)scsi_get_lba(cmd),
> +				(u64)scsi_get_sector(cmd),

Why the cast ? scsi_get_sector() returns a 64bits sector_t.

>  				blk_rq_sectors(cmd->request), bgstat, bghm);
>  	}
>  
> @@ -2980,7 +2980,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>  				"9060 BLKGRD: Ref Tag error in cmd"
>  				" 0x%x lba 0x%llx blk cnt 0x%x "
>  				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
> -				(unsigned long long)scsi_get_lba(cmd),
> +				(u64)scsi_get_sector(cmd),
>  				blk_rq_sectors(cmd->request), bgstat, bghm);
>  	}
>  
> @@ -2997,7 +2997,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>  				"9062 BLKGRD: App Tag error in cmd"
>  				" 0x%x lba 0x%llx blk cnt 0x%x "
>  				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
> -				(unsigned long long)scsi_get_lba(cmd),
> +				(u64)scsi_get_sector(cmd),
>  				blk_rq_sectors(cmd->request), bgstat, bghm);
>  	}
>  
> @@ -3028,7 +3028,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>  			break;
>  		}
>  
> -		failing_sector = scsi_get_lba(cmd);
> +		failing_sector = scsi_get_sector(cmd);
>  		failing_sector += bghm;
>  
>  		/* Descriptor Information */
> @@ -3041,7 +3041,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>  				"9068 BLKGRD: Unknown error in cmd"
>  				" 0x%x lba 0x%llx blk cnt 0x%x "
>  				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
> -				(unsigned long long)scsi_get_lba(cmd),
> +				(u64)scsi_get_sector(cmd),
>  				blk_rq_sectors(cmd->request), bgstat, bghm);
>  
>  		/* Calcuate what type of error it was */
> @@ -3174,7 +3174,7 @@ lpfc_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>  			break;
>  		}
>  
> -		failing_sector = scsi_get_lba(cmd);
> +		failing_sector = scsi_get_sector(cmd);
>  		failing_sector += bghm;
>  
>  		/* Descriptor Information */
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 6/8] qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. Additionally, use lower_32_bits() instead of
> open-coding it. This patch does not change any functionality.
> 
> Cc: Nilesh Javali <njavali@marvell.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/qla2xxx/qla_iocb.c | 9 +++------
>  drivers/scsi/qla2xxx/qla_isr.c  | 8 ++++----
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index 38b5bdde2405..28e30a7e8883 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> @@ -778,8 +778,7 @@ qla24xx_set_t10dif_tags(srb_t *sp, struct fw_dif_context *pkt,
>  		 * No check for ql2xenablehba_err_chk, as it would be an
>  		 * I/O error if hba tag generation is not done.
>  		 */
> -		pkt->ref_tag = cpu_to_le32((uint32_t)
> -		    (0xffffffff & scsi_get_lba(cmd)));
> +		pkt->ref_tag = cpu_to_le32(lower_32_bits(scsi_get_sector(cmd)));
>  
>  		if (!qla2x00_hba_err_chk_enabled(sp))
>  			break;
> @@ -799,8 +798,7 @@ qla24xx_set_t10dif_tags(srb_t *sp, struct fw_dif_context *pkt,
>  		pkt->app_tag_mask[0] = 0x0;
>  		pkt->app_tag_mask[1] = 0x0;
>  
> -		pkt->ref_tag = cpu_to_le32((uint32_t)
> -		    (0xffffffff & scsi_get_lba(cmd)));
> +		pkt->ref_tag = cpu_to_le32(lower_32_bits(scsi_get_sector(cmd)));
>  
>  		if (!qla2x00_hba_err_chk_enabled(sp))
>  			break;
> @@ -824,8 +822,7 @@ qla24xx_set_t10dif_tags(srb_t *sp, struct fw_dif_context *pkt,
>  	 * 16 bit app tag.
>  	 */
>  	case SCSI_PROT_DIF_TYPE1:
> -		pkt->ref_tag = cpu_to_le32((uint32_t)
> -		    (0xffffffff & scsi_get_lba(cmd)));
> +		pkt->ref_tag = cpu_to_le32(lower_32_bits(scsi_get_sector(cmd)));
>  		pkt->app_tag = cpu_to_le16(0);
>  		pkt->app_tag_mask[0] = 0x0;
>  		pkt->app_tag_mask[1] = 0x0;
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 67229af4c142..24d406411f81 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -2632,7 +2632,7 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
>  	    "DIF ERROR in cmd 0x%x lba 0x%llx act ref"
>  	    " tag=0x%x, exp ref_tag=0x%x, act app tag=0x%x, exp app"
>  	    " tag=0x%x, act guard=0x%x, exp guard=0x%x.\n",
> -	    cmd->cmnd[0], (u64)scsi_get_lba(cmd), a_ref_tag, e_ref_tag,
> +	    cmd->cmnd[0], (u64)scsi_get_sector(cmd), a_ref_tag, e_ref_tag,
>  	    a_app_tag, e_app_tag, a_guard, e_guard);
>  
>  	/*
> @@ -2644,10 +2644,10 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
>  	    (scsi_get_prot_type(cmd) != SCSI_PROT_DIF_TYPE3 ||
>  	     a_ref_tag == be32_to_cpu(T10_PI_REF_ESCAPE))) {
>  		uint32_t blocks_done, resid;
> -		sector_t lba_s = scsi_get_lba(cmd);
> +		sector_t sector = scsi_get_sector(cmd);
>  
>  		/* 2TB boundary case covered automatically with this */
> -		blocks_done = e_ref_tag - (uint32_t)lba_s + 1;
> +		blocks_done = e_ref_tag - (uint32_t)sector + 1;
>  
>  		resid = scsi_bufflen(cmd) - (blocks_done *
>  		    cmd->device->sector_size);
> @@ -2677,7 +2677,7 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
>  			if (k != blocks_done) {
>  				ql_log(ql_log_warn, vha, 0x302f,
>  				    "unexpected tag values tag:lba=%x:%llx)\n",
> -				    e_ref_tag, (unsigned long long)lba_s);
> +				    e_ref_tag, (u64)sector);
>  				return 1;
>  			}
>  
> 

Not entirely convinced the casts are needed for the log calls...
Apart from that, looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 7/8] ufs: Fix the tracing code

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. Use scsi_get_sector() for all SCSI requests since all
> SCSI requests have a block layer request attached and hence calling
> scsi_get_sector() is allowed. Convert the scsi_get_sector() result
> from sector_t into an LBA with sectors_to_logical(). Since READ(10),
> WRITE(10), READ(16) and WRITE(16) all have a GROUP NUMBER field, extract
> the GROUP NUMBER field for all four SCSI commands. Apply the 0x3f mask to
> that field since the upper two bits are reserved. Rename the 'transfer_len'
> variable into 'affected_bytes' since it represents the number of bytes
> affected on the storage medium instead of the size of the SCSI data buffer.

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 8/8] core: Remove scsi_get_lba()

[Damien Le Moal]

On 2021/05/14 7:38, Bart Van Assche wrote:
> Remove scsi_get_lba() since all callers have been converted to
> scsi_get_sector().
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  include/scsi/scsi_cmnd.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 7f55faa70301..77a0916c8769 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -284,11 +284,6 @@ static inline unsigned char scsi_get_prot_type(struct scsi_cmnd *scmd)
>  	return scmd->prot_type;
>  }
>  
> -static inline sector_t scsi_get_lba(struct scsi_cmnd *scmd)
> -{
> -	return blk_rq_pos(scmd->request);
> -}
> -
>  static inline sector_t scsi_get_sector(struct scsi_cmnd *scmd)
>  {
>  	return blk_rq_pos(scmd->request);
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 5/8] lpfc: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

On 5/13/21 7:00 PM, Damien Le Moal wrote:
> On 2021/05/14 7:38, Bart Van Assche wrote:
>> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
>> index eefbb9b22798..8c6b25807f12 100644
>> --- a/drivers/scsi/lpfc/lpfc_scsi.c
>> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
>> @@ -2963,7 +2963,7 @@ lpfc_sli4_parse_bg_err(struct lpfc_hba *phba, struct lpfc_io_buf *lpfc_cmd,
>>  				"9059 BLKGRD: Guard Tag error in cmd"
>>  				" 0x%x lba 0x%llx blk cnt 0x%x "
>>  				"bgstat=x%x bghm=x%x\n", cmd->cmnd[0],
>> -				(unsigned long long)scsi_get_lba(cmd),
>> +				(u64)scsi_get_sector(cmd),
> 
> Why the cast ? scsi_get_sector() returns a 64bits sector_t.

Right, the cast can be left out. Commit 72deb455b5ec ("block: remove
CONFIG_LBDAF") changed sector_t from sometimes 32-bit / sometimes 64-bit
into always 64-bit.

Bart.

Re: [PATCH v3 6/8] qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()

[Bart Van Assche]

On 5/13/21 7:02 PM, Damien Le Moal wrote:
> On 2021/05/14 7:38, Bart Van Assche wrote:
>> @@ -2677,7 +2677,7 @@ qla2x00_handle_dif_error(srb_t *sp, struct sts_entry_24xx *sts24)
>>  			if (k != blocks_done) {
>>  				ql_log(ql_log_warn, vha, 0x302f,
>>  				    "unexpected tag values tag:lba=%x:%llx)\n",
>> -				    e_ref_tag, (unsigned long long)lba_s);
>> +				    e_ref_tag, (u64)sector);
>>  				return 1;
>>  			}
>>  
>>
> 
> Not entirely convinced the casts are needed for the log calls...
> Apart from that, looks good.
> 
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

Hi Damien,

I will remove the cast from the ql_log() call.

Thanks,

Bart.

Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()

[Christoph Hellwig]

On Thu, May 13, 2021 at 03:37:50PM -0700, Bart Van Assche wrote:
> Since scsi_get_lba() returns a sector_t value instead of the LBA, the name
> of that function is confusing. Introduce an identical function
> scsi_get_sector(). A later patch will remove scsi_get_lba().

Why not just do a quick rename in a single patch instead of the hard to
review series?

Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()

[Bart Van Assche]

On 5/13/21 11:20 PM, Christoph Hellwig wrote:
> On Thu, May 13, 2021 at 03:37:50PM -0700, Bart Van Assche wrote:
>> Since scsi_get_lba() returns a sector_t value instead of the LBA, the name
>> of that function is confusing. Introduce an identical function
>> scsi_get_sector(). A later patch will remove scsi_get_lba().
> 
> Why not just do a quick rename in a single patch instead of the hard to
> review series?

Hi Christoph,

Something I noticed in the past is that driver maintainers refuse to add
their Reviewed-by: or Acked-by: if a patch modifies code outside the
driver they maintain. Another reason I split this patch series is that I
really want driver maintainers to take a look. My understanding of
section "4.19 Protection Information" in SBC-4 is that the LOGICAL BLOCK
REFERENCE TAG field should contain the least significant four bytes of
the LBA. However, in multiple SCSI LLDs I found code similar to the
following:

	ref_tag = cpu_to_le32(lower_32_bits(scsi_get_lba(cmd)))

or in other words, the starting offset divided by 512 is assigned to the
reference tag instead of the starting offset divided by the logical
block size. I think that's a bug.

Thanks,

Bart.

Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()

[Martin K. Petersen]

Bart,

> or in other words, the starting offset divided by 512 is assigned to
> the reference tag instead of the starting offset divided by the
> logical block size. I think that's a bug.

It is. And I have most of these fixed in my integrity branch that
reworks how LLDs interact with the SCSI midlayer. I've only done
mpt3sas, qla2xxx, and lpfc because that's what I test with. I.e. I
missed zfcp and iser.

I will review your series shortly. But since all the integrity stuff is
transitioning to the t10_pi_foo* interfaces, my initial hunch is that we
probably just want to get rid of scsi_get_lba() completely and use
blk_rq_pos() directly for the places where we want block layer
addressing. I'm not a big fan of _pos() and like _sector() much
better. But I do think that the blk_ prefix makes it clear that we're
referring to sectors and not logical blocks, physical blocks, or
protection intervals.

Anyway. More in an hour or so...

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v3 3/8] zfcp: Use scsi_get_sector() instead of scsi_get_lba()

[Benjamin Block]

On Thu, May 13, 2021 at 03:37:52PM -0700, Bart Van Assche wrote:
> Use scsi_get_sector() instead of scsi_get_lba() since the name of the
> latter is confusing. Additionally, use lower_32_bits() instead of
> open-coding it. This patch does not change any functionality.
> 
> Cc: Benjamin Block <bblock@linux.ibm.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/s390/scsi/zfcp_fsf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
> index 2e4804ef2fb9..3d9a3dc4975b 100644
> --- a/drivers/s390/scsi/zfcp_fsf.c
> +++ b/drivers/s390/scsi/zfcp_fsf.c
> @@ -2600,7 +2600,7 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
>  
>  	if (scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) {
>  		io->data_block_length = scsi_cmnd->device->sector_size;
> -		io->ref_tag_value = scsi_get_lba(scsi_cmnd) & 0xFFFFFFFF;
> +		io->ref_tag_value = lower_32_bits(scsi_get_sector(scsi_cmnd));
>  	}
>  
>  	if (zfcp_fsf_set_data_dir(scsi_cmnd, &io->data_direction))

Looks good.

Acked-by: Benjamin Block <bblock@linux.ibm.com>

-- 
Best Regards, Benjamin Block  / Linux on IBM Z Kernel Development / IBM Systems
IBM Deutschland Research & Development GmbH    /    https://www.ibm.com/privacy
Vorsitz. AufsR.: Gregor Pillen         /        Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294