Advanced Format SAT devices show incorrect physical block size

Per Tom Yan suggestion I'm forwarding bug from bugzilla to this ML:
https://bugzilla.kernel.org/show_bug.cgi?id=102271

=== Dainius Masiliūnas wrote: ===

When using an Advanced Format drive connected through a SCSI-to-ATA Translation device, the physical block size reported by the kernel (in /sys/class/block/sdc/queue/physical_block_size) is 512 bytes. Whereas hdparm -I and smartctl -a do correctly show the physical block size as 4096 bytes.

In my case, the drive in question is a HGST HTS541075A9E680 hard drive, connected to my PC through a USB3 HDD enclosure (USB ID 05e3:0731).

While trying to find more information about this issue, I came across someone else's blog post about the same issue I'm having, which includes some in-depth analysis:
http://nunix.fr/index.php/linux/7-astuces/65-too-hard-to-be-above-2tb

It seems that the kernel uses less reliable means to get details about such devices than hdparm does.

I'm using the kernel that's shipped in openSUSE 13.2.

=== Tom Yan replied: ===

It is probably because of this:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/storage/scsiglue.c?h=v4.4#n217

SCSI READ CAPACITY (10) will not return "Logical blocks per physical block exponent" like READ CAPACITY (16) does:

[tom@localhost ~]$ sudo sg_readcap /dev/sdc
Read Capacity results:
Last logical block address=468862127 (0x1bf244af), Number of blocks=468862128 Logical block length=512 bytes
Hence:
Device size: 240057409536 bytes, 228936.6 MiB, 240.06 GB

[tom@localhost ~]$ sudo sg_readcap -16 /dev/sdc
Read Capacity results:
Protection: prot_en=0, p_type=0, p_i_exponent=0
Logical block provisioning: lbpme=0, lbprz=0
Last logical block address=468862127 (0x1bf244af), Number of logical blocks=468862128 Logical block length=512 bytes
Logical blocks per physical block exponent=3 [so physical block length=4096 bytes] Lowest aligned logical block address=0
Hence:
Device size: 240057409536 bytes, 228936.6 MiB, 240.06 GB

Quick summary: READ CAPACITY(10) does not include physical sector size information whereas READ CAPACITY(16) does. But the kernel uses READ CAPACITY(10) by default for USB drives, because quite a few of them die when given a READ CAPACITY(16) command.

If you can suggest a way to fix this, we'll be glad to hear it.

Huh, intersting to know. Why would they die on 16 and not on 10? Also,

wouldn't the right way to handle it be to use a quirk for broken models, then? Since my disk seems to work fine in that regard.

Quoted 4 lines · On Tuesday 10 January 2017 21:02:09 Alan Stern wrote:

Quick summary: READ CAPACITY(10) does not include physical sector size information whereas READ CAPACITY(16) does. But the kernel uses READ CAPACITY(10) by default for USB drives, because quite a few of them die when given a READ CAPACITY(16) command.

