From e38d9e83a376923454af599b2add53e71cd7508a Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 2 Mar 2022 00:35:51 -0500 Subject: scsi: sd: Use cached ATA Information VPD page Since the ATA Information VPD is now cached at device discovery time it is no longer necessary to request this page when we configure WRITE SAME. Instead use the cached information to determine if this disk sits behind a SCSI-ATA translation layer. Link: https://lore.kernel.org/r/20220302053559.32147-7-martin.petersen@oracle.com Reviewed-by: Christoph Hellwig Reviewed-by: Bart Van Assche Reviewed-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index dc6e55761fd1..027738af5a22 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2996,8 +2996,7 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer) } if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) { - /* too large values might cause issues with arcmsr */ - int vpd_buf_len = 64; + struct scsi_vpd *vpd; sdev->no_report_opcodes = 1; @@ -3005,8 +3004,11 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer) * CODES is unsupported and the device has an ATA * Information VPD page (SAT). */ - if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) + rcu_read_lock(); + vpd = rcu_dereference(sdev->vpd_pg89); + if (vpd) sdev->no_write_same = 1; + rcu_read_unlock(); } if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME_16) == 1) -- cgit v1.2.3 From 7fb019c46eeea4e3cc3ddfd3e01a24e610f34fac Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 2 Mar 2022 00:35:52 -0500 Subject: scsi: sd: Switch to using scsi_device VPD pages Use the VPD pages already provided by the SCSI midlayer. No need to request them individually in the SCSI disk driver. Link: https://lore.kernel.org/r/20220302053559.32147-8-martin.petersen@oracle.com Reviewed-by: Christoph Hellwig Reviewed-by: Bart Van Assche Reviewed-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 80 +++++++++++++++++++++++++++---------------------------- 1 file changed, 40 insertions(+), 40 deletions(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 027738af5a22..1e580e9e4a37 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2844,39 +2844,39 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer) static void sd_read_block_limits(struct scsi_disk *sdkp) { unsigned int sector_sz = sdkp->device->sector_size; - const int vpd_len = 64; - unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL); + struct scsi_vpd *vpd; - if (!buffer || - /* Block Limits VPD */ - scsi_get_vpd_page(sdkp->device, 0xb0, buffer, vpd_len)) + rcu_read_lock(); + + vpd = rcu_dereference(sdkp->device->vpd_pgb0); + if (!vpd || vpd->len < 16) goto out; blk_queue_io_min(sdkp->disk->queue, - get_unaligned_be16(&buffer[6]) * sector_sz); + get_unaligned_be16(&vpd->data[6]) * sector_sz); - sdkp->max_xfer_blocks = get_unaligned_be32(&buffer[8]); - sdkp->opt_xfer_blocks = get_unaligned_be32(&buffer[12]); + sdkp->max_xfer_blocks = get_unaligned_be32(&vpd->data[8]); + sdkp->opt_xfer_blocks = get_unaligned_be32(&vpd->data[12]); - if (buffer[3] == 0x3c) { + if (vpd->len >= 64) { unsigned int lba_count, desc_count; - sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]); + sdkp->max_ws_blocks = (u32)get_unaligned_be64(&vpd->data[36]); if (!sdkp->lbpme) goto out; - lba_count = get_unaligned_be32(&buffer[20]); - desc_count = get_unaligned_be32(&buffer[24]); + lba_count = get_unaligned_be32(&vpd->data[20]); + desc_count = get_unaligned_be32(&vpd->data[24]); if (lba_count && desc_count) sdkp->max_unmap_blocks = lba_count; - sdkp->unmap_granularity = get_unaligned_be32(&buffer[28]); + sdkp->unmap_granularity = get_unaligned_be32(&vpd->data[28]); - if (buffer[32] & 0x80) + if (vpd->data[32] & 0x80) sdkp->unmap_alignment = - get_unaligned_be32(&buffer[32]) & ~(1 << 31); + get_unaligned_be32(&vpd->data[32]) & ~(1 << 31); if (!sdkp->lbpvpd) { /* LBP VPD page not provided */ @@ -2898,7 +2898,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) } out: - kfree(buffer); + rcu_read_unlock(); } /** @@ -2908,18 +2908,21 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) static void sd_read_block_characteristics(struct scsi_disk *sdkp) { struct request_queue *q = sdkp->disk->queue; - unsigned char *buffer; + struct scsi_vpd *vpd; u16 rot; - const int vpd_len = 64; + u8 zoned; - buffer = kmalloc(vpd_len, GFP_KERNEL); + rcu_read_lock(); + vpd = rcu_dereference(sdkp->device->vpd_pgb1); - if (!buffer || - /* Block Device Characteristics VPD */ - scsi_get_vpd_page(sdkp->device, 0xb1, buffer, vpd_len)) - goto out; + if (!vpd || vpd->len < 8) { + rcu_read_unlock(); + return; + } - rot = get_unaligned_be16(&buffer[4]); + rot = get_unaligned_be16(&vpd->data[4]); + zoned = (vpd->data[8] >> 4) & 3; + rcu_read_unlock(); if (rot == 1) { blk_queue_flag_set(QUEUE_FLAG_NONROT, q); @@ -2930,7 +2933,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) /* Host-managed */ blk_queue_set_zoned(sdkp->disk, BLK_ZONED_HM); } else { - sdkp->zoned = (buffer[8] >> 4) & 3; + sdkp->zoned = zoned; if (sdkp->zoned == 1) { /* Host-aware */ blk_queue_set_zoned(sdkp->disk, BLK_ZONED_HA); @@ -2941,7 +2944,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) } if (!sdkp->first_scan) - goto out; + return; if (blk_queue_is_zoned(q)) { sd_printk(KERN_NOTICE, sdkp, "Host-%s zoned block device\n", @@ -2954,9 +2957,6 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) sd_printk(KERN_NOTICE, sdkp, "Drive-managed SMR disk\n"); } - - out: - kfree(buffer); } /** @@ -2965,24 +2965,24 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) */ static void sd_read_block_provisioning(struct scsi_disk *sdkp) { - unsigned char *buffer; - const int vpd_len = 8; + struct scsi_vpd *vpd; if (sdkp->lbpme == 0) return; - buffer = kmalloc(vpd_len, GFP_KERNEL); + rcu_read_lock(); + vpd = rcu_dereference(sdkp->device->vpd_pgb2); - if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb2, buffer, vpd_len)) - goto out; + if (!vpd || vpd->len < 8) { + rcu_read_unlock(); + return; + } sdkp->lbpvpd = 1; - sdkp->lbpu = (buffer[5] >> 7) & 1; /* UNMAP */ - sdkp->lbpws = (buffer[5] >> 6) & 1; /* WRITE SAME(16) with UNMAP */ - sdkp->lbpws10 = (buffer[5] >> 5) & 1; /* WRITE SAME(10) with UNMAP */ - - out: - kfree(buffer); + sdkp->lbpu = (vpd->data[5] >> 7) & 1; /* UNMAP */ + sdkp->lbpws = (vpd->data[5] >> 6) & 1; /* WRITE SAME(16) w/ UNMAP */ + sdkp->lbpws10 = (vpd->data[5] >> 5) & 1; /* WRITE SAME(10) w/ UNMAP */ + rcu_read_unlock(); } static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer) -- cgit v1.2.3 From 631669a256f96c7115c34cc19578a461c72568de Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 2 Mar 2022 00:35:53 -0500 Subject: scsi: sd: Optimal I/O size should be a multiple of reported granularity Commit a83da8a4509d ("scsi: sd: Optimal I/O size should be a multiple of physical block size") validated the reported optimal I/O size against the physical block size to overcome problems with devices reporting nonsensical transfer sizes. However, some devices claim conformity to older SCSI versions that predate the physical block size being reported. Other devices do not report a physical block size at all. We need to be able to validate the optimal I/O size on those devices as well. Many devices report an OPTIMAL TRANSFER LENGTH GRANULARITY in the same VPD page as the OPTIMAL TRANSFER LENGTH. Use this value to validate the optimal I/O size. Also check that the reported granularity is a multiple of the physical block size, if supported. Link: https://lore.kernel.org/r/33fb522e-4f61-1b76-914f-c9e6a3553c9b@gmail.com Link: https://lore.kernel.org/r/20220302053559.32147-9-martin.petersen@oracle.com Reported-by: Bernhard Sulzer Reviewed-by: Christoph Hellwig Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 45 +++++++++++++++++++++++++++++++++++++++++---- drivers/scsi/sd.h | 1 + 2 files changed, 42 insertions(+), 4 deletions(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 1e580e9e4a37..e1c884db7fca 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2843,7 +2843,6 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer) */ static void sd_read_block_limits(struct scsi_disk *sdkp) { - unsigned int sector_sz = sdkp->device->sector_size; struct scsi_vpd *vpd; rcu_read_lock(); @@ -2852,9 +2851,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) if (!vpd || vpd->len < 16) goto out; - blk_queue_io_min(sdkp->disk->queue, - get_unaligned_be16(&vpd->data[6]) * sector_sz); - + sdkp->min_xfer_blocks = get_unaligned_be16(&vpd->data[6]); sdkp->max_xfer_blocks = get_unaligned_be32(&vpd->data[8]); sdkp->opt_xfer_blocks = get_unaligned_be32(&vpd->data[12]); @@ -3112,6 +3109,29 @@ out: kfree(buffer); } +static bool sd_validate_min_xfer_size(struct scsi_disk *sdkp) +{ + struct scsi_device *sdp = sdkp->device; + unsigned int min_xfer_bytes = + logical_to_bytes(sdp, sdkp->min_xfer_blocks); + + if (sdkp->min_xfer_blocks == 0) + return false; + + if (min_xfer_bytes & (sdkp->physical_block_size - 1)) { + sd_first_printk(KERN_WARNING, sdkp, + "Preferred minimum I/O size %u bytes not a " \ + "multiple of physical block size (%u bytes)\n", + min_xfer_bytes, sdkp->physical_block_size); + sdkp->min_xfer_blocks = 0; + return false; + } + + sd_first_printk(KERN_INFO, sdkp, "Preferred minimum I/O size %u bytes\n", + min_xfer_bytes); + return true; +} + /* * Determine the device's preferred I/O size for reads and writes * unless the reported value is unreasonably small, large, not a @@ -3123,6 +3143,8 @@ static bool sd_validate_opt_xfer_size(struct scsi_disk *sdkp, struct scsi_device *sdp = sdkp->device; unsigned int opt_xfer_bytes = logical_to_bytes(sdp, sdkp->opt_xfer_blocks); + unsigned int min_xfer_bytes = + logical_to_bytes(sdp, sdkp->min_xfer_blocks); if (sdkp->opt_xfer_blocks == 0) return false; @@ -3151,6 +3173,15 @@ static bool sd_validate_opt_xfer_size(struct scsi_disk *sdkp, return false; } + if (min_xfer_bytes && opt_xfer_bytes % min_xfer_bytes) { + sd_first_printk(KERN_WARNING, sdkp, + "Optimal transfer size %u bytes not a " \ + "multiple of preferred minimum block " \ + "size (%u bytes)\n", + opt_xfer_bytes, min_xfer_bytes); + return false; + } + if (opt_xfer_bytes & (sdkp->physical_block_size - 1)) { sd_first_printk(KERN_WARNING, sdkp, "Optimal transfer size %u bytes not a " \ @@ -3243,6 +3274,12 @@ static int sd_revalidate_disk(struct gendisk *disk) dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks); q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max); + if (sd_validate_min_xfer_size(sdkp)) + blk_queue_io_min(sdkp->disk->queue, + logical_to_bytes(sdp, sdkp->min_xfer_blocks)); + else + blk_queue_io_min(sdkp->disk->queue, 0); + if (sd_validate_opt_xfer_size(sdkp, dev_max)) { q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks); rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks); diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index d249933ba69e..2abad54fd23f 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -114,6 +114,7 @@ struct scsi_disk { atomic_t openers; sector_t capacity; /* size in logical blocks */ int max_retries; + u32 min_xfer_blocks; u32 max_xfer_blocks; u32 opt_xfer_blocks; u32 max_ws_blocks; -- cgit v1.2.3 From 1e029397d12f9085840691f4f12991a80d89888c Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 2 Mar 2022 00:35:58 -0500 Subject: scsi: sd: Reorganize DIF/DIX code to avoid calling revalidate twice During device discovery we ended up calling revalidate twice and thus requested the same parameters multiple times. This was originally necessary due to the request_queue and gendisk needing to be instantiated to configure the block integrity profile. Since this dependency no longer exists, reorganize the integrity probing code so it can be run once at the end of discovery and drop the superfluous revalidate call. Postponing the registration step involves splitting sd_read_protection() into two functions, one to read the device protection type and one to configure the mode of operation. As part of this cleanup, make the printing code a bit less verbose. Link: https://lore.kernel.org/r/20220302053559.32147-14-martin.petersen@oracle.com Reviewed-by: Christoph Hellwig Reviewed-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 62 +++++++++++++++++++++++++++------------------------ drivers/scsi/sd_dif.c | 8 +++---- 2 files changed, 36 insertions(+), 34 deletions(-) (limited to 'drivers/scsi/sd.c') diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index e1c884db7fca..c583301ff4ec 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2176,40 +2176,48 @@ static int sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer { struct scsi_device *sdp = sdkp->device; u8 type; - int ret = 0; if (scsi_device_protection(sdp) == 0 || (buffer[12] & 1) == 0) { sdkp->protection_type = 0; - return ret; + return 0; } type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */ - if (type > T10_PI_TYPE3_PROTECTION) - ret = -ENODEV; - else if (scsi_host_dif_capable(sdp->host, type)) - ret = 1; - - if (sdkp->first_scan || type != sdkp->protection_type) - switch (ret) { - case -ENODEV: - sd_printk(KERN_ERR, sdkp, "formatted with unsupported" \ - " protection type %u. Disabling disk!\n", - type); - break; - case 1: - sd_printk(KERN_NOTICE, sdkp, - "Enabling DIF Type %u protection\n", type); - break; - case 0: - sd_printk(KERN_NOTICE, sdkp, - "Disabling DIF Type %u protection\n", type); - break; - } + if (type > T10_PI_TYPE3_PROTECTION) { + sd_printk(KERN_ERR, sdkp, "formatted with unsupported" \ + " protection type %u. Disabling disk!\n", + type); + sdkp->protection_type = 0; + return -ENODEV; + } sdkp->protection_type = type; - return ret; + return 0; +} + +static void sd_config_protection(struct scsi_disk *sdkp) +{ + struct scsi_device *sdp = sdkp->device; + + if (!sdkp->first_scan) + return; + + sd_dif_config_host(sdkp); + + if (!sdkp->protection_type) + return; + + if (!scsi_host_dif_capable(sdp->host, sdkp->protection_type)) { + sd_printk(KERN_NOTICE, sdkp, + "Disabling DIF Type %u protection\n", + sdkp->protection_type); + sdkp->protection_type = 0; + } + + sd_printk(KERN_NOTICE, sdkp, "Enabling DIF Type %u protection\n", + sdkp->protection_type); } static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp, @@ -3259,6 +3267,7 @@ static int sd_revalidate_disk(struct gendisk *disk) sd_read_app_tag_own(sdkp, buffer); sd_read_write_same(sdkp, buffer); sd_read_security(sdkp, buffer); + sd_config_protection(sdkp); } /* @@ -3518,11 +3527,6 @@ static int sd_probe(struct device *dev) goto out; } - if (sdkp->capacity) - sd_dif_config_host(sdkp); - - sd_revalidate_disk(gd); - if (sdkp->security) { sdkp->opal_dev = init_opal_dev(sdkp, &sd_sec_submit); if (sdkp->opal_dev) diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c index 349950616adc..968993ee6d5d 100644 --- a/drivers/scsi/sd_dif.c +++ b/drivers/scsi/sd_dif.c @@ -59,8 +59,6 @@ void sd_dif_config_host(struct scsi_disk *sdkp) bi.profile = &t10_pi_type1_crc; bi.tuple_size = sizeof(struct t10_pi_tuple); - sd_printk(KERN_NOTICE, sdkp, - "Enabling DIX %s protection\n", bi.profile->name); if (dif && type) { bi.flags |= BLK_INTEGRITY_DEVICE_CAPABLE; @@ -72,11 +70,11 @@ void sd_dif_config_host(struct scsi_disk *sdkp) bi.tag_size = sizeof(u16) + sizeof(u32); else bi.tag_size = sizeof(u16); - - sd_printk(KERN_NOTICE, sdkp, "DIF application tag size %u\n", - bi.tag_size); } + sd_printk(KERN_NOTICE, sdkp, + "Enabling DIX %s, application tag size %u bytes\n", + bi.profile->name, bi.tag_size); out: blk_integrity_register(disk, &bi); } -- cgit v1.2.3