public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* Re: RAID-10 keeps aborting
       [not found]                   ` <51AF0CCF.8000909@itwm.fraunhofer.de>
@ 2013-06-05 11:38                     ` Bernd Schubert
  2013-06-05 12:53                       ` [PATCH] scsi: Check if the device support WRITE_SAME_10 Bernd Schubert
  2013-06-05 19:11                       ` RAID-10 keeps aborting Martin K. Petersen
  0 siblings, 2 replies; 7+ messages in thread
From: Bernd Schubert @ 2013-06-05 11:38 UTC (permalink / raw)
  Cc: Joe Lawrence, H. Peter Anvin, Martin K. Petersen, Dan Williams,
	linux-raid, linux-scsi

On 06/05/2013 12:02 PM, Bernd Schubert wrote:
> On 06/04/2013 05:39 PM, Joe Lawrence wrote:
>>
>> Just curious, what type drives were in your RAID and what does
>> /sys/class/scsi_disk/*/max_write_same_blocks report?  If you have a spare
>> drive to test, maybe you could try a quick sg_write_same command to see
>> how the drive reacts?
>>
>
> I just run into the same issue with an ancient system from 2006. Except
> that I'm in hurry an need it to stress-test my own work, I can do
> anything with it - it is booted via NFS and disks are only used for
> development/testing.
>
>> (squeeze)fslab1:~# cat /sys/block/md126/queue/write_same_max_bytes
>> 16384
>
>> (squeeze)fslab1:~# cat /sys/block/sd[o,n,m,l]/queue/write_same_max_bytes
>> 0
>> 0
>> 0
>> 0
>
>
> Ah, now I found the reason why it fails, scsi-layer had set
> write_same_max_bytes to zero when it detected that it does not support
> it, but after reloading the arecal module (arcmsr) I now get:
>
>> (squeeze)fslab1:~# cat /sys/block/sd[o,n,m,l]/queue/write_same_max_bytes
>> 33553920
>> 33553920
>> 33553920
>> 33553920

In sd_config_write_same() it sets

	if (sdkp->max_ws_blocks == 0)
		sdkp->max_ws_blocks = SD_MAX_WS10_BLOCKS;

except when sdkp->device->no_write_same is set.
But only ata_scsi_sdev_config() sets that. And I also don't see any 
driver setting max_ws_blocks, so everything except of libata gets the 
default of SD_MAX_WS10_BLOCKS. This also seems to be consistent with the 
33553920 I see, except that there is somewhere a bit shift.
So no surprise that mptsas and arcmsr (and anything else) devices have 
write_same_max_bytes set.
As the correct handling in the md layer seems to be difficult, can we 
send a fake request at device configuration time to figure out if the 
device really support write-same?



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

* [PATCH] scsi: Check if the device support WRITE_SAME_10
  2013-06-05 11:38                     ` RAID-10 keeps aborting Bernd Schubert
@ 2013-06-05 12:53                       ` Bernd Schubert
  2013-06-05 19:14                         ` Martin K. Petersen
  2013-06-05 19:11                       ` RAID-10 keeps aborting Martin K. Petersen
  1 sibling, 1 reply; 7+ messages in thread
From: Bernd Schubert @ 2013-06-05 12:53 UTC (permalink / raw)
  Cc: Joe Lawrence, H. Peter Anvin, Martin K. Petersen, Dan Williams,
	linux-raid, linux-scsi

Here's a rather simply patch for scsi-midlayer

checkpatch.pl complains about style issue, but 
I just did it as the other lines there.

> schubert@squeeze@fsdevel3 linux-stable>scripts/checkpatch.pl patches-linux-3.9.y/ws10 
> ERROR: spaces prohibited around that ':' (ctx:WxW)
> #48: FILE: drivers/scsi/sd.h:87:
> +       unsigned        ws10 : 1;
>                              ^

If someone wants me, I can send another patch to fix the other
lines first.



scsi: Check if the device support WRITE_SAME_10

From: Bernd Schubert <bernd.schubert@itwm.fraunhofer.de>

The md layer currently cannot handle failed WRITE_SAME commands
and the initial easiest fix is to check if the device supports
WRITE_SAME at all. It already tested for WRITE_SAME_16 and
this commit adds a test for WRITE_SAME_10.

Signed-off-by: Bernd Schubert <bernd.schubert@itwm.fraunhofer.de>
---
 drivers/scsi/sd.c |    6 +++++-
 drivers/scsi/sd.h |    1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 82910cc..368f0a4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -742,7 +742,7 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
 	unsigned int logical_block_size = sdkp->device->sector_size;
 	unsigned int blocks = 0;
 
-	if (sdkp->device->no_write_same) {
+	if (sdkp->device->no_write_same || !(sdkp->ws10 || sdkp->ws16)) {
 		sdkp->max_ws_blocks = 0;
 		goto out;
 	}
@@ -2648,6 +2648,10 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp)
 static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
 {
 	if (scsi_report_opcode(sdkp->device, buffer, SD_BUF_SIZE,
+			       WRITE_SAME))
+		sdkp->ws10 = 1;
+
+	if (scsi_report_opcode(sdkp->device, buffer, SD_BUF_SIZE,
 			       WRITE_SAME_16))
 		sdkp->ws16 = 1;
 }
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 2386aeb..7a049de 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -84,6 +84,7 @@ struct scsi_disk {
 	unsigned	lbpws : 1;
 	unsigned	lbpws10 : 1;
 	unsigned	lbpvpd : 1;
+	unsigned	ws10 : 1;
 	unsigned	ws16 : 1;
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,dev)

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

* Re: RAID-10 keeps aborting
  2013-06-05 11:38                     ` RAID-10 keeps aborting Bernd Schubert
  2013-06-05 12:53                       ` [PATCH] scsi: Check if the device support WRITE_SAME_10 Bernd Schubert
@ 2013-06-05 19:11                       ` Martin K. Petersen
  1 sibling, 0 replies; 7+ messages in thread
From: Martin K. Petersen @ 2013-06-05 19:11 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Joe Lawrence, H. Peter Anvin, Martin K. Petersen, Dan Williams,
	linux-raid, linux-scsi

>>>>> "Bernd" == Bernd Schubert <bernd.schubert@itwm.fraunhofer.de> writes:

Bernd> And I also don't see any driver setting max_ws_blocks, so
Bernd> everything except of libata gets the default of
Bernd> SD_MAX_WS10_BLOCKS. 

Yes. That's intentional. Unless the device provides MAXIMUM WRITE SAME
BLOCKS in the BLOCK LIMITS VPD.


Bernd> As the correct handling in the md layer seems to be difficult,
Bernd> can we send a fake request at device configuration time to figure
Bernd> out if the device really support write-same?

The problem is that WRITE SAME is destructive. And unfortunately a block
count of 0 means "write entire device".

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH] scsi: Check if the device support WRITE_SAME_10
  2013-06-05 12:53                       ` [PATCH] scsi: Check if the device support WRITE_SAME_10 Bernd Schubert
@ 2013-06-05 19:14                         ` Martin K. Petersen
  2013-06-05 20:09                           ` Bernd Schubert
  0 siblings, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2013-06-05 19:14 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Joe Lawrence, H. Peter Anvin, Martin K. Petersen, Dan Williams,
	linux-raid, linux-scsi

>>>>> "Bernd" == Bernd Schubert <bernd.schubert@itwm.fraunhofer.de> writes:

Bernd> The md layer currently cannot handle failed WRITE_SAME commands
Bernd> and the initial easiest fix is to check if the device supports
Bernd> WRITE_SAME at all. It already tested for WRITE_SAME_16 and this
Bernd> commit adds a test for WRITE_SAME_10.

No go. That'll disable WRITE SAME for drives which don't support
RSOC. Which means almost all of them.

I propose the following...

-- 
Martin K. Petersen	Oracle Linux Engineering


[PATCH] sd: Update WRITE SAME heuristics

SATA drives located behind a SAS controller would incorrectly receive
WRITE SAME commands. Tweak the heuristics so that:

 - If REPORT SUPPORTED OPERATION CODES is provided we will use that to
   choose between WRITE SAME(16), WRITE SAME(10) and disabled. This also
   fixes an issue with the old code which would issue WRITE SAME(10)
   despite the command not being whitelisted in REPORT SUPPORTED
   OPERATION CODES.

 - If REPORT SUPPORTED OPERATION CODES is not provided we will fall back
   to WRITE SAME(10) unless the device has an ATA Information VPD page.
   The assumption is that a SATL which is smart enough to implement
   WRITE SAME would also provide REPORT SUPPORTED OPERATION CODES.

To facilitate the new heuristics scsi_report_opcode() has been modified
to so we can distinguish between "operation not supported" and "RSOC not
supported".

Reported-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2c0d0ec..3b1ea34 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -1070,8 +1070,8 @@ EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
  * @opcode:	opcode for command to look up
  *
  * Uses the REPORT SUPPORTED OPERATION CODES to look up the given
- * opcode. Returns 0 if RSOC fails or if the command opcode is
- * unsupported. Returns 1 if the device claims to support the command.
+ * opcode. Returns -EINVAL if RSOC fails, 0 if the command opcode is
+ * unsupported and 1 if the device claims to support the command.
  */
 int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
 		       unsigned int len, unsigned char opcode)