Ah :-( Are there lot of "broken" devices for creating blacklist?

Quoted 1 line

If you can suggest a way to fix this, we'll be glad to hear it.

Tom Yan wrote that smartctl/hdparm "works" because they use the SCSI ATA PASSTHROUGH command. It is not an option for kernel?

There _is_ a quirk for broken models. However, we don't know how complete the set of quirk entries is, so we err on the side of caution.

(I pressed reply instead of reply to all, sorry. Resending this.)

Quoted 2 lines · Alan Stern

There _is_ a quirk for broken models. However, we don't know how complete the set of quirk entries is, so we err on the side of caution.

Then what is it used for? There doesn't seem to be much point in a blacklist when the functionality is disabled by default to begin with.

And if there's a whitelist, does it mean that we should submit the IDs of working USB devices for whitelisting? Also, is there a module parameter to toggle the functionality, to facilitate testing whether the device works fine or not?

It is used for preventing the kernel from issuing a READ CAPACITY(16) command to the device. Normally the kernel would do this if the reply to READ CAPACITY(10) indicated there were more than 2^32 blocks (about 2 TB).

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level >= 3, or something along those lines. In general we hesitate to make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level >= 3, or something along those lines. In general we hesitate to make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

Quoted 7 lines · Alan Stern

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level >= 3, or something along those lines. In general we hesitate to make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

You shouldn't have to change anything: it already does (otherwise how else would we detect physical exponent for proper SCSI devices) see sd.c:sd_try_rc16_first(). It always returns false for USB because you set sdev->try_rc_10_first

Quoted 2 lines

Tom Yan wrote that smartctl/hdparm "works" because they use the SCSI ATA PASSTHROUGH command. It is not an option for kernel?

No, because many devices do not implement SCSI ATA PASSTHROUGH. (Consider devices whose underlying technology does not use ATA or SATA, for example.) And some of the ones that don't implement it will die if you try to send them an ATA PASSTHROUGH command.

You have to understand that consumer USB storage really is very low quality in many cases. Vendors aim for low cost rather than high reliability or correctness.

Quoted 4 lines · Alan Stern

It is used for preventing the kernel from issuing a READ CAPACITY(16) command to the device. Normally the kernel would do this if the reply to READ CAPACITY(10) indicated there were more than 2^32 blocks (about 2 TB).

Ah, OK, that makes sense. Though that's still not a proper solution, and if anything it's confusing for the users: if they plug in a >2 TiB drive and it breaks, the user will assume that it's the fault of the drive and not the USB case, or that the case is incapable of handling such large drives, whereas the true reason is completely different.

Quoted 7 lines

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level >= 3, or something along those lines. In general we hesitate to make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

Well, the problems are 1) we don't really know how many such misbehaving drives there are, since only those with >2 TiB drives experience it, thus it's hard to know what effect such change would have, and 2) there is no way to make the kernel output the true value even if you know your drive is working fine.

And when a drive breaks due to the 16 command, is there a way to revive it, perhaps by trying to reconnect? That would allow the kernel to try it first, see that it failed after that command was sent, then issue a warning in the log saying that this device should have a quirk, reset and continue with the quirk applied.

Also, is sg_readcap -16 a proper test to determine whether the device works as expected?

Quoted 7 lines · On Tuesday 10 January 2017 15:29:23 Alan Stern wrote:

Quoted 2 lines

Tom Yan wrote that smartctl/hdparm "works" because they use the SCSI ATA PASSTHROUGH command. It is not an option for kernel?

No, because many devices do not implement SCSI ATA PASSTHROUGH. (Consider devices whose underlying technology does not use ATA or SATA, for example.) And some of the ones that don't implement it will die if you try to send them an ATA PASSTHROUGH command.

It is not possible to detect if underlaying device is ATA?

Quoted 3 lines

You have to understand that consumer USB storage really is very low quality in many cases. Vendors aim for low cost rather than high reliability or correctness.

Understood. But lot of distributions call hdparm for inserted disks and also set some APM (or at least check it)... That means there is already some way how to deal with these problems (in userspace).

Quoted 7 lines · Alan Stern

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level >= 3, or something along those lines. In general we hesitate to make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

You shouldn't have to change anything: it already does (otherwise how else would we detect physical exponent for proper SCSI devices) see sd.c:sd_try_rc16_first(). It always returns false for USB because you set sdev->try_rc_10_first

Quoted 9 lines · James Bottomley

Quoted 7 lines · Alan Stern

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level

Quoted 1 line

= 3, or something along those lines. In general we hesitate to

make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

Quoted 4 lines

You shouldn't have to change anything: it already does (otherwise how else would we detect physical exponent for proper SCSI devices) see sd.c:sd_try_rc16_first(). It always returns false for USB because you set sdev->try_rc_10_first

In fact, this approach probably won't work. See Bugzilla entries #43265 and #43391. The devices in those reports claimed to be ANSI level 4, but they failed anyway.

If you guys want to try the quirk flag, you can apply the patch below. Then set the usb-storage module parameter quirks=vvvv:pppp:k where vvvv and pppp are the Vendor and Product ID codes for your device (as 4 hex digits).

In the long run, however, this is not a viable approach. We'd be better off with an explicit blacklist.

Alan Stern

Index: usb-4.x/drivers/usb/storage/usb.c
===================================================================

drivers/usb/storage/usb.c +4 -1

