From c78be80d20cd52c302b92640550087ede9c4304a Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Tue, 8 Jun 2021 23:39:22 -0400 Subject: scsi: scsi_debug: Remove dump_sector() The function used to dump sectors containing protection information errors was useful during initial development over a decade ago. However, dump_sector() substantially slows down the system during testing due to writing an entire sector's worth of data to syslog on every error. We now log plenty of information about the nature of detected protection information errors throughout the stack. Dumping the entire contents of an offending sector is no longer needed. Link: https://lore.kernel.org/r/20210609033929.3815-9-martin.petersen@oracle.com Reviewed-by: Bart Van Assche Reviewed-by: Douglas Gilbert Signed-off-by: Martin K. Petersen Message-Id: <20210609033929.3815-9-martin.petersen@oracle.com> --- drivers/scsi/scsi_debug.c | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 5b3a20a140f9..9033ab4911ba 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -3232,28 +3232,6 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) return 0; } -static void dump_sector(unsigned char *buf, int len) -{ - int i, j, n; - - pr_err(">>> Sector Dump <<<\n"); - for (i = 0 ; i < len ; i += 16) { - char b[128]; - - for (j = 0, n = 0; j < 16; j++) { - unsigned char c = buf[i+j]; - - if (c >= 0x20 && c < 0x7e) - n += scnprintf(b + n, sizeof(b) - n, - " %c ", buf[i+j]); - else - n += scnprintf(b + n, sizeof(b) - n, - "%02x ", buf[i+j]); - } - pr_err("%04d: %s\n", i, b); - } -} - static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, unsigned int sectors, u32 ei_lba) { @@ -3300,10 +3278,8 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, daddr = diter.addr + dpage_offset; ret = dif_verify(sdt, daddr, sector, ei_lba); - if (ret) { - dump_sector(daddr, sdebug_sector_size); + if (ret) goto out; - } sector++; ei_lba++; -- cgit v1.2.3 From f7be677227a5375cefd084df2c88864fc673e24a Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Tue, 8 Jun 2021 23:39:23 -0400 Subject: scsi: scsi_debug: Improve RDPROTECT/WRPROTECT handling It is useful for testing purposes to be able to inject errors by writing bad protection information to media with checking disabled and then attempting to read it back. Extend scsi_debug's PI verification logic to give the driver feature parity with commercially available drives. Almost all devices with PI capability support RDPROTECT and WRPROTECT values of 0, 1, and 3. Link: https://lore.kernel.org/r/20210609033929.3815-10-martin.petersen@oracle.com Reviewed-by: Douglas Gilbert Signed-off-by: Martin K. Petersen Message-Id: <20210609033929.3815-10-martin.petersen@oracle.com> --- drivers/scsi/scsi_debug.c | 90 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 67 insertions(+), 23 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 9033ab4911ba..25112b15ab14 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -3076,6 +3076,7 @@ static void dif_copy_prot(struct scsi_cmnd *scp, sector_t sector, static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec, unsigned int sectors, u32 ei_lba) { + int ret = 0; unsigned int i; sector_t sector; struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *) @@ -3083,26 +3084,33 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec, struct t10_pi_tuple *sdt; for (i = 0; i < sectors; i++, ei_lba++) { - int ret; - sector = start_sec + i; sdt = dif_store(sip, sector); if (sdt->app_tag == cpu_to_be16(0xffff)) continue; - ret = dif_verify(sdt, lba2fake_store(sip, sector), sector, - ei_lba); - if (ret) { - dif_errors++; - return ret; + /* + * Because scsi_debug acts as both initiator and + * target we proceed to verify the PI even if + * RDPROTECT=3. This is done so the "initiator" knows + * which type of error to return. Otherwise we would + * have to iterate over the PI twice. + */ + if (scp->cmnd[1] >> 5) { /* RDPROTECT */ + ret = dif_verify(sdt, lba2fake_store(sip, sector), + sector, ei_lba); + if (ret) { + dif_errors++; + break; + } } } dif_copy_prot(scp, start_sec, sectors, true); dix_reads++; - return 0; + return ret; } static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) @@ -3196,12 +3204,29 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) /* DIX + T10 DIF */ if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) { - int prot_ret = prot_verify_read(scp, lba, num, ei_lba); - - if (prot_ret) { - read_unlock(macc_lckp); - mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, prot_ret); - return illegal_condition_result; + switch (prot_verify_read(scp, lba, num, ei_lba)) { + case 1: /* Guard tag error */ + if (cmd[1] >> 5 != 3) { /* RDPROTECT != 3 */ + read_unlock(macc_lckp); + mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1); + return check_condition_result; + } else if (scp->prot_flags & SCSI_PROT_GUARD_CHECK) { + read_unlock(macc_lckp); + mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1); + return illegal_condition_result; + } + break; + case 3: /* Reference tag error */ + if (cmd[1] >> 5 != 3) { /* RDPROTECT != 3 */ + read_unlock(macc_lckp); + mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 3); + return check_condition_result; + } else if (scp->prot_flags & SCSI_PROT_REF_CHECK) { + read_unlock(macc_lckp); + mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 3); + return illegal_condition_result; + } + break; } } @@ -3277,9 +3302,11 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, sdt = piter.addr + ppage_offset; daddr = diter.addr + dpage_offset; - ret = dif_verify(sdt, daddr, sector, ei_lba); - if (ret) - goto out; + if (SCpnt->cmnd[1] >> 5 != 3) { /* WRPROTECT */ + ret = dif_verify(sdt, daddr, sector, ei_lba); + if (ret) + goto out; + } sector++; ei_lba++; @@ -3456,12 +3483,29 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) /* DIX + T10 DIF */ if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) { - int prot_ret = prot_verify_write(scp, lba, num, ei_lba); - - if (prot_ret) { - write_unlock(macc_lckp); - mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, prot_ret); - return illegal_condition_result; + switch (prot_verify_write(scp, lba, num, ei_lba)) { + case 1: /* Guard tag error */ + if (scp->prot_flags & SCSI_PROT_GUARD_CHECK) { + write_unlock(macc_lckp); + mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1); + return illegal_condition_result; + } else if (scp->cmnd[1] >> 5 != 3) { /* WRPROTECT != 3 */ + write_unlock(macc_lckp); + mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1); + return check_condition_result; + } + break; + case 3: /* Reference tag error */ + if (scp->prot_flags & SCSI_PROT_REF_CHECK) { + write_unlock(macc_lckp); + mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 3); + return illegal_condition_result; + } else if (scp->cmnd[1] >> 5 != 3) { /* WRPROTECT != 3 */ + write_unlock(macc_lckp); + mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 3); + return check_condition_result; + } + break; } } -- cgit v1.2.3 From a6e76e6f2c0efd9e2cf8cf93f532c4d46070e5c5 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 9 Aug 2021 16:03:44 -0700 Subject: scsi: scsi_debug: Use scsi_cmd_to_rq() instead of scsi_cmnd.request Prepare for removal of the request pointer by using scsi_cmd_to_rq() instead. This patch does not change any functionality. Link: https://lore.kernel.org/r/20210809230355.8186-42-bvanassche@acm.org Acked-by: Douglas Gilbert Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/scsi_debug.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 25112b15ab14..31529d8add0d 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -4722,7 +4722,7 @@ fini: static struct sdebug_queue *get_queue(struct scsi_cmnd *cmnd) { u16 hwq; - u32 tag = blk_mq_unique_tag(cmnd->request); + u32 tag = blk_mq_unique_tag(scsi_cmd_to_rq(cmnd)); hwq = blk_mq_unique_tag_to_hwq(tag); @@ -4735,7 +4735,7 @@ static struct sdebug_queue *get_queue(struct scsi_cmnd *cmnd) static u32 get_tag(struct scsi_cmnd *cmnd) { - return blk_mq_unique_tag(cmnd->request); + return blk_mq_unique_tag(scsi_cmd_to_rq(cmnd)); } /* Queued (deferred) command completions converge here. */ @@ -5384,7 +5384,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, { bool new_sd_dp; bool inject = false; - bool hipri = (cmnd->request->cmd_flags & REQ_HIPRI); + bool hipri = scsi_cmd_to_rq(cmnd)->cmd_flags & REQ_HIPRI; int k, num_in_q, qdepth; unsigned long iflags; u64 ns_from_boot = 0; @@ -5587,8 +5587,9 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, if (sdebug_statistics) sd_dp->issuing_cpu = raw_smp_processor_id(); if (unlikely(sd_dp->aborted)) { - sdev_printk(KERN_INFO, sdp, "abort request tag %d\n", cmnd->request->tag); - blk_abort_request(cmnd->request); + sdev_printk(KERN_INFO, sdp, "abort request tag %d\n", + scsi_cmd_to_rq(cmnd)->tag); + blk_abort_request(scsi_cmd_to_rq(cmnd)); atomic_set(&sdeb_inject_pending, 0); sd_dp->aborted = false; } @@ -7414,7 +7415,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost, (u32)cmd[k]); } sdev_printk(KERN_INFO, sdp, "%s: tag=%#x, cmd %s\n", my_name, - blk_mq_unique_tag(scp->request), b); + blk_mq_unique_tag(scsi_cmd_to_rq(scp)), b); } if (unlikely(inject_now && (sdebug_opts & SDEBUG_OPT_HOST_BUSY))) return SCSI_MLQUEUE_HOST_BUSY; -- cgit v1.2.3