All of lore.kernel.org
 help / color / mirror / Atom feed
* Invalid optimal transfer size 33553920 accepted when physical_block_size 512
@ 2020-03-22 14:32 Bernhard Sulzer
  2020-03-22 15:45 ` Martin K. Petersen
  2020-03-22 20:57 ` [PATCH] scsi: sd: Optimal I/O size should be a multiple of reported granularity Martin K. Petersen
  0 siblings, 2 replies; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 14:32 UTC (permalink / raw)
  To: linux-scsi

According to the gparted forum 
(http://gparted-forum.surf4.info/viewtopic.php?id=17839) a couple 
sata-usb adapters seem to report an invalid optimal transfer size of 
33553920 (which is 0xFFFF * 512).

This should have been fixed in a83da8a4509d about a year ago by in 
sd_validate_opt_xfer_size by checking whether the optimal transfer size 
opt_xfer_bytes is a multiple of physical_block_size. This works when 
physical block size is 4096, unfortunately it does not when the physical 
block size is 512 (which it is in my case).
Related commit: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.1&id=a83da8a4509d3ebfe03bb7fffce022e4d5d4764f

This is pretty curious in itself, because lsblk reports 4096 physical 
sector size / 512 logical sector size and I would have thought physical 
sector size and physical block size should be the same. Is this a bug 
that should be reported?

Anyway, maybe 33553920 should always be marked as invalid?


Equipment used:
     # uname -srv
     Linux 5.6.0-rc6-1-git-00137-gb74b991fb8b9 #1 SMP PREEMPT Sat Mar 21 
22:13:06 CET 2020

     # lsblk /dev/sdc -t
     NAME ALIGNMENT MIN-IO   OPT-IO PHY-SEC LOG-SEC ROTA SCHED       
RQ-SIZE  RA WSAME
     sdc          0   4096 33553920    4096     512    1 
mq-deadline      60 128   32M

     # lsblk /dev/sdc -o +KNAME,HCTL,VENDOR,MODEL,SERIAL,REV
     NAME MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT KNAME HCTL VENDOR   
MODEL          SERIAL    REV
     sdc    8:32   0  7.3T  0 disk            sdc   6:0:0:0 Seagate  
Backup+_Hub_BK NA9Q19AM D781

     # cat /proc/scsi/scsi
     Attached devices:
     [...]
     Host: scsi6 Channel: 00 Id: 00 Lun: 00
       Vendor: Seagate  Model: Backup+ Hub BK   Rev: D781
       Type:   Direct-Access                    ANSI  SCSI revision: 06

     # sudo lsusb -tvvv
     /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
         ID 1d6b:0003 Linux Foundation 3.0 root hub
         /sys/bus/usb/devices/usb4  /dev/bus/usb/004/001
         |__ Port 2: Dev 6, If 0, Class=Hub, Driver=hub/3p, 5000M
             ID 0bc2:ab45 Seagate RSS LLC
             /sys/bus/usb/devices/4-2  /dev/bus/usb/004/006
             |__ Port 1: Dev 7, If 0, Class=Mass Storage, Driver=uas, 5000M
                 ID 0bc2:ab38 Seagate RSS LLC Backup Plus Hub
                 /sys/bus/usb/
     [...]


Thanks, and please be easy on me


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 14:32 Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Bernhard Sulzer
@ 2020-03-22 15:45 ` Martin K. Petersen
       [not found]   ` <accd7d25-ee35-11b9-e49b-76e20d9550f2@gmail.com>
  2020-03-22 20:57 ` [PATCH] scsi: sd: Optimal I/O size should be a multiple of reported granularity Martin K. Petersen
  1 sibling, 1 reply; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-22 15:45 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: linux-scsi


Bernhard,

> This is pretty curious in itself, because lsblk reports 4096 physical
> sector size / 512 logical sector size and I would have thought
> physical sector size and physical block size should be the same.  Is
> this a bug that should be reported?

That's peculiar. What does:

# dmesg | grep Optimal

have to say?

Also, please send the output of:

# sg_readcap -l /dev/sdc
# sg_vpd -p bl /dev/sdc

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
       [not found]     ` <yq1pnd4uxof.fsf@oracle.com>
@ 2020-03-22 17:41       ` Bernhard Sulzer
  0 siblings, 0 replies; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 17:41 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi

> Can you send me the output of:
> # dmesg | grep "sd 2:0:0:0"
>
> and
>
> # sg_inq /dev/sdc
>
> Thanks!

Relevant section from dmesg:

     [11987.820542] scsi host6: uas
[11987.821278] scsi 6:0:0:0: Direct-Access     Seagate  Backup+ Hub BK   
D781 PQ: 0 ANSI: 6
[11987.822003] sd 6:0:0:0: Attached scsi generic sg3 type 0
[11987.822117] sd 6:0:0:0: [sdc] Spinning up disk...
[11988.874931] .............ready
[12001.355881] sd 6:0:0:0: [sdc] 15628053167 512-byte logical blocks: 
(8.00 TB/7.28 TiB)
[12001.388502] sd 6:0:0:0: [sdc] Write Protect is off
[12001.388511] sd 6:0:0:0: [sdc] Mode Sense: 4f 00 00 00
[12001.388843] sd 6:0:0:0: [sdc] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[12001.389144] sd 6:0:0:0: [sdc] Optimal transfer size 33553920 bytes
[12001.475091] sd 6:0:0:0: [sdc] Attached SCSI disk

# sg_inq /dev/sdc
standard INQUIRY:
   PQual=0  Device_type=0  RMB=0  LU_CONG=0 version=0x06  [SPC-4]
   [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0 Resp_data_format=2
   SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0 [BQue=0]
   EncServ=0  MultiP=0  [MChngr=0] [ACKREQQ=0]  Addr16=0
   [RelAdr=0]  WBus16=0  Sync=0  [Linked=0] [TranDis=0]  CmdQue=1
   [SPI: Clocking=0x0  QAS=0  IUS=0]
     length=76 (0x4c)   Peripheral device type: disk
Vendor identification: Seagate
Product identification: Backup+ Hub BK
Product revision level: D781
Unit serial number: NA9Q19AM

Hope this helps


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
       [not found]     ` <yq1pnd4tbxm.fsf@oracle.com>
@ 2020-03-22 19:45       ` Bernhard Sulzer
  2020-03-22 21:06         ` Martin K. Petersen
  0 siblings, 1 reply; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 19:45 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi

>>      # sg_readcap -l /dev/sdc
>>      Read Capacity results:
>>         Protection: prot_en=0, p_type=0, p_i_exponent=0
>>         Logical block provisioning: lbpme=0, lbprz=0
>>         Last LBA=15628053166 (0x3a3812aae), Number of logical
>> blocks=15628053167
>>         Logical block length=512 bytes
>>         Logical blocks per physical block exponent=3 [so physical block
>> length=4096 bytes]
>>         Lowest aligned LBA=0
>>      Hence:
>>         Device size: 8001563221504 bytes, 7630885.3 MiB, 8001.56 GB, 8.00 TB
> Please send me the output of:
>
> # sg_readcap /dev/sdc
>
> (i.e. without the -l from the previous run).
>
> I am very puzzled as to why we end up in the older capacity code for a
> device this big (8TB).
>

Here you go:

# sg_readcap /dev/sdc
READ CAPACITY (10) indicates device capacity too large
   now trying 16 byte cdb variant
Read Capacity results:
    Protection: prot_en=0, p_type=0, p_i_exponent=0
    Logical block provisioning: lbpme=0, lbprz=0
    Last LBA=15628053166 (0x3a3812aae), Number of logical blocks=15628053167
    Logical block length=512 bytes
    Logical blocks per physical block exponent=3 [so physical block 
length=4096 bytes]
    Lowest aligned LBA=0
Hence:
    Device size: 8001563221504 bytes, 7630885.3 MiB, 8001.56 GB, 8.00 TB

Hope this helps to solve it


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

* [PATCH] scsi: sd: Optimal I/O size should be a multiple of reported granularity
  2020-03-22 14:32 Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Bernhard Sulzer
  2020-03-22 15:45 ` Martin K. Petersen
@ 2020-03-22 20:57 ` Martin K. Petersen
  1 sibling, 0 replies; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-22 20:57 UTC (permalink / raw)
  To: linux-scsi; +Cc: Martin K. Petersen, Bernhard Sulzer

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, some devices claim conformity to older SCSI versions that predate
the physical block size being reported. Other devices do not report a
physical block size at all. We need to be able to validate the optimal I/O
size on those devices as well.

Many devices report an OPTIMAL TRANSFER LENGTH GRANULARITY in the same VPD
page as the OPTIMAL TRANSFER LENGTH. Use this value to validate the optimal
I/O size. Also check that the reported granularity is a multiple of the
physical block size, if supported.

Link: https://lore.kernel.org/r/33fb522e-4f61-1b76-914f-c9e6a3553c9b@gmail.com
Reported-by: Bernhard Sulzer <micraft.b@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c | 43 +++++++++++++++++++++++++++++++++++++++----
 drivers/scsi/sd.h |  1 +
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8ca9299ffd36..e41f8eb00787 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2858,7 +2858,6 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
  */
 static void sd_read_block_limits(struct scsi_disk *sdkp)
 {
-	unsigned int sector_sz = sdkp->device->sector_size;
 	const int vpd_len = 64;
 	unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL);
 
@@ -2867,9 +2866,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
 	    scsi_get_vpd_page(sdkp->device, 0xb0, buffer, vpd_len))
 		goto out;
 
