Re: Kernel crash with unsupported DIF protection type

2012-09-21 Thread Hannes Reinecke 
On 09/20/2012 09:53 PM, Martin K. Petersen wrote:
 Hannes == Hannes Reinecke h...@suse.de writes:
 
 Hannes I recently got my hands on some weird drives, insisting on
 Hannes having been formatted with protection type 7:
 
 Lovely :|
 
Hehe. Probably _really_ future drives :-)

 
 Hannes I've attached a tentative patch, which allows the system to
 Hannes boot.  However, I'm not completely happy with that, as the
 Hannes capacity is _still_ updated after revalidation:
 
 How about this?
 
 
 sd: Ensure we correctly disable devices with unknown protection type
 
 We set the capacity to zero when we discovered a device formatted with
 an unknown DIF protection type. However, the read_capacity code would
 override the capacity and cause the device to be enabled regardless.
 
 Make sd_read_protection_type() return an error if the protection type is
 unknown. Also prevent duplicate printk lines when the device is being
 revalidated.
 
 Reported-by: Hannes Reinecke h...@suse.de
 Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
 
 diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
 index 1cf2d5d..1c54564 100644
 --- a/drivers/scsi/sd.c
 +++ b/drivers/scsi/sd.c
 @@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
  /*
   * Determine whether disk supports Data Integrity Field.
   */
 -static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char 
 *buffer)
 +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)
 - return;
 + return ret;
  
   type = ((buffer[12]  1)  7) + 1; /* P_TYPE 0 = Type 1 */
  
 - if (type == sdkp-protection_type || !sdkp-first_scan)
 - return;
 + if (type  SD_DIF_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;
 + }
  
   sdkp-protection_type = type;
  
 - if (type  SD_DIF_TYPE3_PROTECTION) {
 - sd_printk(KERN_ERR, sdkp, formatted with unsupported  \
 -   protection type %u. Disabling disk!\n, type);
 - sdkp-capacity = 0;
 - return;
 - }
 -
 - if (scsi_host_dif_capable(sdp-host, type))
 - sd_printk(KERN_NOTICE, sdkp,
 -   Enabling DIF Type %u protection\n, type);
 - else
 - sd_printk(KERN_NOTICE, sdkp,
 -   Disabling DIF Type %u protection\n, type);
 + return ret;
  }
  
  static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device 
 *sdp,
 @@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, 
 struct scsi_device *sdp,
   sector_size = get_unaligned_be32(buffer[8]);
   lba = get_unaligned_be64(buffer[0]);
  
 - sd_read_protection_type(sdkp, buffer);
 + if (sd_read_protection_type(sdkp, buffer)  0) {
 + sdkp-capacity = 0;
 + return -ENODEV;
 + }
  
   if ((sizeof(sdkp-capacity) == 4)  (lba = 0xULL)) {
   sd_printk(KERN_ERR, sdkp, Too big for this kernel. Use a 
 
Yup. That's what I had in mind.

Acked-by: Hannes Reinecke h...@suse.de

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel crash with unsupported DIF protection type

2012-09-21 Thread Hannes Reinecke 
On 09/20/2012 09:53 PM, Martin K. Petersen wrote:
 Hannes == Hannes Reinecke h...@suse.de writes:
 
 Hannes I recently got my hands on some weird drives, insisting on
 Hannes having been formatted with protection type 7:
 
 Lovely :|
 
 
 Hannes I've attached a tentative patch, which allows the system to
 Hannes boot.  However, I'm not completely happy with that, as the
 Hannes capacity is _still_ updated after revalidation:
 
 How about this?
 
 
 sd: Ensure we correctly disable devices with unknown protection type
 
 We set the capacity to zero when we discovered a device formatted with
 an unknown DIF protection type. However, the read_capacity code would
 override the capacity and cause the device to be enabled regardless.
 
 Make sd_read_protection_type() return an error if the protection type is
 unknown. Also prevent duplicate printk lines when the device is being
 revalidated.
 
 Reported-by: Hannes Reinecke h...@suse.de
 Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
 
 diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
 index 1cf2d5d..1c54564 100644
 --- a/drivers/scsi/sd.c
 +++ b/drivers/scsi/sd.c
 @@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
  /*
   * Determine whether disk supports Data Integrity Field.
   */
 -static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char 
 *buffer)
 +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)
 - return;
 + return ret;
  
   type = ((buffer[12]  1)  7) + 1; /* P_TYPE 0 = Type 1 */
  
 - if (type == sdkp-protection_type || !sdkp-first_scan)
 - return;
 + if (type  SD_DIF_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;
 + }
  
   sdkp-protection_type = type;
  
 - if (type  SD_DIF_TYPE3_PROTECTION) {
 - sd_printk(KERN_ERR, sdkp, formatted with unsupported  \
 -   protection type %u. Disabling disk!\n, type);
 - sdkp-capacity = 0;
 - return;
 - }
 -
 - if (scsi_host_dif_capable(sdp-host, type))
 - sd_printk(KERN_NOTICE, sdkp,
 -   Enabling DIF Type %u protection\n, type);
 - else
 - sd_printk(KERN_NOTICE, sdkp,
 -   Disabling DIF Type %u protection\n, type);
 + return ret;
  }
  
  static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device 
 *sdp,
 @@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, 
 struct scsi_device *sdp,
   sector_size = get_unaligned_be32(buffer[8]);
   lba = get_unaligned_be64(buffer[0]);
  
 - sd_read_protection_type(sdkp, buffer);
 + if (sd_read_protection_type(sdkp, buffer)  0) {
 + sdkp-capacity = 0;
 + return -ENODEV;
 + }
  
   if ((sizeof(sdkp-capacity) == 4)  (lba = 0xULL)) {
   sd_printk(KERN_ERR, sdkp, Too big for this kernel. Use a 

Hehe. It helps to actually test patches.

Now I get:
sd 6:0:0:0: [sdb] formatted with unsupported protection type 7.
Disabling disk!

and silence after that.
However, I still get:

sd 6:0:0:0: [sdb] Write Protect is off
sd 6:0:0:0: [sdb] Mode Sense: cf 00 10 08
sd 6:0:0:0: [sdb] Write cache: disabled, read cache: enabled,
supports DPO and FUA
sd 6:0:0:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection
sd 6:0:0:0: [sdb] Attached SCSI disk

which is a bit odd, given that the disk is disabled.
Plus that we never actually claimed to support type1.
So maybe we should add this patch, too?

diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 0cb39ff..e0500bf 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -320,7 +320,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
dif = 0; dix = 1;
}

-   if (!dix)
+   if (!dix || type  SD_DIF_TYPE3_PROTECTION)
return;

/* Enable DMA of protection information */

But then, main issue is resolved, as the system continues to boot.
So my Acked-by: still stands.

Cheers,

Re: Kernel crash with unsupported DIF protection type

2012-09-21 Thread Martin K. Petersen 
 Hannes == Hannes Reinecke h...@suse.de writes:

Hannes Hehe. It helps to actually test patches.

I did. However, with scsi_debug I have all sorts of sanity checks in
place. Not sure what your disk is connected to?


Hannes sd 6:0:0:0: [sdb] Write Protect is off sd 6:0:0:0: [sdb] Mode
Hannes Sense: cf 00 10 08 sd 6:0:0:0: [sdb] Write cache: disabled, read
Hannes cache: enabled, supports DPO and FUA sd 6:0:0:0: [sdb] Enabling
Hannes DIX T10-DIF-TYPE1-CRC protection sd 6:0:0:0: [sdb] Attached SCSI
Hannes disk

DIX and DIF are orthogonal. If the HBA driver does not support the
appropriate DIF type we'll still enable DIX. That's a feature.

But there are two issues here. First we should not configure DIX if
capacity is 0. Secondly, we need some bounds checking in the host mask
checking to protect against crackpot disks.

Updated patch below...


sd: Ensure we correctly disable devices with unknown protection type

We set the capacity to zero when we discovered a device formatted with
an unknown DIF protection type. However, the read_capacity code would
override the capacity and cause the device to be enabled regardless.

Make sd_read_protection_type() return an error if the protection type is
unknown. Also prevent duplicate printk lines when the device is being
revalidated.

Reported-by: Hannes Reinecke h...@suse.de
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1cf2d5d..7a22db3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
 /*
  * Determine whether disk supports Data Integrity Field.
  */
-static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char 
*buffer)
+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)
-   return;
+   return ret;
 