@@ -1081,7 +1081,7 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
 	int result;
 
 	if (sdev->no_report_opcodes || sdev->scsi_level < SCSI_SPC_3)
-		return 0;
+		return -EINVAL;
 
 	memset(cmd, 0, 16);
 	cmd[0] = MAINTENANCE_IN;
@@ -1097,7 +1097,7 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
 	if (result && scsi_sense_valid(&sshdr) &&
 	    sshdr.sense_key == ILLEGAL_REQUEST &&
 	    (sshdr.asc == 0x20 || sshdr.asc == 0x24) && sshdr.ascq == 0x00)
-		return 0;
+		return -EINVAL;
 
 	if ((buffer[1] & 3) == 3) /* Command supported */
 		return 1;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a37eda9..366b661 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -442,8 +442,15 @@ sd_store_write_same_blocks(struct device *dev, struct device_attribute *attr,
 
 	if (max == 0)
 		sdp->no_write_same = 1;
-	else if (max <= SD_MAX_WS16_BLOCKS)
-		sdkp->max_ws_blocks = max;
+	else
+		sdp->no_write_same = 0;
+
+	if (sdkp->ws16)
+		sdkp->max_ws_blocks =
+			max_t(unsigned long, max, SD_MAX_WS16_BLOCKS);
+	else
+		sdkp->max_ws_blocks =
+			max_t(unsigned long, max, SD_MAX_WS10_BLOCKS);
 
 	sd_config_write_same(sdkp);
 
@@ -762,16 +769,16 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
 	 * blocks per I/O unless the device explicitly advertises a
 	 * bigger limit.
 	 */
-	if (sdkp->max_ws_blocks == 0)
-		sdkp->max_ws_blocks = SD_MAX_WS10_BLOCKS;
-
-	if (sdkp->ws16 || sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS)
+	if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS)
 		blocks = min_not_zero(sdkp->max_ws_blocks,
 				      (u32)SD_MAX_WS16_BLOCKS);
 	else
 		blocks = min_not_zero(sdkp->max_ws_blocks,
 				      (u32)SD_MAX_WS10_BLOCKS);
 