-	blk_queue_io_min(sdkp->disk->queue,
-			 get_unaligned_be16(&buffer[6]) * sector_sz);
-
+	sdkp->min_xfer_blocks = get_unaligned_be16(&buffer[6]);
 	sdkp->max_xfer_blocks = get_unaligned_be32(&buffer[8]);
 	sdkp->opt_xfer_blocks = get_unaligned_be32(&buffer[12]);
 
@@ -3036,6 +3033,29 @@ static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
 		sdkp->security = 1;
 }
 
+static bool sd_validate_min_xfer_size(struct scsi_disk *sdkp)
+{
+	struct scsi_device *sdp = sdkp->device;
+	unsigned int min_xfer_bytes =
+		logical_to_bytes(sdp, sdkp->min_xfer_blocks);
+
+	if (sdkp->min_xfer_blocks == 0)
+		return false;
+
+	if (min_xfer_bytes & (sdkp->physical_block_size - 1)) {
+		sd_first_printk(KERN_WARNING, sdkp,
+				"Preferred minimum I/O size %u bytes not a " \
+				"multiple of physical block size (%u bytes)\n",
+				min_xfer_bytes, sdkp->physical_block_size);
+		sdkp->min_xfer_blocks = 0;
+		return false;
+	}
+
+	sd_first_printk(KERN_INFO, sdkp, "Preferred minimum I/O size %u bytes\n",
+			min_xfer_bytes);
+	return true;
+}
+
 /*
  * Determine the device's preferred I/O size for reads and writes
  * unless the reported value is unreasonably small, large, not a
@@ -3047,6 +3067,8 @@ static bool sd_validate_opt_xfer_size(struct scsi_disk *sdkp,
 	struct scsi_device *sdp = sdkp->device;
 	unsigned int opt_xfer_bytes =
 		logical_to_bytes(sdp, sdkp->opt_xfer_blocks);
+	unsigned int min_xfer_bytes =
+		logical_to_bytes(sdp, sdkp->min_xfer_blocks);
 
 	if (sdkp->opt_xfer_blocks == 0)
 		return false;
@@ -3083,6 +3105,15 @@ static bool sd_validate_opt_xfer_size(struct scsi_disk *sdkp,
 		return false;
 	}
 
+	if (min_xfer_bytes && opt_xfer_bytes & (min_xfer_bytes - 1)) {
+		sd_first_printk(KERN_WARNING, sdkp,
+				"Optimal transfer size %u bytes not a " \
+				"multiple of preferred minimum block " \
+				"size (%u bytes)\n",
+				opt_xfer_bytes, min_xfer_bytes);
+		return false;
+	}
+
 	sd_first_printk(KERN_INFO, sdkp, "Optimal transfer size %u bytes\n",
 			opt_xfer_bytes);
 	return true;
@@ -3166,6 +3197,10 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks);
 	q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max);
 
+	if (sd_validate_min_xfer_size(sdkp))
+		blk_queue_io_min(sdkp->disk->queue,
+				 logical_to_bytes(sdp, sdkp->min_xfer_blocks));
+
 	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);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 50fff0bf8c8e..7efdcb103c2a 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -82,6 +82,7 @@ struct scsi_disk {
 #endif
 	atomic_t	openers;
 	sector_t	capacity;	/* size in logical blocks */
