[PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[David Sebek]

The 5TB 2.5-inch WD Black P10 external USB hard drive seems
to use SMR technology. It supports TRIM via the unmap operation.
Maybe because it is marketed as a drive for gaming that is
compatible with PlayStation, Xbox, PC and Mac, it does not
support UASP protocol but uses bulk-only transport.
Therefore, Linux does not attempt to read the VPD and does
not enable TRIM by default. (Currently, there is a bug and
Linux incorrectly enables a writesame_16 TRIM operation on
the drive and does not change it to the correct unmap TRIM
operation because it does not attempt to read the info from
the VPD. I already submitted a patch for that bug.)

This patch adds this drive to the scsi_static_device_list
with a BLIST_TRY_VPD_PAGES flag. Although there are comments
in the code indicating that this list is deprecated and that
'echo "WD:Game Drive:0x10000400" > /proc/scsi/device_info'
should be used instead, I haven't found a better place to
persist this information. Moreover, the list already contains
a similar entry for the SanDisk Cruzer Blade USB flash drive.

Signed-off-by: David Sebek <dasebek@gmail.com>
---
 drivers/scsi/scsi_devinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index d92cec12454c..3ed558c168be 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -256,6 +256,7 @@ static struct {
 	{"WangDAT", "Model 2600", "01.7", BLIST_SELECT_NO_ATN},
 	{"WangDAT", "Model 3200", "02.2", BLIST_SELECT_NO_ATN},
 	{"WangDAT", "Model 1300", "02.4", BLIST_SELECT_NO_ATN},
+	{"WD", "Game Drive", NULL, BLIST_TRY_VPD_PAGES | BLIST_INQUIRY_36},
 	{"WDC WD25", "00JB-00FUA0", NULL, BLIST_NOREPORTLUN},
 	{"XYRATEX", "RS", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
 	{"Zzyzx", "RocketStor 500S", NULL, BLIST_SPARSELUN},
-- 
2.31.1

Re: [PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[Martin K. Petersen]

David,

>(Currently, there is a bug and Linux incorrectly enables a writesame_16
>TRIM operation on the drive

This is intentional as we support devices which conform to an earlier
version of the spec that did not have the LBP VPD indicating which
command to use for discards.

I have a patch impending that postpones enabling WRITE SAME until after
all VPD pages have been queried. That gives us a slightly better
heuristic and removes a window of error for devices that report
conflicting limits for UNMAP and WRITE SAME.

> This patch adds this drive to the scsi_static_device_list
> with a BLIST_TRY_VPD_PAGES flag. Although there are comments
> in the code indicating that this list is deprecated and that
> 'echo "WD:Game Drive:0x10000400" > /proc/scsi/device_info'
> should be used instead, I haven't found a better place to
> persist this information.





Moreover, the list already contains
> a similar entry for the SanDisk Cruzer Blade USB flash drive.
>
> Signed-off-by: David Sebek <dasebek@gmail.com>
> ---
>  drivers/scsi/scsi_devinfo.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> index d92cec12454c..3ed558c168be 100644
> --- a/drivers/scsi/scsi_devinfo.c
> +++ b/drivers/scsi/scsi_devinfo.c
> @@ -256,6 +256,7 @@ static struct {
>  	{"WangDAT", "Model 2600", "01.7", BLIST_SELECT_NO_ATN},
>  	{"WangDAT", "Model 3200", "02.2", BLIST_SELECT_NO_ATN},
>  	{"WangDAT", "Model 1300", "02.4", BLIST_SELECT_NO_ATN},
> +	{"WD", "Game Drive", NULL, BLIST_TRY_VPD_PAGES | BLIST_INQUIRY_36},
>  	{"WDC WD25", "00JB-00FUA0", NULL, BLIST_NOREPORTLUN},
>  	{"XYRATEX", "RS", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
>  	{"Zzyzx", "RocketStor 500S", NULL, BLIST_SPARSELUN},

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[Martin K. Petersen]

David,

Sorry for the partial mail, my VPN froze at an inopportune moment.

>> This patch adds this drive to the scsi_static_device_list
>> with a BLIST_TRY_VPD_PAGES flag. Although there are comments
>> in the code indicating that this list is deprecated and that
>> 'echo "WD:Game Drive:0x10000400" > /proc/scsi/device_info'
>> should be used instead, I haven't found a better place to
>> persist this information.

We just try to limit adding entries to cases where we fail to establish
a good heuristic.

>> +	{"WD", "Game Drive", NULL, BLIST_TRY_VPD_PAGES | BLIST_INQUIRY_36},

Is BLIST_INQUIRY_36 really needed?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[David Sebek]

Martin,

Thank you for the clarification.

>> Is BLIST_INQUIRY_36 really needed?

No, BLIST_INQUIRY_36 is not needed. I included it only because the nearby
comment "Note that all USB devices should have the BLIST_INQUIRY_36 flag."
said so.

David Sebek

Re: [PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[Martin K. Petersen]

David,

> No, BLIST_INQUIRY_36 is not needed. I included it only because the
> nearby comment "Note that all USB devices should have the
> BLIST_INQUIRY_36 flag."  said so.

I would appreciate it if you could send me the output of:

# sg_inq /dev/sdN
# sg_readcap -l /dev/sdN
# sg_vpd -p bl /dev/sdN
# sg_vpd -p lbpv /dev/sdN

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[David Sebek]

[-- Attachment #1: Type: text/plain, Size: 52 bytes --]

Here is the output you requested.

Thank you,
David

[-- Attachment #2: bhd3.txt --]
[-- Type: text/plain, Size: 2509 bytes --]

sudo sg_inq /dev/sda

standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  LU_CONG=0  version=0x06  [SPC-4]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=1  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=0
  [SPI: Clocking=0x0  QAS=0  IUS=0]
    length=96 (0x60)   Peripheral device type: disk
 Vendor identification: WD      
 Product identification: Game Drive      
 Product revision level: 5002
 Unit serial number: WXF2E20DKCFL    


sudo sg_readcap -l /dev/sda

Read Capacity results:
   Protection: prot_en=0, p_type=0, p_i_exponent=0
   Logical block provisioning: lbpme=1, lbprz=0
   Last LBA=9767475199 (0x2462fd7ff), Number of logical blocks=9767475200
   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: 5000947302400 bytes, 4769275.0 MiB, 5000.95 GB, 5.00 TB


sudo sg_vpd -p bl /dev/sda

Block limits VPD page (SBC):
  Write same non-zero (WSNZ): 0
  Maximum compare and write length: 0 blocks [Command not implemented]
  Optimal transfer length granularity: 8 blocks
  Maximum transfer length: 2048 blocks
  Optimal transfer length: 2048 blocks
  Maximum prefetch transfer length: 65535 blocks
  Maximum unmap LBA count: -1 [unbounded]
  Maximum unmap block descriptor count: 31
  Optimal unmap granularity: 0 blocks [not reported]
  Unmap granularity alignment valid: false
  Unmap granularity alignment: 0 [invalid]
  Maximum write same length: 0 blocks [not reported]
  Maximum atomic transfer length: 0 blocks [not reported]
  Atomic alignment: 0 [unaligned atomic writes permitted]
  Atomic transfer length granularity: 0 [no granularity requirement
  Maximum atomic transfer length with atomic boundary: 0 blocks [not reported]
  Maximum atomic boundary size: 0 blocks [can only write atomic 1 block]


sudo sg_vpd -p lbpv /dev/sda

Logical block provisioning VPD page (SBC):
  Unmap command supported (LBPU): 1
  Write same (16) with unmap bit supported (LBPWS): 0
  Write same (10) with unmap bit supported (LBPWS10): 0
  Logical block provisioning read zeros (LBPRZ): 0
  Anchored LBAs supported (ANC_SUP): 1
  Threshold exponent: 0 [threshold sets not supported]
  Descriptor present (DP): 0
  Minimum percentage: 0 [not reported]
  Provisioning type: 1 (resource provisioned)
  Threshold percentage: 0 [percentages not supported]

Re: [PATCH] scsi: Set BLIST_TRY_VPD_PAGES for WD Black P10 external HDD

[Martin K. Petersen]

David,

> Here is the output you requested.

Great, thanks! Looks like my discard negotiation should work in your
case. I have a couple more corner cases to fix before I post.

-- 
Martin K. Petersen	Oracle Linux Engineering