type = ((buffer[12]  1)  7) + 1; /* P_TYPE 0 = Type 1 */
 
-   if (type == sdkp-protection_type || !sdkp-first_scan)
-   return;
+   if (type  SD_DIF_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;
+   }
 
sdkp-protection_type = type;
 
-   if (type  SD_DIF_TYPE3_PROTECTION) {
-   sd_printk(KERN_ERR, sdkp, formatted with unsupported  \
- protection type %u. Disabling disk!\n, type);
-   sdkp-capacity = 0;
-   return;
-   }
-
-   if (scsi_host_dif_capable(sdp-host, type))
-   sd_printk(KERN_NOTICE, sdkp,
- Enabling DIF Type %u protection\n, type);
-   else
-   sd_printk(KERN_NOTICE, sdkp,
- Disabling DIF Type %u protection\n, type);
+   return ret;
 }
 
 static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device 
*sdp,
@@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, 
struct scsi_device *sdp,
sector_size = get_unaligned_be32(buffer[8]);
lba = get_unaligned_be64(buffer[0]);
 
-   sd_read_protection_type(sdkp, buffer);
+   if (sd_read_protection_type(sdkp, buffer)  0) {
+   sdkp-capacity = 0;
+   return -ENODEV;
+   }
 
if ((sizeof(sdkp-capacity) == 4)  (lba = 0xULL)) {
sd_printk(KERN_ERR, sdkp, Too big for this kernel. Use a 
@@ -2788,7 +2799,8 @@ static void sd_probe_async(void *data, async_cookie_t 
cookie)
}
 
add_disk(gd);
-   sd_dif_config_host(sdkp);
+   if (sdkp-capacity)
+   sd_dif_config_host(sdkp);
 
sd_revalidate_disk(gd);
 
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 5f7d5b3..b19223d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -871,7 +871,11 @@ static inline unsigned int scsi_host_dif_capable(struct 
Scsi_Host *shost, unsign
static unsigned char cap[] = { 0,
   SHOST_DIF_TYPE1_PROTECTION,

Re: Kernel crash with unsupported DIF protection type

2012-09-21 Thread Martin K. Petersen 
 Martin == Martin K Petersen martin.peter...@oracle.com writes:

Martin Updated patch below...

This time without mangled closing bracket...


sd: Ensure we correctly disable devices with unknown protection type

We set the capacity to zero when we discovered a device formatted with
an unknown DIF protection type. However, the read_capacity code would
override the capacity and cause the device to be enabled regardless.

Make sd_read_protection_type() return an error if the protection type is
unknown. Also prevent duplicate printk lines when the device is being
revalidated.

Reported-by: Hannes Reinecke h...@suse.de
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1cf2d5d..7a22db3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
 /*
  * Determine whether disk supports Data Integrity Field.
  */
-static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char 
*buffer)
+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)
-   return;
+   return ret;
 
type = ((buffer[12]  1)  7) + 1; /* P_TYPE 0 = Type 1 */
 
-   if (type == sdkp-protection_type || !sdkp-first_scan)
-   return;
+   if (type  SD_DIF_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;
+   }
 
sdkp-protection_type = type;
 
-   if (type  SD_DIF_TYPE3_PROTECTION) {
-   sd_printk(KERN_ERR, sdkp, formatted with unsupported  \
- protection type %u. Disabling disk!\n, type);
-   sdkp-capacity = 0;
-   return;
-   }
-
-   if (scsi_host_dif_capable(sdp-host, type))
-   sd_printk(KERN_NOTICE, sdkp,
- Enabling DIF Type %u protection\n, type);
-   else
-   sd_printk(KERN_NOTICE, sdkp,
- Disabling DIF Type %u protection\n, type);
+   return ret;
 }
 
 static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device 