+	u32		min_xfer_blocks;
 	u32		max_xfer_blocks;
 	u32		opt_xfer_blocks;
 	u32		max_ws_blocks;
-- 
2.24.1


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 19:45       ` Bernhard Sulzer
@ 2020-03-22 21:06         ` Martin K. Petersen
  2020-03-22 21:20           ` Bernhard Sulzer
  0 siblings, 1 reply; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-22 21:06 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: Martin K. Petersen, linux-scsi


Bernhard,

> # sg_readcap /dev/sdc
> READ CAPACITY (10) indicates device capacity too large
>   now trying 16 byte cdb variant
> Read Capacity results:
>    Protection: prot_en=0, p_type=0, p_i_exponent=0
>    Logical block provisioning: lbpme=0, lbprz=0
>    Last LBA=15628053166 (0x3a3812aae), Number of logical blocks=15628053167
>    Logical block length=512 bytes
>    Logical blocks per physical block exponent=3 [so physical block
> length=4096 bytes]
>    Lowest aligned LBA=0
> Hence:
>    Device size: 8001563221504 bytes, 7630885.3 MiB, 8001.56 GB, 8.00 TB

I sent a patch that I would like you to test. It adds some additional
sanity checking to the block limits handling. Given the VPD output you
sent earlier, I am hoping it will work around the issue.

I still can't explain how the physical block size can be unset given
that it is reported by the device and the capacity is > 0xffffffff. I
even tried to tweak scsi_debug to see if somehow the no_read_capacity_16
flag for card readers happened to be set in your case and caused us to
go down the wrong path. But no. I'm stumped.

Do you have any READ CAPACITY errors or messages in your log? There were
none in the output you sent.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 21:06         ` Martin K. Petersen
@ 2020-03-22 21:20           ` Bernhard Sulzer
  2020-03-22 21:53             ` Bernhard Sulzer
  0 siblings, 1 reply; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 21:20 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi


> I sent a patch that I would like you to test. It adds some additional
> sanity checking to the block limits handling. Given the VPD output you
> sent earlier, I am hoping it will work around the issue.
>
> I still can't explain how the physical block size can be unset given
> that it is reported by the device and the capacity is > 0xffffffff. I
> even tried to tweak scsi_debug to see if somehow the no_read_capacity_16
> flag for card readers happened to be set in your case and caused us to
> go down the wrong path. But no. I'm stumped.
>
> Do you have any READ CAPACITY errors or messages in your log? There were
> none in the output you sent.
>

Currently compiling...


# dmesg | grep -i capacity

... returns empty. Are there other tests I should run?

Meanwhile, this is everything dmesg gives me when plugging in the drive, 
running lsblk and launching gparted:

# dmesg -wH