+	if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes)
+		sdkp->max_ws_blocks = blocks;
+
 out:
 	blk_queue_max_write_same_sectors(q, blocks * (logical_block_size >> 9));
 }
@@ -2645,9 +2652,24 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp)
 
 static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
 {
-	if (scsi_report_opcode(sdkp->device, buffer, SD_BUF_SIZE,
-			       WRITE_SAME_16))
+	struct scsi_device *sdev = sdkp->device;
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) {
+		sdev->no_report_opcodes = 1;
+
+		/* Disable WRITE SAME if REPORT SUPPORTED OPERATION
+		 * CODES is unsupported and the device has an ATA
+		 * Information VPD page (SAT).
+		 */
+		if (!scsi_get_vpd_page(sdev, 0x89, buffer, SD_BUF_SIZE))
+			sdev->no_write_same = 1;
+	}
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME_16) == 1)
 		sdkp->ws16 = 1;
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME) == 1)
+		sdkp->ws10 = 1;
 }
 
 static int sd_try_extended_inquiry(struct scsi_device *sdp)
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 2386aeb..7a049de 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -84,6 +84,7 @@ struct scsi_disk {
 	unsigned	lbpws : 1;
 	unsigned	lbpws10 : 1;
 	unsigned	lbpvpd : 1;
+	unsigned	ws10 : 1;
 	unsigned	ws16 : 1;
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,dev)

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