*sdp,
@@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, 
struct scsi_device *sdp,
sector_size = get_unaligned_be32(buffer[8]);
lba = get_unaligned_be64(buffer[0]);
 
-   sd_read_protection_type(sdkp, buffer);
+   if (sd_read_protection_type(sdkp, buffer)  0) {
+   sdkp-capacity = 0;
+   return -ENODEV;
+   }
 
if ((sizeof(sdkp-capacity) == 4)  (lba = 0xULL)) {
sd_printk(KERN_ERR, sdkp, Too big for this kernel. Use a 
@@ -2788,7 +2799,8 @@ static void sd_probe_async(void *data, async_cookie_t 
cookie)
}
 
add_disk(gd);
-   sd_dif_config_host(sdkp);
+   if (sdkp-capacity)
+   sd_dif_config_host(sdkp);
 
sd_revalidate_disk(gd);
 
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 5f7d5b3..4908480 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -873,6 +873,9 @@ static inline unsigned int scsi_host_dif_capable(struct 
Scsi_Host *shost, unsign
   SHOST_DIF_TYPE2_PROTECTION,
   SHOST_DIF_TYPE3_PROTECTION };
 
+   if (target_type  SHOST_DIF_TYPE3_PROTECTION)
+   return 0;
+
return shost-prot_capabilities  cap[target_type] ? target_type : 0;
 }
 