[Mar22 22:18] usb 3-2: new SuperSpeed Gen 1 USB device number 8 using 
xhci_hcd
[  +0.033073] usb 3-2: New USB device found, idVendor=0bc2, 
idProduct=ab45, bcdDevice=48.85
[  +0.000004] usb 3-2: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
[  +0.000001] usb 3-2: Product: Backup+ Hub
[  +0.000001] usb 3-2: Manufacturer: Seagate
[  +0.000001] usb 3-2: SerialNumber: 01CB8213B6KJ
[  +0.002245] hub 3-2:1.0: USB hub found
[  +0.000314] hub 3-2:1.0: 3 ports detected
[  +0.334402] usb 3-2.1: new SuperSpeed Gen 1 USB device number 9 using 
xhci_hcd
[  +0.030705] usb 3-2.1: New USB device found, idVendor=0bc2, 
idProduct=ab38, bcdDevice= 1.00
[  +0.000003] usb 3-2.1: New USB device strings: Mfr=2, Product=3, 
SerialNumber=1
[  +0.000001] usb 3-2.1: Product: Backup+ Hub BK
[  +0.000001] usb 3-2.1: Manufacturer: Seagate
[  +0.000001] usb 3-2.1: SerialNumber: NA9Q19AM
[  +0.003609] scsi host6: uas
[  +0.000551] scsi 6:0:0:0: Direct-Access     Seagate  Backup+ Hub BK   
D781 PQ: 0 ANSI: 6
[  +0.000554] sd 6:0:0:0: Attached scsi generic sg3 type 0
[  +0.000177] sd 6:0:0:0: [sdc] Spinning up disk...
[  +1.054179] ...........ready
[ +10.400840] sd 6:0:0:0: [sdc] 15628053167 512-byte logical blocks: 
(8.00 TB/7.28 TiB)
[  +0.038524] sd 6:0:0:0: [sdc] Write Protect is off
[  +0.000002] sd 6:0:0:0: [sdc] Mode Sense: 4f 00 00 00
[  +0.000156] sd 6:0:0:0: [sdc] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  +0.000243] sd 6:0:0:0: [sdc] Optimal transfer size 33553920 bytes
[  +0.086311] sd 6:0:0:0: [sdc] Attached SCSI disk
[  +0.235116] audit: type=1130 audit(1584911910.652:444): pid=1 uid=0 
auid=4294967295 ses=4294967295 msg='unit=lvm2-pvscan@8:32 comm="systemd" 
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
[ +20.268639] audit: type=1100 audit(1584911930.923:445): pid=17078 
uid=1000 auid=1000 ses=3 msg='op=PAM:authentication 
grantors=pam_unix,pam_permit acct="bernhard" 
exe="/usr/lib/polkit-1/polkit-agent-helper-1" hostname=? addr=? 
terminal=? res=success'
[  +0.000213] audit: type=1101 audit(1584911930.923:446): pid=17078 
uid=1000 auid=1000 ses=3 msg='op=PAM:accounting 
grantors=pam_unix,pam_permit,pam_time acct="bernhard" 
exe="/usr/lib/polkit-1/polkit-agent-helper-1" hostname=? addr=? 
terminal=? res=success'
[  +0.019962] audit: type=1105 audit(1584911930.943:447): pid=17071 
uid=1000 auid=1000 ses=3 msg='op=PAM:session_open 
grantors=pam_limits,pam_unix,pam_permit acct="root" 
exe="/usr/bin/pkexec" hostname=? addr=? terminal=? res=success'
[  +0.418886] JFS: nTxBlock = 8192, nTxLock = 65536
[  +0.120146] SGI XFS with ACLs, security attributes, realtime, scrub, 
repair, no debug enabled



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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 21:20           ` Bernhard Sulzer
@ 2020-03-22 21:53             ` Bernhard Sulzer
  2020-03-22 22:45               ` Martin K. Petersen
  0 siblings, 1 reply; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 21:53 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi


>> I sent a patch that I would like you to test. It adds some additional
>> sanity checking to the block limits handling. Given the VPD output you
>> sent earlier, I am hoping it will work around the issue.
>
> Currently compiling...

With the patch applied, the situation looks unchanged. I did gain a line 
for preferred minimum though.

[   92.984044] usbcore: registered new interface driver usb-storage
[   92.990020] scsi host6: uas
[   92.990113] usbcore: registered new interface driver uas
[   92.990480] scsi 6:0:0:0: Direct-Access     Seagate Backup+ Hub BK   
D781 PQ: 0 ANSI: 6
[   92.991014] sd 6:0:0:0: Attached scsi generic sg3 type 0
[   92.991203] sd 6:0:0:0: [sdc] Spinning up disk...
[   93.029139] usb 2-2-port2: Cannot enable. Maybe the USB cable is bad?
[   94.012043] ............ready
[  105.159757] sd 6:0:0:0: [sdc] 15628053167 512-byte logical blocks: 
(8.00 TB/7.28 TiB)
[  105.196980] sd 6:0:0:0: [sdc] Write Protect is off
[  105.196986] sd 6:0:0:0: [sdc] Mode Sense: 4f 00 00 00
[  105.197182] sd 6:0:0:0: [sdc] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  105.197403] sd 6:0:0:0: [sdc] Preferred minimum I/O size 512 bytes
[  105.197407] sd 6:0:0:0: [sdc] Optimal transfer size 33553920 bytes
[  105.240714] sd 6:0:0:0: [sdc] Attached SCSI disk

Does this help?


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 21:53             ` Bernhard Sulzer
@ 2020-03-22 22:45               ` Martin K. Petersen
  2020-03-22 23:10                 ` Bernhard Sulzer
  2020-03-22 23:22                 ` Bernhard Sulzer
  0 siblings, 2 replies; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-22 22:45 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: Martin K. Petersen, linux-scsi


Bernhard,

> [  105.197403] sd 6:0:0:0: [sdc] Preferred minimum I/O size 512 bytes

In the sg_vpd -p bl output you sent this field was set to 8 blocks
(i.e. 4096 bytes). And in the sg_readcap -l output the physical block
size exponent was reported as 3 (i.e. also 4096 bytes).

But when we inspect these values during device discovery they appear to
be either 0 or 1. What is weird is that if the device somehow updated
them on the fly, I would also expect the optimal transfer length value
to be 0 as well. But it is consistently reported as 0xffff.

Do the reported values change if you do the following a while after you
plugged the drive in?

# lsblk -t
# echo 1 > /sys/block/sdc/device/rescan
# sleep 10
# lsblk -t

The only way I can replicate your results is by making scsi_debug return
zeroes during discovery and then switch to reporting the correct values
after a while. I did a quick hack where I returned zeroes for the
optimal transfer length granularity and the physical block size exponent
the first few times they were requested. That produces results similar
to yours.

I also attached a quick debug patch for capturing some more info.

-- 
Martin K. Petersen	Oracle Linux Engineering


diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e41f8eb00787..e1e3213ab155 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2333,6 +2333,11 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 	/* Logical blocks per physical block exponent */
 	sdkp->physical_block_size = (1 << (buffer[13] & 0xf)) * sector_size;
 
+	sd_printk(KERN_ERR, sdkp, "%s: result %d, retries %u\n", __func__,
+		  the_result, retries);
+	sd_printk(KERN_ERR, sdkp, "%s: lbs %u, pbs %u, last LBA %llx\n", __func__,
+		  sector_size, sdkp->physical_block_size, lba);
+
 	/* RC basis */
 	sdkp->rc_basis = (buffer[12] >> 4) & 0x3;
 
@@ -2402,6 +2407,11 @@ static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
 	sector_size = get_unaligned_be32(&buffer[4]);
 	lba = get_unaligned_be32(&buffer[0]);
 
+	sd_printk(KERN_ERR, sdkp, "%s: result %d, retries %u\n", __func__,
+		  the_result, retries);
+	sd_printk(KERN_ERR, sdkp, "%s: lbs %u, last LBA %llx\n", __func__,
+		  sector_size, lba);
+
 	if (sdp->no_read_capacity_16 && (lba == 0xffffffff)) {
 		/* Some buggy (usb cardreader) devices return an lba of
 		   0xffffffff when the want to report a size of 0 (with
@@ -2438,6 +2448,9 @@ sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer)
 	int sector_size;
 	struct scsi_device *sdp = sdkp->device;
 
+	sd_printk(KERN_ERR, sdkp, "%s: rc10_first %u, rc16_first: %u\n",
+		  __func__, sdp->try_rc_10_first, sd_try_rc16_first(sdp));
+
 	if (sd_try_rc16_first(sdp)) {
 		sector_size = read_capacity_16(sdkp, sdp, buffer);
 		if (sector_size == -EOVERFLOW)
@@ -2457,7 +2470,7 @@ sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer)
 		if ((sizeof(sdkp->capacity) > 4) &&
 		    (sdkp->capacity > 0xffffffffULL)) {
 			int old_sector_size = sector_size;
-			sd_printk(KERN_NOTICE, sdkp, "Very big device. "
+			sd_printk(KERN_ERR, sdkp, "Very big device. "
 					"Trying to use READ CAPACITY(16).\n");
 			sector_size = read_capacity_16(sdkp, sdp, buffer);
 			if (sector_size < 0) {

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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 22:45               ` Martin K. Petersen
@ 2020-03-22 23:10                 ` Bernhard Sulzer
  2020-03-22 23:22                 ` Bernhard Sulzer
  1 sibling, 0 replies; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 23:10 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi

> Do the reported values change if you do the following a while after you
> plugged the drive in?
>
> # lsblk -t
> # echo 1 > /sys/block/sdc/device/rescan
> # sleep 10
> # lsblk -t

I cant say they do, as far as I could observe:

# lsblk /dev/sdc -t; echo 1 | tee /sys/block/sdc/device/rescan; sleep 
10; lsblk /dev/sdc -t
NAME ALIGNMENT MIN-IO   OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE  RA WSAME
sdc          0   4096 33553920    4096     512    1 mq-deadline      60 
128   32M
1
NAME ALIGNMENT MIN-IO   OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE  RA WSAME
sdc          0   4096 33553920    4096     512    1 mq-deadline      60 
128   32M



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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 22:45               ` Martin K. Petersen
  2020-03-22 23:10                 ` Bernhard Sulzer
@ 2020-03-22 23:22                 ` Bernhard Sulzer
  2020-03-22 23:32                   ` Martin K. Petersen
  1 sibling, 1 reply; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 23:22 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi


> I also attached a quick debug patch for capturing some more info.

This is, what I am getting now with both patches applied in order:

[  +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.982900] ..
[  +0.300907] ..........ready
[Mär23 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)
[  +0.070974] sd 6:0:0:0: [sdc] Write Protect is off
[  +0.000002] sd 6:0:0:0: [sdc] Mode Sense: 4f 00 00 00
[  +0.000153] sd 6:0:0:0: [sdc] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  +0.000200] sd 6:0:0:0: [sdc] Preferred minimum I/O size 512 bytes
[  +0.000001] sd 6:0:0:0: [sdc] Optimal transfer size 33553920 bytes
[  +0.020349] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000094] 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 4096, 
last LBA 3a3812aae
[  +0.021930] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000079] 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 4096, 
last LBA 3a3812aae
[  +0.000567] sd 6:0:0:0: [sdc] Attached SCSI disk


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 23:22                 ` Bernhard Sulzer
@ 2020-03-22 23:32                   ` Martin K. Petersen
  2020-03-22 23:40                     ` Bernhard Sulzer
  0 siblings, 1 reply; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-22 23:32 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: Martin K. Petersen, linux-scsi


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

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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 23:32                   ` Martin K. Petersen
@ 2020-03-22 23:40                     ` Bernhard Sulzer
  2020-03-23  1:41                       ` Martin K. Petersen
                                         ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-22 23:40 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi

On 23.03.20 00:32, Martin K. Petersen wrote:
> 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?
>
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)
[  +0.070974] sd 6:0:0:0: [sdc] Write Protect is off
[  +0.000002] sd 6:0:0:0: [sdc] Mode Sense: 4f 00 00 00
[  +0.000153] sd 6:0:0:0: [sdc] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  +0.000200] sd 6:0:0:0: [sdc] Preferred minimum I/O size 512 bytes
[  +0.000001] sd 6:0:0:0: [sdc] Optimal transfer size 33553920 bytes
[  +0.020349] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000094] 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 4096, 
last LBA 3a3812aae
[  +0.021930] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000079] 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 4096, 
last LBA 3a3812aae
[  +0.000567] sd 6:0:0:0: [sdc] Attached SCSI disk
[  +0.130848] audit: type=1130 audit(1584919143.380:209): pid=1 uid=0 
auid=4294967295 ses=4294967295 msg='unit=lvm2-pvscan@8:32 comm="systemd" 
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
[  +8.838420] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000104] sd 6:0:0:0: [sdc] read_capacity_16: result 0, retries 2
[  +0.000003] sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, 
last LBA 3a3812aae
[ +43.549880] psmouse serio1: Touchpad at isa0060/serio1/input0 lost 
sync at byte 6
[  +0.035220] psmouse serio1: Touchpad at isa0060/serio1/input0 - driver 
resynced.
[Mar23 00:21] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000093] sd 6:0:0:0: [sdc] read_capacity_16: result 0, retries 2
[  +0.000005] sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, 
last LBA 3a3812aae


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 23:40                     ` Bernhard Sulzer
@ 2020-03-23  1:41                       ` Martin K. Petersen
  2020-03-24 13:49                         ` Bryan Gurney
  2020-03-24 15:47                       ` [PATCH] scsi: sd: Fix optimal I/O size for devices that change reported values Martin K. Petersen
  2020-03-24 15:52                       ` Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Martin K. Petersen
  2 siblings, 1 reply; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-23  1:41 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: Martin K. Petersen, linux-scsi


Bernhard,

> 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:

OK, got a workaround. Will send tomorrow.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-23  1:41                       ` Martin K. Petersen
@ 2020-03-24 13:49                         ` Bryan Gurney
  0 siblings, 0 replies; 19+ messages in thread
From: Bryan Gurney @ 2020-03-24 13:49 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: Bernhard Sulzer, linux-scsi

On Sun, Mar 22, 2020 at 9:43 PM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Bernhard,
>
> > 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:
>
> OK, got a workaround. Will send tomorrow.
>

Hi Martin,

I might be able to help test this, as I have a USB drive enclosure
that is exhibiting the same kernel message of "Optimal transfer size
33553920 bytes not a multiple of physical block size (4096 bytes)".  I
noticed this as I was using the enclosure to random overwrite a batch
of old drives, and I started seeing that message in a terminal running
"journalctl -kf" after I had connected a couple of Advanced Format
(4096 byte physical, 512 byte logical) drives.

Here's an example of the USB enclosure with a 512e drive plugged in:

$ sudo sg_inq /dev/sdc
standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  LU_CONG=0  version=0x06  [SPC-4]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=2
  SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  [Linked=0]  [TranDis=0]  CmdQue=1
  [SPI: Clocking=0x0  QAS=0  IUS=0]
    length=76 (0x4c)   Peripheral device type: disk
 Vendor identification: TOSHIBA
 Product identification: MK1059GSM
 Product revision level: 0
 Unit serial number: 1000000000CA

$ 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=1953525167 (0x74706daf), Number of
logical blocks=1953525168
   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: 1000204886016 bytes, 953869.7 MiB, 1000.20 GB
$ sudo sg_readcap -H -16 /dev/sdc
 00     00 00 00 00 74 70 6d af  00 00 02 00 00 03 00 00
 10     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00

$ sudo sg_vpd -p 0xb0 /dev/sdc
Block limits VPD page (SBC):
  Write same non-zero (WSNZ): 0
  Maximum compare and write length: 0 blocks
  Optimal transfer length granularity: 8 blocks
  Maximum transfer length: 65535 blocks
  Optimal transfer length: 65535 blocks
  Maximum prefetch length: 65535 blocks
  Maximum unmap LBA count: 0
  Maximum unmap block descriptor count: 0
  Optimal unmap granularity: 0
  Unmap granularity alignment valid: 0
  Unmap granularity alignment: 0
  Maximum write same length: 0x0 blocks
  Maximum atomic transfer length: 0
  Atomic alignment: 0
  Atomic transfer length granularity: 0
  Maximum atomic transfer length with atomic boundary: 0
  Maximum atomic boundary size: 0
$ sudo sg_vpd -H -p 0xb0 /dev/sdc
Block limits VPD page (SBC):
 00     00 b0 00 3c 00 00 00 08  00 00 ff ff 00 00 ff ff    ...<............
 10     00 00 ff ff 00 00 00 00  00 00 00 00 00 00 00 00    ................
 20     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................
 30     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................

...and here's the same drive, plugged in directly to a Serial ATA port
on the mainboard:

$ sudo sg_inq /dev/sdb
standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  LU_CONG=0  version=0x05  [SPC-3]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=2
  SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  [Linked=0]  [TranDis=0]  CmdQue=1
  [SPI: Clocking=0x0  QAS=0  IUS=0]
    length=96 (0x60)   Peripheral device type: disk
 Vendor identification: ATA
 Product identification: TOSHIBA MK1059GS
 Product revision level: 1U
 Unit serial number:            91GCPH6RT

$ sudo sg_readcap -16 /dev/sdb
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=1953525167 (0x74706daf), Number of
logical blocks=1953525168
   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: 1000204886016 bytes, 953869.7 MiB, 1000.20 GB
$ sudo sg_readcap -H -16 /dev/sdb
 00     00 00 00 00 74 70 6d af  00 00 02 00 00 03 00 00
 10     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00

$ sudo sg_vpd -p 0xb0 /dev/sdb
Block limits VPD page (SBC):
  Write same non-zero (WSNZ): 0
  Maximum compare and write length: 0 blocks
  Optimal transfer length granularity: 8 blocks
  Maximum transfer length: 0 blocks
  Optimal transfer length: 0 blocks
  Maximum prefetch length: 0 blocks
  Maximum unmap LBA count: 0
  Maximum unmap block descriptor count: 0
  Optimal unmap granularity: 0
  Unmap granularity alignment valid: 0
  Unmap granularity alignment: 0
  Maximum write same length: 0x0 blocks
  Maximum atomic transfer length: 0
  Atomic alignment: 0
  Atomic transfer length granularity: 0
  Maximum atomic transfer length with atomic boundary: 0
  Maximum atomic boundary size: 0
$ sudo sg_vpd -H -p 0xb0 /dev/sdb
Block limits VPD page (SBC):
 00     00 b0 00 3c 00 00 00 08  00 00 00 00 00 00 00 00    ...<............
 10     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................
 20     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................
 30     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00    ................


Thanks,

Bryan


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

* [PATCH] scsi: sd: Fix optimal I/O size for devices that change reported values
  2020-03-22 23:40                     ` Bernhard Sulzer
  2020-03-23  1:41                       ` Martin K. Petersen
@ 2020-03-24 15:47                       ` Martin K. Petersen
  2020-03-24 15:52                       ` Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Martin K. Petersen
  2 siblings, 0 replies; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-24 15:47 UTC (permalink / raw)
  To: linux-scsi; +Cc: Martin K. Petersen, Bryan Gurney, Bernhard Sulzer

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 <bgurney@redhat.com>
Reported-by: Bernhard Sulzer <micraft.b@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 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


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-22 23:40                     ` Bernhard Sulzer
  2020-03-23  1:41                       ` Martin K. Petersen
  2020-03-24 15:47                       ` [PATCH] scsi: sd: Fix optimal I/O size for devices that change reported values Martin K. Petersen
