diff mbox series

[13/14] scsi: sd: Reorganize DIF/DIX code to avoid calling revalidate twice

Message ID 20220302053559.32147-14-martin.petersen@oracle.com (mailing list archive)
State Changes Requested
Headers show
Series [01/14] scsi: mpt3sas: Use cached ATA Information VPD page | expand

Commit Message

Martin K. Petersen March 2, 2022, 5:35 a.m. UTC
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.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c     | 62 +++++++++++++++++++++++--------------------
 drivers/scsi/sd_dif.c |  8 +++---
 2 files changed, 36 insertions(+), 34 deletions(-)

Comments

Christoph Hellwig March 2, 2022, 9:57 a.m. UTC | #1
Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Johannes Thumshirn March 4, 2022, 9:36 a.m. UTC | #2
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
diff mbox series

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 9d6b2205339d..163697dd799a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2338,40 +2338,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,
@@ -3409,6 +3417,7 @@  static int sd_revalidate_disk(struct gendisk *disk)
 		sd_config_write_same(sdkp);
 		sd_config_discard(sdkp, SD_LBP_DEFAULT);
 		sd_config_write_zeroes(sdkp, SD_ZERO_DEFAULT);
+		sd_config_protection(sdkp);
 	}
 
 	/*
@@ -3667,11 +3676,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);
 }