* Re: [PATCH] scsi: Check if the device support WRITE_SAME_10
  2013-06-05 19:14                         ` Martin K. Petersen
@ 2013-06-05 20:09                           ` Bernd Schubert
  2013-06-07  2:15                             ` Martin K. Petersen
  0 siblings, 1 reply; 7+ messages in thread
From: Bernd Schubert @ 2013-06-05 20:09 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Joe Lawrence, H. Peter Anvin, Dan Williams, linux-raid,
	linux-scsi

On 06/05/2013 09:14 PM, Martin K. Petersen wrote:>>>>>> "Bernd" == Bernd Schubert <bernd.schubert@itwm.fraunhofer.de> writes:
> 
> Bernd> The md layer currently cannot handle failed WRITE_SAME commands
> Bernd> and the initial easiest fix is to check if the device supports
> Bernd> WRITE_SAME at all. It already tested for WRITE_SAME_16 and this
> Bernd> commit adds a test for WRITE_SAME_10.
> 
> No go. That'll disable WRITE SAME for drives which don't support
> RSOC. Which means almost all of them.

Ah, sorry, I didn't check the specs.

> 
> I propose the following...
> 

> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -442,8 +442,15 @@ sd_store_write_same_blocks(struct device *dev, struct device_attribute *attr,
>  
>  	if (max == 0)
>  		sdp->no_write_same = 1;
> -	else if (max <= SD_MAX_WS16_BLOCKS)
> -		sdkp->max_ws_blocks = max;
> +	else
> +		sdp->no_write_same = 0;
> +
> +	if (sdkp->ws16)
> +		sdkp->max_ws_blocks =
> +			max_t(unsigned long, max, SD_MAX_WS16_BLOCKS);
> +	else
> +		sdkp->max_ws_blocks =
> +			max_t(unsigned long, max, SD_MAX_WS10_BLOCKS);
>  
>  	sd_config_write_same(sdkp);

Max? Not min_t()?


> @@ -762,16 +769,16 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
>  	 * blocks per I/O unless the device explicitly advertises a
>  	 * bigger limit.
>  	 */
> -	if (sdkp->max_ws_blocks == 0)
> -		sdkp->max_ws_blocks = SD_MAX_WS10_BLOCKS;
> -
> -	if (sdkp->ws16 || sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS)
> +	if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS)
>  		blocks = min_not_zero(sdkp->max_ws_blocks,
>  				      (u32)SD_MAX_WS16_BLOCKS);
>  	else
>  		blocks = min_not_zero(sdkp->max_ws_blocks,
>  				      (u32)SD_MAX_WS10_BLOCKS);
>  
> +	if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes)
> +		sdkp->max_ws_blocks = blocks;
> +
>  out:
>  	blk_queue_max_write_same_sectors(q, blocks * (logical_block_size >> 9));
>  }

blk_queue_max_write_same_sectors(q, sdkp->max_ws_blocks * (logical_block_size >> 9)) ?
Otherwise sdkp->max_ws_blocks and the queue might have different values, wouldn't they?


I cant't provide a comment about scsi_get_vpd_page, I simply don't know. You certainly
know the scsi specs ways better than I do!


Thanks,
Bernd




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

* Re: [PATCH] scsi: Check if the device support WRITE_SAME_10
  2013-06-05 20:09                           ` Bernd Schubert
@ 2013-06-07  2:15                             ` Martin K. Petersen
  2013-06-12 19:34                               ` Bernd Schubert
  0 siblings, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2013-06-07  2:15 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Martin K. Petersen, Joe Lawrence, H. Peter Anvin, Dan Williams,
	linux-raid, linux-scsi

>>>>> "Bernd" == Bernd Schubert <bernd.schubert@itwm.fraunhofer.de> writes:

>> max_t(unsigned long, max, SD_MAX_WS10_BLOCKS);

Bernd> Max? Not min_t()?

Brain fart. Updated patch with a few other adjustments.

I have tested this on a couple of JBODs with a mishmash of SATA and SAS
drives, including a few specimens that report MAX WRITE SAME BLOCKS.

-- 
Martin K. Petersen	Oracle Linux Engineering