@ 2020-03-24 15:52                       ` Martin K. Petersen
  2020-03-24 16:14                         ` Bernhard Sulzer
  2 siblings, 1 reply; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-24 15:52 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: Martin K. Petersen, linux-scsi, Bryan Gurney


Bernhard,

>> 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?

I have been working on a set of patches that will address devices like
this that change their tune halfway through discovery. Your case is
really just the tip of the iceberg, more changes are needed to handle
this gracefully.

I'll try to get these changes completed in time for 5.7. However, we
need a smaller fix for 5.6 and stable. Can you please try the patch I
just sent?

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-24 15:52                       ` Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Martin K. Petersen
@ 2020-03-24 16:14                         ` Bernhard Sulzer
  2020-03-27  0:54                           ` Martin K. Petersen
  0 siblings, 1 reply; 19+ messages in thread
From: Bernhard Sulzer @ 2020-03-24 16:14 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: linux-scsi, Bryan Gurney


>>> 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?
> I have been working on a set of patches that will address devices like
> this that change their tune halfway through discovery. Your case is
> really just the tip of the iceberg, more changes are needed to handle
> this gracefully.
>
> I'll try to get these changes completed in time for 5.7. However, we
> need a smaller fix for 5.6 and stable. Can you please try the patch I
> just sent?
>
> Thanks!
>
The patch seems to work great.

# dmesg -wH

[  +0.005956] scsi host6: uas
[  +0.000153] usbcore: registered new interface driver uas
[  +0.000392] scsi 6:0:0:0: Direct-Access     Seagate  Backup+ Hub BK   
D781 PQ: 0 ANSI: 6
[  +0.000577] sd 6:0:0:0: Attached scsi generic sg3 type 0
[  +0.000199] sd 6:0:0:0: [sdc] Spinning up disk...
[  +0.059686] usb 2-2-port2: Cannot enable. Maybe the USB cable is bad?
[  +0.957465] .............ready
[ +12.207661] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000108] sd 6:0:0:0: [sdc] read_capacity_16: result 0, retries 2
[  +0.000005] sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 512, last 
LBA 3a3812aae
[  +0.000278] sd 6:0:0:0: [sdc] 15628053167 512-byte logical blocks: 
(8.00 TB/7.28 TiB)
[  +0.031496] sd 6:0:0:0: [sdc] Write Protect is off
[  +0.000014] sd 6:0:0:0: [sdc] Mode Sense: 4f 00 00 00
[  +0.000160] sd 6:0:0:0: [sdc] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  +0.000200] sd 6:0:0:0: [sdc] Preferred minimum I/O size 512 bytes
[  +0.000003] sd 6:0:0:0: [sdc] Optimal transfer size 33553920 bytes
[  +0.020620] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000075] sd 6:0:0:0: [sdc] read_capacity_16: result 0, retries 2
[  +0.000003] sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, 
last LBA 3a3812aae
[  +0.021509] sd 6:0:0:0: [sdc] sd_read_capacity: rc10_first 0, 
rc16_first: 1
[  +0.000077] sd 6:0:0:0: [sdc] read_capacity_16: result 0, retries 2
[  +0.000003] sd 6:0:0:0: [sdc] read_capacity_16: lbs 512, pbs 4096, 
last LBA 3a3812aae
[  +0.000595] sd 6:0:0:0: [sdc] Attached SCSI disk

dmesg does not seem too different, but lsblk's reporting is now correct:

# lsblk -t /dev/sdc
NAME ALIGNMENT MIN-IO OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE  RA WSAME
sdc          0   4096      0    4096     512    1 mq-deadline 60 128   32M


As you can see I have left the other two patches in too, not sure if I 
was supposed to do that.

Thank you so much. Any chance any of this could be backported?


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

* Re: Invalid optimal transfer size 33553920 accepted when physical_block_size 512
  2020-03-24 16:14                         ` Bernhard Sulzer
