linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* T10 PI bugfixes
@ 2012-02-21  1:20 Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 1/3] sd: Avoid remapping bad reference tags Martin K. Petersen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Martin K. Petersen @ 2012-02-21  1:20 UTC (permalink / raw)
  To: linux-scsi; +Cc: James.Bottomley


This is a repost of the DIF patches I sent out a while back. The only
change is converting to from simple_stroul() to kstrouint() in patch 2.

-- 
Martin K. Petersen	Oracle Linux Engineering


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] sd: Avoid remapping bad reference tags
  2012-02-21  1:20 T10 PI bugfixes Martin K. Petersen
@ 2012-02-21  1:20 ` Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 2/3] sd: Allow protection_type to be overridden Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 3/3] scsi: Disable DIF on Hitachi Ultrastar 15K300 Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2012-02-21  1:20 UTC (permalink / raw)
  To: linux-scsi; +Cc: James.Bottomley, Martin K. Petersen

From: "Martin K. Petersen" <martin.petersen@oracle.com>

It does not make sense to translate ref tags with unexpected values.
Instead we simply ignore them and let the upper layers catch the
problem. Ref tags that contain the expected value are still remapped.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c     |    5 ++---
 drivers/scsi/sd.h     |    2 +-
 drivers/scsi/sd_dif.c |   25 ++++++-------------------
 3 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index bd17cf8..77822b9 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -803,9 +803,8 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 		SCpnt->cmnd[0] = WRITE_6;
 		SCpnt->sc_data_direction = DMA_TO_DEVICE;
 
-		if (blk_integrity_rq(rq) &&
-		    sd_dif_prepare(rq, block, sdp->sector_size) == -EIO)
-			goto out;
+		if (blk_integrity_rq(rq))
+			sd_dif_prepare(rq, block, sdp->sector_size);
 
 	} else if (rq_data_dir(rq) == READ) {
 		SCpnt->cmnd[0] = READ_6;
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index f703f48..47c52a6 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -156,7 +156,7 @@ struct sd_dif_tuple {
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 
 extern void sd_dif_config_host(struct scsi_disk *);
-extern int sd_dif_prepare(struct request *rq, sector_t, unsigned int);
+extern void sd_dif_prepare(struct request *rq, sector_t, unsigned int);
 extern void sd_dif_complete(struct scsi_cmnd *, unsigned int);
 
 #else /* CONFIG_BLK_DEV_INTEGRITY */
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 0cb39ff..1c47fa4 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -366,7 +366,8 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
  *
  * Type 3 does not have a reference tag so no remapping is required.
  */
-int sd_dif_prepare(struct request *rq, sector_t hw_sector, unsigned int sector_sz)
+void sd_dif_prepare(struct request *rq, sector_t hw_sector,
+		    unsigned int sector_sz)
 {
 	const int tuple_sz = sizeof(struct sd_dif_tuple);
 	struct bio *bio;
@@ -378,7 +379,7 @@ int sd_dif_prepare(struct request *rq, sector_t hw_sector, unsigned int sector_s
 	sdkp = rq->bio->bi_bdev->bd_disk->private_data;
 
 	if (sdkp->protection_type == SD_DIF_TYPE3_PROTECTION)
-		return 0;
+		return;
 
 	phys = hw_sector & 0xffffffff;
 
@@ -397,10 +398,9 @@ int sd_dif_prepare(struct request *rq, sector_t hw_sector, unsigned int sector_s
 
 			for (j = 0 ; j < iv->bv_len ; j += tuple_sz, sdt++) {
 
-				if (be32_to_cpu(sdt->ref_tag) != virt)
-					goto error;
+				if (be32_to_cpu(sdt->ref_tag) == virt)
+					sdt->ref_tag = cpu_to_be32(phys);
 
-				sdt->ref_tag = cpu_to_be32(phys);
 				virt++;
 				phys++;
 			}
@@ -410,16 +410,6 @@ int sd_dif_prepare(struct request *rq, sector_t hw_sector, unsigned int sector_s
 
 		bio->bi_flags |= BIO_MAPPED_INTEGRITY;
 	}
-
-	return 0;
-
-error:
-	kunmap_atomic(sdt, KM_USER0);
-	sd_printk(KERN_ERR, sdkp, "%s: virt %u, phys %u, ref %u, app %4x\n",
-		  __func__, virt, phys, be32_to_cpu(sdt->ref_tag),
-		  be16_to_cpu(sdt->app_tag));
-
-	return -EILSEQ;
 }
 
 /*
@@ -463,10 +453,7 @@ void sd_dif_complete(struct scsi_cmnd *scmd, unsigned int good_bytes)
 					return;
 				}
 
-				if (be32_to_cpu(sdt->ref_tag) != phys &&
-				    sdt->app_tag != 0xffff)
-					sdt->ref_tag = 0xffffffff; /* Bad ref */
-				else
+				if (be32_to_cpu(sdt->ref_tag) == phys)
 					sdt->ref_tag = cpu_to_be32(virt);
 
 				virt++;
-- 
1.7.8.3.21.gab8a7


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] sd: Allow protection_type to be overridden
  2012-02-21  1:20 T10 PI bugfixes Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 1/3] sd: Avoid remapping bad reference tags Martin K. Petersen
@ 2012-02-21  1:20 ` Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 3/3] scsi: Disable DIF on Hitachi Ultrastar 15K300 Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2012-02-21  1:20 UTC (permalink / raw)
  To: linux-scsi; +Cc: James.Bottomley, Martin K. Petersen