[PATCH] sd: Update WRITE SAME heuristics

SATA drives located behind a SAS controller would incorrectly receive
WRITE SAME commands. Tweak the heuristics so that:

 - If REPORT SUPPORTED OPERATION CODES is provided we will use that to
   choose between WRITE SAME(16), WRITE SAME(10) and disabled. This also
   fixes an issue with the old code which would issue WRITE SAME(10)
   despite the command not being whitelisted in REPORT SUPPORTED
   OPERATION CODES.

 - If REPORT SUPPORTED OPERATION CODES is not provided we will fall back
   to WRITE SAME(10) unless the device has an ATA Information VPD page.
   The assumption is that a SATL which is smart enough to implement
   WRITE SAME would also provide REPORT SUPPORTED OPERATION CODES.

To facilitate the new heuristics scsi_report_opcode() has been modified
to so we can distinguish between "operation not supported" and "RSOC not
supported".

Reported-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: <stable@vger.kernel.org>

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2c0d0ec..3b1ea34 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -1070,8 +1070,8 @@ EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
  * @opcode:	opcode for command to look up
  *
  * Uses the REPORT SUPPORTED OPERATION CODES to look up the given
- * opcode. Returns 0 if RSOC fails or if the command opcode is
- * unsupported. Returns 1 if the device claims to support the command.
+ * opcode. Returns -EINVAL if RSOC fails, 0 if the command opcode is
+ * unsupported and 1 if the device claims to support the command.
  */
 int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
 		       unsigned int len, unsigned char opcode)
@@ -1081,7 +1081,7 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
 	int result;
 
 	if (sdev->no_report_opcodes || sdev->scsi_level < SCSI_SPC_3)
-		return 0;
+		return -EINVAL;
 
 	memset(cmd, 0, 16);
 	cmd[0] = MAINTENANCE_IN;
@@ -1097,7 +1097,7 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
 	if (result && scsi_sense_valid(&sshdr) &&
 	    sshdr.sense_key == ILLEGAL_REQUEST &&
 	    (sshdr.asc == 0x20 || sshdr.asc == 0x24) && sshdr.ascq == 0x00)
-		return 0;
+		return -EINVAL;
 
 	if ((buffer[1] & 3) == 3) /* Command supported */
 		return 1;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a37eda9..420e763 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -442,8 +442,10 @@ sd_store_write_same_blocks(struct device *dev, struct device_attribute *attr,
 
 	if (max == 0)
 		sdp->no_write_same = 1;
-	else if (max <= SD_MAX_WS16_BLOCKS)
+	else if (max <= SD_MAX_WS16_BLOCKS) {
+		sdp->no_write_same = 0;
 		sdkp->max_ws_blocks = max;
+	}
 
 	sd_config_write_same(sdkp);
 
@@ -750,7 +752,6 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
 {
 	struct request_queue *q = sdkp->disk->queue;
 	unsigned int logical_block_size = sdkp->device->sector_size;
-	unsigned int blocks = 0;
 
 	if (sdkp->device->no_write_same) {
 		sdkp->max_ws_blocks = 0;
@@ -762,18 +763,20 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
 	 * blocks per I/O unless the device explicitly advertises a
 	 * bigger limit.
 	 */
-	if (sdkp->max_ws_blocks == 0)
-		sdkp->max_ws_blocks = SD_MAX_WS10_BLOCKS;
-
-	if (sdkp->ws16 || sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS)
-		blocks = min_not_zero(sdkp->max_ws_blocks,
-				      (u32)SD_MAX_WS16_BLOCKS);
-	else
-		blocks = min_not_zero(sdkp->max_ws_blocks,
-				      (u32)SD_MAX_WS10_BLOCKS);
+	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->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes)
+		sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks,
+						   (u32)SD_MAX_WS10_BLOCKS);
+	else {
+		sdkp->device->no_write_same = 1;
+		sdkp->max_ws_blocks = 0;
+	}
 
 out:
-	blk_queue_max_write_same_sectors(q, blocks * (logical_block_size >> 9));
+	blk_queue_max_write_same_sectors(q, sdkp->max_ws_blocks *
+					 (logical_block_size >> 9));
 }
 
 /**
@@ -2645,9 +2648,24 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp)
 
 static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
 {
-	if (scsi_report_opcode(sdkp->device, buffer, SD_BUF_SIZE,
-			       WRITE_SAME_16))
+	struct scsi_device *sdev = sdkp->device;
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) {
+		sdev->no_report_opcodes = 1;
+
+		/* Disable WRITE SAME if REPORT SUPPORTED OPERATION
+		 * CODES is unsupported and the device has an ATA
+		 * Information VPD page (SAT).
+		 */
+		if (!scsi_get_vpd_page(sdev, 0x89, buffer, SD_BUF_SIZE))
+			sdev->no_write_same = 1;
+	}
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME_16) == 1)
 		sdkp->ws16 = 1;
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME) == 1)
+		sdkp->ws10 = 1;
 }
 
 static int sd_try_extended_inquiry(struct scsi_device *sdp)
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 2386aeb..7a049de 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -84,6 +84,7 @@ struct scsi_disk {
 	unsigned	lbpws : 1;
 	unsigned	lbpws10 : 1;
 	unsigned	lbpvpd : 1;
+	unsigned	ws10 : 1;
 	unsigned	ws16 : 1;
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,dev)

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

