[PATCH v2 1/1] sd: add missing scenario for sd_config_write_same

[PATCH v2 1/1] sd: add missing scenario for sd_config_write_same

[tom.ty89]

From: Tom Yan <tom.ty89@gmail.com>

Example:

[root@localhost ~]# sg_opcodes /dev/sdb > /dev/null
Report supported operation codes: Illegal request, invalid opcode

[root@localhost ~]# sg_vpd -p bl /dev/sdb | grep 'write same'
  Maximum write same length: 0x0 blocks

[root@localhost ~]# cat /sys/block/sdb/queue/write_same_max_bytes
33553920

Signed-off-by: Tom Yan <tom.ty89@gmail.com>

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d749da7..1179ec1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -807,6 +807,8 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
 	if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS)
 		sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks,
 						   (u32)SD_MAX_WS16_BLOCKS);
+	else if (sdkp->max_ws_blocks == 0 && sdkp->device->no_report_opcodes)
+		sdkp->device->no_write_same = 1;
 	else if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes)
 		sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks,
 						   (u32)SD_MAX_WS10_BLOCKS);
-- 
2.7.1

Re: [PATCH v2 1/1] sd: add missing scenario for sd_config_write_same

[Martin K. Petersen]

>>>>> "tom" == tom ty89 <tom.ty89@gmail.com> writes:

Tom,

tom> [root@localhost ~]# sg_opcodes /dev/sdb > /dev/null Report
tom> supported operation codes: Illegal request, invalid opcode

tom> [root@localhost ~]# sg_vpd -p bl /dev/sdb | grep 'write same'
tom> Maximum write same length: 0x0 blocks

"A MAXIMUM WRITE SAME LENGTH field set to zero indicates that the device
server does not report a limit on the number of logical blocks that it
allows to be unmapped or written in a single WRITE SAME command."

I.e. that parameter says nothing about whether WRITE SAME is supported
or not.

tom> [root@localhost ~]# cat /sys/block/sdb/queue/write_same_max_bytes
tom> 33553920

That's correct behavior. Unless otherwise noted we default to issuing 32
MB WRITE SAME commands and will subsequently turn it off if the drive
returns a failure.

WRITE SAME support on storage devices predates RSOC, most of the common
VPD pages, etc. So there is no reliable value we can key off of to
determine whether a drive supports the command. And because a block
count of 0 is interpreted as "the entire device" we can't issue a
non-destructive WRITE SAME to determine whether device supports it or
not. Consequently we are stuck with assuming it works unless RSOC is
supported or the device sits behind a SATL.

Do you have a particular drive that is causing problems? We could quirk
it or add additional heuristics in addition to the ATA Information VPD.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 1/1] sd: add missing scenario for sd_config_write_same

[Tom Yan]

On 27 February 2016 at 06:12, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> "A MAXIMUM WRITE SAME LENGTH field set to zero indicates that the device
> server does not report a limit on the number of logical blocks that it
> allows to be unmapped or written in a single WRITE SAME command."
>
> I.e. that parameter says nothing about whether WRITE SAME is supported
> or not.
>

Fair enough.

>
> Do you have a particular drive that is causing problems? We could quirk
> it or add additional heuristics in addition to the ATA Information VPD.
>

Nah so far the device never showed any problems. It's actually a
SATA/USB adapter with UAS support. In fact it is seemingly coherent to
the heuristics:

[tom@localhost ~]$ sudo sg_vpd /dev/sdb
Supported VPD pages VPD page:
  Supported VPD pages [sv]
  Unit serial number [sn]
  Device identification [di]
  Block limits (SBC) [bl]

[tom@localhost ~]$ sudo sg_vpd -p bl /dev/sdb
Block limits VPD page (SBC):
  Write same non-zero (WSNZ): 0
  Maximum compare and write length: 0 blocks
  Optimal transfer length granularity: 1 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

Since you've mentioned it, why do we disable write same for all
devices with a ATA Information VPD?

Re: [PATCH v2 1/1] sd: add missing scenario for sd_config_write_same

[Martin K. Petersen]

>>>>> "Tom" == Tom Yan <tom.ty89@gmail.com> writes:

Tom> Since you've mentioned it, why do we disable write same for all
Tom> devices with a ATA Information VPD?

Generally speaking, SAS controllers that properly translate SCSI WRITE
SAME to ATA ditto also support modern features such as the REPORT
SUPPORTED OPERATION CODES command.

For the SAS controllers that do not support RSOC we check for the ATA
Information VPD page to determine whether we are talking to a SAS or
SATA device. If it's a SATA device we skip WRITE SAME because in most
cases either the controller's SATL or the drive itself will fail
miserably.

-- 
Martin K. Petersen	Oracle Linux Engineering