From: "Martin K. Petersen" <martin.petersen@oracle.com>

We have encountered a few devices that misbehaved when operating in T10
PI mode. Allow T10 PI protection type to be overridden from userland.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 77822b9..1f837b6 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -261,6 +261,28 @@ sd_show_protection_type(struct device *dev, struct device_attribute *attr,
 }
 
 static ssize_t
+sd_store_protection_type(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct scsi_disk *sdkp = to_scsi_disk(dev);
+	unsigned int val;
+	int err;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	err = kstrtouint(buf, 10, &val);
+
+	if (err)
+		return err;
+
+	if (val >= 0 && val <= SD_DIF_TYPE3_PROTECTION)
+		sdkp->protection_type = val;
+
+	return count;
+}
+
+static ssize_t
 sd_show_protection_mode(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
@@ -380,7 +402,8 @@ static struct device_attribute sd_disk_attrs[] = {
 	       sd_store_allow_restart),
 	__ATTR(manage_start_stop, S_IRUGO|S_IWUSR, sd_show_manage_start_stop,
 	       sd_store_manage_start_stop),
-	__ATTR(protection_type, S_IRUGO, sd_show_protection_type, NULL),
+	__ATTR(protection_type, S_IRUGO|S_IWUSR, sd_show_protection_type,
+	       sd_store_protection_type),
 	__ATTR(protection_mode, S_IRUGO, sd_show_protection_mode, NULL),
 	__ATTR(app_tag_own, S_IRUGO, sd_show_app_tag_own, NULL),
 	__ATTR(thin_provisioning, S_IRUGO, sd_show_thin_provisioning, NULL),
-- 
1.7.8.3.21.gab8a7


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] scsi: Disable DIF on Hitachi Ultrastar 15K300
  2012-02-21  1:20 T10 PI bugfixes Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 1/3] sd: Avoid remapping bad reference tags Martin K. Petersen
  2012-02-21  1:20 ` [PATCH 2/3] sd: Allow protection_type to be overridden Martin K. Petersen
@ 2012-02-21  1:20 ` Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2012-02-21  1:20 UTC (permalink / raw)
  To: linux-scsi; +Cc: James.Bottomley, Martin K. Petersen

From: "Martin K. Petersen" <martin.petersen@oracle.com>

Hitachi Ultrastar 15K300 is quirky. Disable T10 PI (DIF).

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/scsi_devinfo.c |    1 +
 drivers/scsi/scsi_scan.c    |    3 +++
 include/scsi/scsi_device.h  |    4 ++++
 include/scsi/scsi_devinfo.h |    1 +
 4 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index cf8dfab..43fca91 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -172,6 +172,7 @@ static struct {
 	{"HITACHI", "DF400", "*", BLIST_REPORTLUN2},
 	{"HITACHI", "DF500", "*", BLIST_REPORTLUN2},
 	{"HITACHI", "DISK-SUBSYSTEM", "*", BLIST_REPORTLUN2},
+	{"HITACHI", "HUS1530", "*", BLIST_NO_DIF},
 	{"HITACHI", "OPEN-", "*", BLIST_REPORTLUN2},
 	{"HITACHI", "OP-C-", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
 	{"HITACHI", "3380-", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 29c4c04..9143e81 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -923,6 +923,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	if (*bflags & BLIST_RETRY_HWERROR)
 		sdev->retry_hwerror = 1;
 
+	if (*bflags & BLIST_NO_DIF)
+		sdev->no_dif = 1;
+
 	transport_configure_device(&sdev->sdev_gendev);
 
 	if (sdev->host->hostt->slave_configure) {
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 77273f2..f48d94e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -151,6 +151,7 @@ struct scsi_device {
 	unsigned no_read_disc_info:1;	/* Avoid READ_DISC_INFO cmds */
 	unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */
 	unsigned is_visible:1;	/* is the device visible in sysfs */
+	unsigned no_dif:1;	/* T10 PI (DIF) should be disabled */
 
 	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
 	struct list_head event_list;	/* asserted events */
@@ -468,6 +469,9 @@ static inline int scsi_device_enclosure(struct scsi_device *sdev)
 
 static inline int scsi_device_protection(struct scsi_device *sdev)
 {
+	if (sdev->no_dif)
+		return 0;
+
 	return sdev->scsi_level > SCSI_2 && sdev->inquiry[5] & (1<<0);
 }
 
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index b4ddd3b..cc1f3e7 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -30,4 +30,5 @@
 #define BLIST_RETRY_HWERROR	0x400000 /* retry HARDWARE_ERROR */
 #define BLIST_MAX_512		0x800000 /* maximum 512 sector cdb length */
 #define BLIST_ATTACH_PQ3	0x1000000 /* Scan: Attach to PQ3 devices */
+#define BLIST_NO_DIF		0x2000000 /* Disable T10 PI (DIF) */
 #endif
-- 
1.7.8.3.21.gab8a7


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-02-21  1:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-21  1:20 T10 PI bugfixes Martin K. Petersen
2012-02-21  1:20 ` [PATCH 1/3] sd: Avoid remapping bad reference tags Martin K. Petersen
2012-02-21  1:20 ` [PATCH 2/3] sd: Allow protection_type to be overridden Martin K. Petersen
2012-02-21  1:20 ` [PATCH 3/3] scsi: Disable DIF on Hitachi Ultrastar 15K300 Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).