* [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector()
@ 2021-05-13 22:37 Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 1/8] core: Introduce scsi_get_sector() Bart Van Assche
` (7 more replies)
0 siblings, 8 replies; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, 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(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/8] core: Introduce scsi_get_sector()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 1:57 ` Damien Le Moal
2021-05-14 6:20 ` Christoph Hellwig
2021-05-13 22:37 ` [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba() Bart Van Assche
` (6 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Christoph Hellwig, Ming Lei, Hannes Reinecke
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;
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 1/8] core: Introduce scsi_get_sector() Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 1:57 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 3/8] zfcp: " Bart Van Assche
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Sagi Grimberg
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",
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 3/8] zfcp: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 1/8] core: Introduce scsi_get_sector() Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba() Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 1:58 ` Damien Le Moal
2021-05-17 17:23 ` Benjamin Block
2021-05-13 22:37 ` [PATCH v3 4/8] isci: " Bart Van Assche
` (4 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Benjamin Block
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))
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 4/8] isci: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
` (2 preceding siblings ...)
2021-05-13 22:37 ` [PATCH v3 3/8] zfcp: " Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 1:58 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 5/8] lpfc: " Bart Van Assche
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Lee Duncan, Artur Paszkiewicz
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;
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 5/8] lpfc: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
` (3 preceding siblings ...)
2021-05-13 22:37 ` [PATCH v3 4/8] isci: " Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 2:00 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 6/8] qla2xxx: " Bart Van Assche
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, James Smart
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 */
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 6/8] qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
` (4 preceding siblings ...)
2021-05-13 22:37 ` [PATCH v3 5/8] lpfc: " Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 2:02 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 7/8] ufs: Fix the tracing code Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 8/8] core: Remove scsi_get_lba() Bart Van Assche
7 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Nilesh Javali
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;
}
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 7/8] ufs: Fix the tracing code
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
` (5 preceding siblings ...)
2021-05-13 22:37 ` [PATCH v3 6/8] qla2xxx: " Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 2:05 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 8/8] core: Remove scsi_get_lba() Bart Van Assche
7 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Can Guo, Alim Akhtar, Avri Altman, Stanley Chu,
Bean Huo, Adrian Hunter
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
)
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 8/8] core: Remove scsi_get_lba()
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
` (6 preceding siblings ...)
2021-05-13 22:37 ` [PATCH v3 7/8] ufs: Fix the tracing code Bart Van Assche
@ 2021-05-13 22:37 ` Bart Van Assche
2021-05-14 2:06 ` Damien Le Moal
7 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Bart Van Assche, Christoph Hellwig, Ming Lei, Hannes Reinecke
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);
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()
2021-05-13 22:37 ` [PATCH v3 1/8] core: Introduce scsi_get_sector() Bart Van Assche
@ 2021-05-14 1:57 ` Damien Le Moal
2021-05-14 6:20 ` Christoph Hellwig
1 sibling, 0 replies; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 1:57 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Christoph Hellwig, Ming Lei, Hannes Reinecke
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba() Bart Van Assche
@ 2021-05-14 1:57 ` Damien Le Moal
0 siblings, 0 replies; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 1:57 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, Sagi Grimberg
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 3/8] zfcp: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 3/8] zfcp: " Bart Van Assche
@ 2021-05-14 1:58 ` Damien Le Moal
2021-05-17 17:23 ` Benjamin Block
1 sibling, 0 replies; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 1:58 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, Benjamin Block
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/8] isci: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 4/8] isci: " Bart Van Assche
@ 2021-05-14 1:58 ` Damien Le Moal
0 siblings, 0 replies; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 1:58 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, Lee Duncan,
Artur Paszkiewicz
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/8] lpfc: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 5/8] lpfc: " Bart Van Assche
@ 2021-05-14 2:00 ` Damien Le Moal
2021-05-14 2:58 ` Bart Van Assche
0 siblings, 1 reply; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 2:00 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, James Smart
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/8] qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 6/8] qla2xxx: " Bart Van Assche
@ 2021-05-14 2:02 ` Damien Le Moal
2021-05-14 3:01 ` Bart Van Assche
0 siblings, 1 reply; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 2:02 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, Nilesh Javali
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/8] ufs: Fix the tracing code
2021-05-13 22:37 ` [PATCH v3 7/8] ufs: Fix the tracing code Bart Van Assche
@ 2021-05-14 2:05 ` Damien Le Moal
0 siblings, 0 replies; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 2:05 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, Can Guo,
Alim Akhtar, Avri Altman, Stanley Chu, Bean Huo, Adrian Hunter
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 8/8] core: Remove scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 8/8] core: Remove scsi_get_lba() Bart Van Assche
@ 2021-05-14 2:06 ` Damien Le Moal
0 siblings, 0 replies; 23+ messages in thread
From: Damien Le Moal @ 2021-05-14 2:06 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi,
Christoph Hellwig, Ming Lei, Hannes Reinecke
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/8] lpfc: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-14 2:00 ` Damien Le Moal
@ 2021-05-14 2:58 ` Bart Van Assche
0 siblings, 0 replies; 23+ messages in thread
From: Bart Van Assche @ 2021-05-14 2:58 UTC (permalink / raw)
To: Damien Le Moal, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, James Smart
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/8] qla2xxx: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-14 2:02 ` Damien Le Moal
@ 2021-05-14 3:01 ` Bart Van Assche
0 siblings, 0 replies; 23+ messages in thread
From: Bart Van Assche @ 2021-05-14 3:01 UTC (permalink / raw)
To: Damien Le Moal, Martin K . Petersen
Cc: James E . J . Bottomley, Jaegeuk Kim, linux-scsi, Nilesh Javali
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()
2021-05-13 22:37 ` [PATCH v3 1/8] core: Introduce scsi_get_sector() Bart Van Assche
2021-05-14 1:57 ` Damien Le Moal
@ 2021-05-14 6:20 ` Christoph Hellwig
2021-05-14 16:06 ` Bart Van Assche
1 sibling, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2021-05-14 6:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, Jaegeuk Kim,
linux-scsi, Christoph Hellwig, Ming Lei, Hannes Reinecke
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?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()
2021-05-14 6:20 ` Christoph Hellwig
@ 2021-05-14 16:06 ` Bart Van Assche
2021-05-14 17:10 ` Martin K. Petersen
0 siblings, 1 reply; 23+ messages in thread
From: Bart Van Assche @ 2021-05-14 16:06 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K . Petersen, James E . J . Bottomley, Jaegeuk Kim,
linux-scsi, Ming Lei, Hannes Reinecke
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/8] core: Introduce scsi_get_sector()
2021-05-14 16:06 ` Bart Van Assche
@ 2021-05-14 17:10 ` Martin K. Petersen
0 siblings, 0 replies; 23+ messages in thread
From: Martin K. Petersen @ 2021-05-14 17:10 UTC (permalink / raw)
To: Bart Van Assche
Cc: Christoph Hellwig, Martin K . Petersen, James E . J . Bottomley,
Jaegeuk Kim, linux-scsi, Ming Lei, Hannes Reinecke
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 3/8] zfcp: Use scsi_get_sector() instead of scsi_get_lba()
2021-05-13 22:37 ` [PATCH v3 3/8] zfcp: " Bart Van Assche
2021-05-14 1:58 ` Damien Le Moal
@ 2021-05-17 17:23 ` Benjamin Block
1 sibling, 0 replies; 23+ messages in thread
From: Benjamin Block @ 2021-05-17 17:23 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, Jaegeuk Kim, linux-scsi
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
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-05-17 17:23 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-13 22:37 [PATCH v3 0/8] Rename scsi_get_lba() into scsi_get_sector() Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 1/8] core: Introduce scsi_get_sector() Bart Van Assche
2021-05-14 1:57 ` Damien Le Moal
2021-05-14 6:20 ` Christoph Hellwig
2021-05-14 16:06 ` Bart Van Assche
2021-05-14 17:10 ` Martin K. Petersen
2021-05-13 22:37 ` [PATCH v3 2/8] iser: Use scsi_get_sector() instead of scsi_get_lba() Bart Van Assche
2021-05-14 1:57 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 3/8] zfcp: " Bart Van Assche
2021-05-14 1:58 ` Damien Le Moal
2021-05-17 17:23 ` Benjamin Block
2021-05-13 22:37 ` [PATCH v3 4/8] isci: " Bart Van Assche
2021-05-14 1:58 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 5/8] lpfc: " Bart Van Assche
2021-05-14 2:00 ` Damien Le Moal
2021-05-14 2:58 ` Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 6/8] qla2xxx: " Bart Van Assche
2021-05-14 2:02 ` Damien Le Moal
2021-05-14 3:01 ` Bart Van Assche
2021-05-13 22:37 ` [PATCH v3 7/8] ufs: Fix the tracing code Bart Van Assche
2021-05-14 2:05 ` Damien Le Moal
2021-05-13 22:37 ` [PATCH v3 8/8] core: Remove scsi_get_lba() Bart Van Assche
2021-05-14 2:06 ` Damien Le Moal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.