[PATCH] scsi: sd: Use UNMAP in case the device doesn't support WRITE_SAME

[PATCH] scsi: sd: Use UNMAP in case the device doesn't support WRITE_SAME

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

There exists a storage device that supports READ_CAPACITY, but doesn't
support WRITE_SAME. The problem is that WRITE SAME heuristics doesn't work
for this kind of storage device since its block limits VPD page doesn't
contain the LBP information. Currently we set its provisioning_mode
"writesame_16" and didn't check "no_write_same". If we didn't manually change
this default provisioning_mode to "unmap" through sysfs, provisioning_mode
will be set to "disabled" after the first WRITE_SAME command with the following
error occurs:

sd 0:0:0:3: [sdd] tag#4 UNKNOWN(0x2003) Result: hostbyte=0x00 driverbyte=0x08 cmd_age=0s
sd 0:0:0:3: [sdd] tag#4 Sense Key : 0x5 [current]
sd 0:0:0:3: [sdd] tag#4 ASC=0x20 ASCQ=0x0
sd 0:0:0:3: [sdd] tag#4 CDB: opcode=0x93 93 08 00 00 00 00 00 00 05 4b 00 00 00 1f 00 00
blk_update_request: critical target error, dev sdd, sector 10840 op 0x3:(DISCARD) flags 0x800 phys_seg 1 prio class 0

Comparing to manually change provisioning_mode in sysfs, it is better to set
provisioning_mode to "unmap" in case WRITE_SAME is not supported.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/sd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 95018e650f2d..93fb41741b21 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2372,7 +2372,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 		if (buffer[14] & 0x40) /* LBPRZ */
 			sdkp->lbprz = 1;
 
-		sd_config_discard(sdkp, SD_LBP_WS16);
+		if (sdp->no_write_same)
+			sd_config_discard(sdkp, SD_LBP_UNMAP);
+		else
+			sd_config_discard(sdkp, SD_LBP_WS16);
 	}
 
 	sdkp->capacity = lba + 1;
-- 
2.17.1

Re: [PATCH] scsi: sd: Use UNMAP in case the device doesn't support WRITE_SAME

[Martin K. Petersen]

Bean,

> There exists a storage device that supports READ_CAPACITY, but doesn't
> support WRITE_SAME. The problem is that WRITE SAME heuristics doesn't work
> for this kind of storage device since its block limits VPD page doesn't
> contain the LBP information. Currently we set its provisioning_mode
> "writesame_16" and didn't check "no_write_same".

There is something odd with what your device is reporting.

We support WRITE SAME on a bunch of devices that predate the Logical
Block Provisioning VPD page and the various Block Limits parameters
being introduced to the spec. Consequently we set the provisioning mode
to "writesame_16" if the device reports LBPME=1 in READ CAPACITY(16) and
nothing relevant is reported in the VPD pages. That is by design.

> If we didn't manually change this default provisioning_mode to "unmap"
> through sysfs, provisioning_mode will be set to "disabled" after the
> first WRITE_SAME command with the following error occurs:

If your device supports UNMAP it *must* report it in the Logical Block
Provisioning VPD by setting LBPU=1 and report MAXIMUM UNMAP LBA COUNT
and MAXIMUM UNMAP BLOCK DESCRIPTOR COUNT in the Block Limits VPD.

Also, "no_write_same" disables attempting to use WRITE SAME to zero
block ranges. That's orthogonal to the logic controlling which command
to use for performing an unmap operation. An unfortunate choice of
naming which can be attributed to the SCSI protocol using the WRITE SAME
command for two completely different operations.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: sd: Use UNMAP in case the device doesn't support WRITE_SAME

[Bean Huo]

On Wed, 2020-10-07 at 23:47 -0400, Martin K. Petersen wrote:
> > support WRITE_SAME. The problem is that WRITE SAME heuristics
> > doesn't work
> > for this kind of storage device since its block limits VPD page
> > doesn't
> > contain the LBP information. Currently we set its provisioning_mode
> > "writesame_16" and didn't check "no_write_same".
> 
> There is something odd with what your device is reporting.
> 
> We support WRITE SAME on a bunch of devices that predate the Logical
> Block Provisioning VPD page and the various Block Limits parameters
> being introduced to the spec. Consequently we set the provisioning
> mode
> to "writesame_16" if the device reports LBPME=1 in READ CAPACITY(16)
> and
> nothing relevant is reported in the VPD pages. That is by design.
> 
> > If we didn't manually change this default provisioning_mode to
> > "unmap"
> > through sysfs, provisioning_mode will be set to "disabled" after
> > the
> > first WRITE_SAME command with the following error occurs:
> 
> If your device supports UNMAP it *must* report it in the Logical
> Block
> Provisioning VPD by setting LBPU=1 and report MAXIMUM UNMAP LBA COUNT
> and MAXIMUM UNMAP BLOCK DESCRIPTOR COUNT in the Block Limits VPD.
> 
> Also, "no_write_same" disables attempting to use WRITE SAME to zero
> block ranges. That's orthogonal to the logic controlling which
> command
> to use for performing an unmap operation. An unfortunate choice of
> naming which can be attributed to the SCSI protocol using the WRITE
> SAME
> command for two completely different operations.
> 

Hi Martin
thanks very much for your above comments, very detailed. The case I
encountered is very strange to me as well. I will double check on the
product level.

thanks,
Bean