@ 2020-03-27  0:54                           ` Martin K. Petersen
  0 siblings, 0 replies; 19+ messages in thread
From: Martin K. Petersen @ 2020-03-27  0:54 UTC (permalink / raw)
  To: Bernhard Sulzer; +Cc: Martin K. Petersen, linux-scsi, Bryan Gurney


Bernhard,

> dmesg does not seem too different, but lsblk's reporting is now correct:
>
> # lsblk -t /dev/sdc
> NAME ALIGNMENT MIN-IO OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE  RA WSAME
> sdc          0   4096      0    4096     512    1 mq-deadline 60 128   32M
>
> Thank you so much. Any chance any of this could be backported?

Thanks for testing! It's tagged for stable.

-- 
Martin K. Petersen	Oracle Linux Engineering

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

end of thread, other threads:[~2020-03-27  0:59 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-22 14:32 Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Bernhard Sulzer
2020-03-22 15:45 ` Martin K. Petersen
     [not found]   ` <accd7d25-ee35-11b9-e49b-76e20d9550f2@gmail.com>
     [not found]     ` <yq1pnd4uxof.fsf@oracle.com>
2020-03-22 17:41       ` Bernhard Sulzer
     [not found]     ` <yq1pnd4tbxm.fsf@oracle.com>
2020-03-22 19:45       ` Bernhard Sulzer
2020-03-22 21:06         ` Martin K. Petersen
2020-03-22 21:20           ` Bernhard Sulzer
2020-03-22 21:53             ` Bernhard Sulzer
2020-03-22 22:45               ` Martin K. Petersen
2020-03-22 23:10                 ` Bernhard Sulzer
2020-03-22 23:22                 ` Bernhard Sulzer
2020-03-22 23:32                   ` Martin K. Petersen
2020-03-22 23:40                     ` Bernhard Sulzer
2020-03-23  1:41                       ` Martin K. Petersen
2020-03-24 13:49                         ` Bryan Gurney
2020-03-24 15:47                       ` [PATCH] scsi: sd: Fix optimal I/O size for devices that change reported values Martin K. Petersen
2020-03-24 15:52                       ` Invalid optimal transfer size 33553920 accepted when physical_block_size 512 Martin K. Petersen
2020-03-24 16:14                         ` Bernhard Sulzer
2020-03-27  0:54                           ` Martin K. Petersen
2020-03-22 20:57 ` [PATCH] scsi: sd: Optimal I/O size should be a multiple of reported granularity Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.