Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512

Bernhard,

Ugh, it *does* change on the fly:

sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 512, last LBA 3a3812aae

[...]

sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, last LBA 3a3812aae

[...]

sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, last LBA 3a3812aae

And I am guessing your device does not trigger a Unit Attention as a
result. You don't see something like "Inquiry data has changed" or
"Capacity data has changed" in the log, do you?

-- 
Martin K. Petersen	Oracle Linux Engineering

On 23.03.20 00:32, Martin K. Petersen wrote:

Bernhard,

Ugh, it *does* change on the fly:

quoted

sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 512, last LBA 3a3812aae

[...]

quoted

sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, last LBA 3a3812aae

[...]

quoted

sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, last LBA 3a3812aae

And I am guessing your device does not trigger a Unit Attention as a
result. You don't see something like "Inquiry data has changed" or
"Capacity data has changed" in the log, do you?

There does not seem to be a change in capaticy detected - you got a almost complete section of dmesg from me. Anyway, here's the whole thing after plugging in:

[  +8.214282] usb 4-2: new SuperSpeed Gen 1 USB device number 2 using xhci_hcd
[  +0.021643] usb 4-2: New USB device found, idVendor=0bc2, idProduct=ab45, bcdDevice=48.85
[  +0.000002] usb 4-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  +0.000001] usb 4-2: Product: Backup+ Hub
[  +0.000002] usb 4-2: Manufacturer: Seagate
[  +0.000000] usb 4-2: SerialNumber: 01CB8213B6KJ
[  +0.002367] hub 4-2:1.0: USB hub found
[  +0.000277] hub 4-2:1.0: 3 ports detected
[  +0.115567] usb 2-2: new high-speed USB device number 2 using xhci_hcd
[  +0.019555] usb 2-2: New USB device found, idVendor=0bc2, idProduct=ab44, bcdDevice=48.85
[  +0.000002] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  +0.000001] usb 2-2: Product: Backup+ Hub
[  +0.000001] usb 2-2: Manufacturer: Seagate
[  +0.000001] usb 2-2: SerialNumber: 01CB8213B6KJ
[  +0.000618] hub 2-2:1.0: USB hub found
[  +0.000261] hub 2-2:1.0: 3 ports detected
[  +0.146258] usb 4-2.1: new SuperSpeed Gen 1 USB device number 3 using xhci_hcd
[  +0.017416] usb 4-2.1: New USB device found, idVendor=0bc2, idProduct=ab38, bcdDevice= 1.00
[  +0.000062] usb 4-2.1: New USB device strings: Mfr=2, Product=3, SerialNumber=1
[  +0.000002] usb 4-2.1: Product: Backup+ Hub BK
[  +0.000001] usb 4-2.1: Manufacturer: Seagate
[  +0.000001] usb 4-2.1: SerialNumber: NA9Q19AM
[  +0.854292] usbcore: registered new interface driver usb-storage
[  +0.006271] scsi host6: uas
[  +0.000099] usbcore: registered new interface driver uas
[  +0.000690] scsi 6:0:0:0: Direct-Access     Seagate  Backup+ Hub BK   D781 PQ: 0 ANSI: 6
[  +0.001009] sd 6:0:0:0: Attached scsi generic sg3 type 0
[  +0.000199] sd 6:0:0:0: [sdc] Spinning up disk...
[  +0.043686] usb 2-2-port2: Cannot enable. Maybe the USB cable is bad?
[  +0.982900] ..
[  +1.648526] audit: type=1131 audit(1584919133.626:206): pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-hostnamed comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
[  +0.092344] audit: type=1334 audit(1584919133.721:207): prog-id=28 op=UNLOAD
[  +0.000007] audit: type=1334 audit(1584919133.721:208): prog-id=27 op=UNLOAD
[  +0.300907] ..........ready
[Mar23 00:19] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, rc16_first: 1
[  +0.000068] sd 6:0:0:0: [sdc] read_capacity_16: result 0, retries 2
[  +0.000002] sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 512, last LBA 3a3812aae
[  +0.000236] sd 6:0:0:0: [sdc] 15628053167 512-byte logical blocks: (8.00 TB/7.28 TiB)

22 more lines

Some USB bridge devices will return a default set of characteristics during
initialization. And then, once an attached drive has spun up, substitute
the actual parameters reported by the drive. According to the SCSI spec,
the device should return a UNIT ATTENTION in case any reported parameters
change. But in this case the change is made silently after a small window
where default values are reported.

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, this validation did not account for the fact that
aforementioned devices will return default values during a brief window
during spin-up. The subsequent change in reported characteristics would
invalidate the checking that had previously been performed.

Unset a previously configured optimal I/O size should the sanity checking
fail on subsequent revalidate attempts.

Link: https://lore.kernel.org/r/33fb522e-4f61-1b76-914f-c9e6a3553c9b@gmail.com
Cc: Bryan Gurney <redacted>
Reported-by: Bernhard Sulzer <redacted>
Signed-off-by: Martin K. Petersen <redacted>
---
 drivers/scsi/sd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8ca9299ffd36..2710a0e5ae6d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c

@@ -3169,9 +3169,11 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	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);
-	} else
+	} else {
+		q->limits.io_opt = 0;
 		rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
 				      (sector_t)BLK_DEF_MAX_SECTORS);
+	}
 
 	/* Do not exceed controller limit */
 	rw_max = min(rw_max, queue_max_hw_sectors(q));

-- 
2.24.1