@@ -884,6 +887,9 @@ static inline unsigned int scsi_host_dix_capable(struct 
Scsi_Host *shost, unsign
   SHOST_DIX_TYPE2_PROTECTION,
   SHOST_DIX_TYPE3_PROTECTION };
 
+   if (target_type  SHOST_DIX_TYPE3_PROTECTION)
+   return 0;
+
return shost-prot_capabilities  cap[target_type];
 #endif
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at

Re: Kernel crash with unsupported DIF protection type

2012-09-20 Thread Martin K. Petersen 
 Hannes == Hannes Reinecke h...@suse.de writes:

Hannes I recently got my hands on some weird drives, insisting on
Hannes having been formatted with protection type 7:

Lovely :|


Hannes I've attached a tentative patch, which allows the system to
Hannes boot.  However, I'm not completely happy with that, as the
Hannes capacity is _still_ updated after revalidation:

How about this?


sd: Ensure we correctly disable devices with unknown protection type

We set the capacity to zero when we discovered a device formatted with
an unknown DIF protection type. However, the read_capacity code would
override the capacity and cause the device to be enabled regardless.

Make sd_read_protection_type() return an error if the protection type is
unknown. Also prevent duplicate printk lines when the device is being
revalidated.

Reported-by: Hannes Reinecke h...@suse.de
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1cf2d5d..1c54564 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
 /*
  * Determine whether disk supports Data Integrity Field.
  */
-static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char 
*buffer)
+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)
-   return;
+   return ret;
 
type = ((buffer[12]  1)  7) + 1; /* P_TYPE 0 = Type 1 */
 
-   if (type == sdkp-protection_type || !sdkp-first_scan)
-   return;
+   if (type  SD_DIF_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;
+   }
 
sdkp-protection_type = type;
 
-   if (type  SD_DIF_TYPE3_PROTECTION) {
-   sd_printk(KERN_ERR, sdkp, formatted with unsupported  \
- protection type %u. Disabling disk!\n, type);
-   sdkp-capacity = 0;
-   return;
-   }
-
-   if (scsi_host_dif_capable(sdp-host, type))
-   sd_printk(KERN_NOTICE, sdkp,
- Enabling DIF Type %u protection\n, type);
-   else
-   sd_printk(KERN_NOTICE, sdkp,
- Disabling DIF Type %u protection\n, type);
+   return ret;
 }
 
 static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device 
*sdp,
@@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, 
struct scsi_device *sdp,
sector_size = get_unaligned_be32(buffer[8]);
lba = get_unaligned_be64(buffer[0]);
 
-   sd_read_protection_type(sdkp, buffer);
+   if (sd_read_protection_type(sdkp, buffer)  0) {
+   sdkp-capacity = 0;
+   return -ENODEV;
+   }
 
if ((sizeof(sdkp-capacity) == 4)  (lba = 0xULL)) {
sd_printk(KERN_ERR, sdkp, Too big for this kernel. Use a 
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel crash with unsupported DIF protection type

2012-09-18 Thread Hannes Reinecke 
On 09/18/2012 11:30 AM, Douglas Gilbert wrote:
 On 12-09-18 11:04 AM, Hannes Reinecke wrote:
 Hi all,

 I recently got my hands on some weird drives, insisting on having
 been formatted with protection type 7:

 # sg_readcap --16 /dev/sdb
 Read Capacity results:
 Protection: prot_en=1, p_type=6, p_i_exponent=0
 Logical block provisioning: lbpme=0, lbprz=0
 Last logical block address=1758174767 (0x68cb9e2f), Number of
 logical blocks=1758174768
 Logical block length=512 bytes
 Logical blocks per physical block exponent=0
 Lowest aligned logical block address=0
 Hence:
 Device size: 900185481216 bytes, 858483.8 MiB, 900.19 GB

 (I know. I've already complained.)
 However, this drive causes a horrible crash:

 sd 0:0:1:0: [sdb] formatted with unsupported protection type 7.
 Disabling disk!
 sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900
 GB/838 GiB)
 sd 0:0:1:0: [sdb]  Result: hostbyte=DID_OK
 driverbyte=DRIVER_SENSE
 sd 0:0:1:0: [sdb]  Sense Key : Medium Error [current]
 sd 0:0:1:0: [sdb]  Add. Sense: Medium format corrupted
 sd 0:0:1:0: [sdb] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00
 end_request: I/O error, dev sdb, sector 0
 Buffer I/O error on device sdb, logical block 0

 [ tons and tons of I/O errors ]

 [   15.551689] sd 0:0:1:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC
 protection
 [   15.561353] [ cut here ]
 [   15.569416] kernel BUG at
 /usr/src/linux-3.0/drivers/scsi/scsi_lib.c:1069!

 There are several odd things happening here:
 - It says 'Disabling disk', which _should_ have set the
capacity to '0':

 drivers/scsi/sd.c:sd_read_protection_type()
 if (type  SD_DIF_TYPE3_PROTECTION) {
 sd_printk(KERN_ERR, sdkp, formatted with unsupported \
   protection type %u. Disabling disk!\n, type);
 sdkp-capacity = 0;
 return;
 }

 - it enables type 1 protection, which it evidently is not.

 I've attached a tentative patch, which allows the system to boot.
 However, I'm not completely happy with that, as the capacity is
 _still_ updated after revalidation:

 sd 0:0:1:0: [sdb] formatted with unsupported protection type 7.
 Disabling disk!
 sd 0:0:1:0: [sdb] 0 512-byte logical blocks: (0 B/0 B)
 sd 0:0:1:0: [sdb] Write Protect is off
 sd 0:0:1:0: [sdb] Mode Sense: cf 00 10 08
 sd 0:0:1:0: [sdb] Write cache: disabled, read cache: enabled,
 supports DPO and FUA
 sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838
 GiB)
 sd 0:0:1:0: [sdb] Attached SCSI disk

 Thoughts?
 
 The Medium format corrupted additional sense qualifier occurs
 (with Seagate disks) when a FORMAT UNIT command is interrupted.
 So maybe, for good measure, that disk also sets the DIF
 protection type to an unsupported value (i.e. 7). So re-doing a
 FORMAT  UNIT may clear that state.
 
Yeah, that's what I'm doing now.

Incidentally:

sg_format manpage says:

   Format with type 1 protection:

  sg_format --format --fmtpinfo=3 --pfu /dev/sdm

but the '--pfu' option requires an argument.
According to sbc3r23 the command should read:

sg_format --format --fmtpinfo=3 --pfu=1 /dev/sdm

Correct?

 Obviously the kernel should not crash when faced with such
 a disk.
 
Precisely.
Especially the 'Disabling disk' no-op is worrying.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel crash with unsupported DIF protection type

2012-09-18 Thread Douglas Gilbert 

On 12-09-18 11:35 AM, Hannes Reinecke wrote:

On 09/18/2012 11:30 AM, Douglas Gilbert wrote:

On 12-09-18 11:04 AM, Hannes Reinecke wrote:

Hi all,

I recently got my hands on some weird drives, insisting on having
been formatted with protection type 7:

# sg_readcap --16 /dev/sdb
Read Capacity results:
 Protection: prot_en=1, p_type=6, p_i_exponent=0
 Logical block provisioning: lbpme=0, lbprz=0
 Last logical block address=1758174767 (0x68cb9e2f), Number of
logical blocks=1758174768
 Logical block length=512 bytes
 Logical blocks per physical block exponent=0
 Lowest aligned logical block address=0
Hence:
 Device size: 900185481216 bytes, 858483.8 MiB, 900.19 GB

(I know. I've already complained.)
However, this drive causes a horrible crash:

sd 0:0:1:0: [sdb] formatted with unsupported protection type 7.
Disabling disk!
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900
GB/838 GiB)
sd 0:0:1:0: [sdb]  Result: hostbyte=DID_OK
driverbyte=DRIVER_SENSE
sd 0:0:1:0: [sdb]  Sense Key : Medium Error [current]
sd 0:0:1:0: [sdb]  Add. Sense: Medium format corrupted
sd 0:0:1:0: [sdb] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00
end_request: I/O error, dev sdb, sector 0
Buffer I/O error on device sdb, logical block 0

[ tons and tons of I/O errors ]

[   15.551689] sd 0:0:1:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC
protection
[   15.561353] [ cut here ]
[   15.569416] kernel BUG at
/usr/src/linux-3.0/drivers/scsi/scsi_lib.c:1069!

There are several odd things happening here:
- It says 'Disabling disk', which _should_ have set the
capacity to '0':

drivers/scsi/sd.c:sd_read_protection_type()
 if (type  SD_DIF_TYPE3_PROTECTION) {
 sd_printk(KERN_ERR, sdkp, formatted with unsupported \
   protection type %u. Disabling disk!\n, type);
 sdkp-capacity = 0;
 return;
 }

- it enables type 1 protection, which it evidently is not.

I've attached a tentative patch, which allows the system to boot.
However, I'm not completely happy with that, as the capacity is
_still_ updated after revalidation:

sd 0:0:1:0: [sdb] formatted with unsupported protection type 7.
Disabling disk!
sd 0:0:1:0: [sdb] 0 512-byte logical blocks: (0 B/0 B)
sd 0:0:1:0: [sdb] Write Protect is off
sd 0:0:1:0: [sdb] Mode Sense: cf 00 10 08
sd 0:0:1:0: [sdb] Write cache: disabled, read cache: enabled,
supports DPO and FUA
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838
GiB)
sd 0:0:1:0: [sdb] Attached SCSI disk

Thoughts?


The Medium format corrupted additional sense qualifier occurs
(with Seagate disks) when a FORMAT UNIT command is interrupted.
So maybe, for good measure, that disk also sets the DIF
protection type to an unsupported value (i.e. 7). So re-doing a
FORMAT  UNIT may clear that state.


Yeah, that's what I'm doing now.

Incidentally:

sg_format manpage says:

Format with type 1 protection:

   sg_format --format --fmtpinfo=3 --pfu /dev/sdm

but the '--pfu' option requires an argument.
According to sbc3r23 the command should read:

sg_format --format --fmtpinfo=3 --pfu=1 /dev/sdm

Correct?


Yes. And it is corrected in the 1.34 betas that I have
sent you :-)


I placed a copy of my latest sg3_utils-1.34 beta
in the News section at the top of this page:
  http://sg.danny.cz/sg

Doug Gilbert


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html