@@ -498,7 +498,7 @@ void usb_stor_adjust_quirks(struct usb_d
 			US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
 			US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
 			US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS |
-			US_FL_ALWAYS_SYNC);
+			US_FL_ALWAYS_SYNC | US_FL_NEEDS_CAP16);
 
 	p = quirks;
 	while (*p) {

@@ -551,6 +551,9 @@ void usb_stor_adjust_quirks(struct usb_d
 		case 'j':
 			f |= US_FL_NO_REPORT_LUNS;
 			break;
+		case 'k':
+			f |= US_FL_NEEDS_CAP16;
+			break;
 		case 'l':
 			f |= US_FL_NOT_LOCKABLE;
 			break;

Quoted 15 lines · On Wednesday 11 January 2017 16:23:29 Alan Stern wrote:

Quoted 13 lines · James Bottomley

Quoted 7 lines · Alan Stern

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level

Quoted 1 line

= 3, or something along those lines. In general we hesitate to

make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

You shouldn't have to change anything: it already does (otherwise how else would we detect physical exponent for proper SCSI devices) see sd.c:sd_try_rc16_first(). It always returns false for USB because you set sdev->try_rc_10_first

Quoted 3 lines

In fact, this approach probably won't work. See Bugzilla entries #43265 and #43391. The devices in those reports claimed to be ANSI level 4, but they failed anyway.

Seems those devices return capacity 0x7F000000000001 or 0xFF000000000001 Maybe there is some error pattern?

Quoted 5 lines

If you guys want to try the quirk flag, you can apply the patch below. Then set the usb-storage module parameter quirks=vvvv:pppp:k where vvvv and pppp are the Vendor and Product ID codes for your device (as 4 hex digits).

Quoted 2 lines

In the long run, however, this is not a viable approach. We'd be better off with an explicit blacklist.

Ok, so what are next steps? I think that explicit blacklist would be needed if "bad" devices is less.

How many bug reports were there?

Quoted 20 lines · Pali Rohár

Quoted 19 lines · On Wednesday 11 January 2017 16:23:29 Alan Stern wrote:

Quoted 17 lines · James Bottomley

Quoted 11 lines · Alan Stern

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level

Quoted 2 lines

= 3, or something along those lines. In general we hesitate to

make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

You shouldn't have to change anything: it already does (otherwise how else would we detect physical exponent for proper SCSI devices) see sd.c:sd_try_rc16_first(). It always returns false for USB because you set sdev->try_rc_10_first

Quoted 6 lines

Quoted 3 lines

In fact, this approach probably won't work. See Bugzilla entries #43265 and #43391. The devices in those reports claimed to be ANSI level 4, but they failed anyway.

Seems those devices return capacity 0x7F000000000001 or 0xFF000000000001 Maybe there is some error pattern?

As far as I can tell, they both reported 0xFF000000000001. That's a pattern -- unless somebody really does have a storage device that large (18 exabytes). For the time being, perhaps we can ignore this possibility.

Quoted 11 lines

Quoted 7 lines

If you guys want to try the quirk flag, you can apply the patch below. Then set the usb-storage module parameter quirks=vvvv:pppp:k where vvvv and pppp are the Vendor and Product ID codes for your device (as 4 hex digits).

In the long run, however, this is not a viable approach. We'd be better off with an explicit blacklist.

Ok, so what are next steps? I think that explicit blacklist would be needed if "bad" devices is less.

Quoted 1 line

How many bug reports were there?

I don't know.

Anyway, please try out the patch below. I don't know if it will be acceptable to the SCSI maintainers, but we should at least make sure it fixes your problem before submitting it.

Alan Stern

Index: usb-4.x/drivers/scsi/sd.c
===================================================================

drivers/scsi/sd.c +6 -0

@@ -2157,6 +2157,13 @@ static int read_capacity_16(struct scsi_
 		return -ENODEV;
 	}
 
+	/* Some buggy devices report an impossibly large size */
+	if (lba >= (1ULL << 54)) {
+		sd_printk(KERN_WARNING, sdkp, "Read Capacity(16) returned
 excessively large value: %llu", lba); +		sdkp->capacity = 0;
+		return -EINVAL;
+	}
+
 	if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) {
 		sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
 			"kernel compiled with support for large block "

Index: usb-4.x/drivers/usb/storage/scsiglue.c
===================================================================

drivers/usb/storage/scsiglue.c +4 -1

@@ -247,8 +247,11 @@ static int slave_configure(struct scsi_d
 		 * Tell the SCSI layer to try READ_CAPACITY_10 first.
 		 * However some USB 3.0 drive enclosures return capacity
 		 * modulo 2TB. Those must use READ_CAPACITY_16
+		 *
+		 * Assume SPC3 or later devices can handle READ_CAPACITY_16.
 		 */
-		if (!(us->fflags & US_FL_NEEDS_CAP16))
+		if (sdev->scsi_level <= SCSI_SPC_2 &&
+				!(us->fflags & US_FL_NEEDS_CAP16))
 			sdev->try_rc_10_first = 1;
 
 		/* assume SPC3 or latter devices support sense size > 18 */

Quoted 48 lines · On Monday 30 January 2017 17:17:03 Alan Stern wrote:

Quoted 20 lines · Pali Rohár

Quoted 19 lines · On Wednesday 11 January 2017 16:23:29 Alan Stern wrote:

Quoted 17 lines · James Bottomley

Quoted 11 lines · Alan Stern

In theory, I suppose we could change the kernel so that it would default to READ CAPACITY(16) for devices that report a SCSI level

Quoted 2 lines

= 3, or something along those lines. In general we hesitate to

make changes of this sort, because they almost always end up breaking _some_ devices -- and if that happens then the change is reverted, with no exceptions. Linus has a very strict rule about not breaking working systems.

You shouldn't have to change anything: it already does (otherwise how else would we detect physical exponent for proper SCSI devices) see sd.c:sd_try_rc16_first(). It always returns false for USB because you set sdev->try_rc_10_first

Quoted 6 lines

Quoted 3 lines

In fact, this approach probably won't work. See Bugzilla entries #43265 and #43391. The devices in those reports claimed to be ANSI level 4, but they failed anyway.

Seems those devices return capacity 0x7F000000000001 or 0xFF000000000001 Maybe there is some error pattern?

As far as I can tell, they both reported 0xFF000000000001. That's a pattern -- unless somebody really does have a storage device that large (18 exabytes). For the time being, perhaps we can ignore this possibility.

Quoted 5 lines

Quoted 5 lines

If you guys want to try the quirk flag, you can apply the patch below. Then set the usb-storage module parameter quirks=vvvv:pppp:k where vvvv and pppp are the Vendor and Product ID codes for your device (as 4 hex digits).

Quoted 7 lines

Quoted 2 lines

In the long run, however, this is not a viable approach. We'd be better off with an explicit blacklist.

Ok, so what are next steps? I think that explicit blacklist would be needed if "bad" devices is less.

How many bug reports were there?

I don't know.

Quoted 3 lines

Anyway, please try out the patch below. I don't know if it will be acceptable to the SCSI maintainers, but we should at least make sure it fixes your problem before submitting it.

I'm not original reporter of this problem.

Dainius, can you test it?

Quoted 40 lines

Alan Stern

Index: usb-4.x/drivers/scsi/sd.c
===================================================================

drivers/scsi/sd.c +7 -0

@@ -2157,6 +2157,13 @@ static int read_capacity_16(struct scsi_
 		return -ENODEV;
 	}
 
+	/* Some buggy devices report an impossibly large size */
+	if (lba >= (1ULL << 54)) {
+		sd_printk(KERN_WARNING, sdkp, "Read Capacity(16) returned excessively large value: %llu", lba);
+		sdkp->capacity = 0;
+		return -EINVAL;
+	}
+
 	if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) {
 		sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
 			"kernel compiled with support for large block "

Index: usb-4.x/drivers/usb/storage/scsiglue.c
===================================================================

drivers/usb/storage/scsiglue.c +4 -1

@@ -247,8 +247,11 @@ static int slave_configure(struct scsi_d
 		 * Tell the SCSI layer to try READ_CAPACITY_10 first.
 		 * However some USB 3.0 drive enclosures return capacity
 		 * modulo 2TB. Those must use READ_CAPACITY_16
+		 *
+		 * Assume SPC3 or later devices can handle READ_CAPACITY_16.
 		 */
-		if (!(us->fflags & US_FL_NEEDS_CAP16))
+		if (sdev->scsi_level <= SCSI_SPC_2 &&
+				!(us->fflags & US_FL_NEEDS_CAP16))
 			sdev->try_rc_10_first = 1;
 
 		/* assume SPC3 or latter devices support sense size > 18 */