* Re: [PATCH] scsi: Check if the device support WRITE_SAME_10
  2013-06-07  2:15                             ` Martin K. Petersen
@ 2013-06-12 19:34                               ` Bernd Schubert
  0 siblings, 0 replies; 7+ messages in thread
From: Bernd Schubert @ 2013-06-12 19:34 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Joe Lawrence, H. Peter Anvin, Dan Williams, linux-raid,
	linux-scsi

On 06/07/2013 04:15 AM, Martin K. Petersen wrote:
>>>>>> "Bernd" == Bernd Schubert <bernd.schubert@itwm.fraunhofer.de> writes:
>
>>> max_t(unsigned long, max, SD_MAX_WS10_BLOCKS);
>
> Bernd> Max? Not min_t()?
>
> Brain fart. Updated patch with a few other adjustments.
>
> I have tested this on a couple of JBODs with a mishmash of SATA and SAS
> drives, including a few specimens that report MAX WRITE SAME BLOCKS.
>

Thanks for the update!

I'm far too long at work again, but I managed to test it now and it 
works fine for the ancient areca controller (ARC-1260) of this test-lab. 
So you may add

Tested-by: Bernd Schubert <bernd.schubert@itwm.fraunhofer.de>

and

Reviewed-by: Bernd Schubert <bernd.schubert@itwm.fraunhofer.de>

(although the latter probably does not count much for linux-scsi).


Cheers,
Bernd

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

end of thread, other threads:[~2013-06-12 19:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <51AC1440.7020505@zytor.com>
     [not found] ` <CAA9_cmddLfReYeAhgwh5=j6ELMBNx5Oq7Gg8K+fo0PneaEfrVA@mail.gmail.com>
     [not found]   ` <51AC3283.4000403@zytor.com>
     [not found]     ` <CAA9_cme6tYpYnrZDbrDduwPCjVn+PFbx_rZNPFazBEU9EF0upw@mail.gmail.com>
     [not found]       ` <51ACBAA0.40604@zytor.com>
     [not found]         ` <CAA9_cmc3Gs91C4aV6okUw-=q+fACm1+dooyafOZi+Lnj+Ne_ig@mail.gmail.com>
     [not found]           ` <51ACD511.4030604@zytor.com>
     [not found]             ` <yq1y5art543.fsf@sermon.lab.mkp.net>
     [not found]               ` <51AD2485.9000601@zytor.com>
     [not found]                 ` <alpine.DEB.2.02.1306041132260.19072@jlaw-desktop.mno.stratus.com>
     [not found]                   ` <51AF0CCF.8000909@itwm.fraunhofer.de>
2013-06-05 11:38                     ` RAID-10 keeps aborting Bernd Schubert
2013-06-05 12:53                       ` [PATCH] scsi: Check if the device support WRITE_SAME_10 Bernd Schubert
2013-06-05 19:14                         ` Martin K. Petersen
2013-06-05 20:09                           ` Bernd Schubert
2013-06-07  2:15                             ` Martin K. Petersen
2013-06-12 19:34                               ` Bernd Schubert
2013-06-05 19:11                       ` RAID-10 keeps aborting Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox