diff mbox

[v2,03/30] cxlflash: Fix read capacity timeout

Message ID 1442438806-49198-1-git-send-email-mrochs@linux.vnet.ibm.com (mailing list archive)
State Superseded
Headers show

Commit Message

Matthew R. Ochs Sept. 16, 2015, 9:26 p.m. UTC
From: Manoj Kumar <kumarmn@us.ibm.com>

The timeout value for read capacity is too small. Certain devices
may take longer to respond and thus the command may prematurely
timeout. Additionally the literal used for the timeout is stale.

Update the timeout to 30 seconds (matches the value used in sd.c)
and rework the timeout literal to a more appropriate description.

Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
Suggested-by: Brian King <brking@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/superpipe.c | 9 ++++-----
 drivers/scsi/cxlflash/superpipe.h | 2 +-
 drivers/scsi/cxlflash/vlun.c      | 4 ++--
 3 files changed, 7 insertions(+), 8 deletions(-)

Comments

Brian King Sept. 18, 2015, 1:21 a.m. UTC | #1
On 09/16/2015 04:26 PM, Matthew R. Ochs wrote:
> @@ -296,7 +296,7 @@ static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
>  	int rc = 0;
>  	int result = 0;
>  	int retry_cnt = 0;
> -	u32 tout = (MC_DISCOVERY_TIMEOUT * HZ);
> +	u32 to = (CMD_TIMEOUT * HZ);
> 
>  retry:
>  	cmd_buf = kzalloc(CMD_BUFSIZE, GFP_KERNEL);
> @@ -315,8 +315,7 @@ retry:
>  		retry_cnt ? "re" : "", scsi_cmd[0]);
> 
>  	result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
> -			      CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
> -			      0, NULL);
> +			      CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
> 
>  	if (driver_byte(result) == DRIVER_SENSE) {
>  		result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
> @@ -1376,8 +1375,8 @@ out_attach:
>  	attach->block_size = gli->blk_len;
>  	attach->mmio_size = sizeof(afu->afu_map->hosts[0].harea);
>  	attach->last_lba = gli->max_lba;
> -	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT) /
> -		gli->blk_len;
> +	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT);
> +	attach->max_xfer /= gli->blk_len;

This change and the one above are not really part of the patch. Not a big deal, but
in future would be good to either call out the fact that there are a couple of unrelated
formatting changes, or keep them out and stick them in a separate cleanup patch.

Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
Tomas Henzl Sept. 21, 2015, 11:36 a.m. UTC | #2
On 16.9.2015 23:26, Matthew R. Ochs wrote:
> From: Manoj Kumar <kumarmn@us.ibm.com>
>
> The timeout value for read capacity is too small. Certain devices
> may take longer to respond and thus the command may prematurely
> timeout. Additionally the literal used for the timeout is stale.
>
> Update the timeout to 30 seconds (matches the value used in sd.c)
> and rework the timeout literal to a more appropriate description.
>
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
> Suggested-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>  drivers/scsi/cxlflash/superpipe.c | 9 ++++-----
>  drivers/scsi/cxlflash/superpipe.h | 2 +-
>  drivers/scsi/cxlflash/vlun.c      | 4 ++--
>  3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
> index 7df985d..fa513ba 100644
> --- a/drivers/scsi/cxlflash/superpipe.c
> +++ b/drivers/scsi/cxlflash/superpipe.c
> @@ -296,7 +296,7 @@ static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
>  	int rc = 0;
>  	int result = 0;
>  	int retry_cnt = 0;
> -	u32 tout = (MC_DISCOVERY_TIMEOUT * HZ);
> +	u32 to = (CMD_TIMEOUT * HZ);

In V3 please remove the parenthesis here^ 

>  
>  retry:
>  	cmd_buf = kzalloc(CMD_BUFSIZE, GFP_KERNEL);
...

> @@ -1376,8 +1375,8 @@ out_attach:
>  	attach->block_size = gli->blk_len;
>  	attach->mmio_size = sizeof(afu->afu_map->hosts[0].harea);
>  	attach->last_lba = gli->max_lba;
> -	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT) /
> -		gli->blk_len;
> +	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT);

and here^ too.

Thanks,
Tomas
Matthew R. Ochs Sept. 21, 2015, 10:11 p.m. UTC | #3
> On Sep 21, 2015, at 6:36 AM, Tomas Henzl <thenzl@redhat.com> wrote:
> On 16.9.2015 23:26, Matthew R. Ochs wrote:
>> From: Manoj Kumar <kumarmn@us.ibm.com>
>> 
>> The timeout value for read capacity is too small. Certain devices
>> may take longer to respond and thus the command may prematurely
>> timeout. Additionally the literal used for the timeout is stale.
>> 
>> Update the timeout to 30 seconds (matches the value used in sd.c)
>> and rework the timeout literal to a more appropriate description.
>> 
>> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
>> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
>> Suggested-by: Brian King <brking@linux.vnet.ibm.com>
>> ---
>> drivers/scsi/cxlflash/superpipe.c | 9 ++++-----
>> drivers/scsi/cxlflash/superpipe.h | 2 +-
>> drivers/scsi/cxlflash/vlun.c      | 4 ++--
>> 3 files changed, 7 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
>> index 7df985d..fa513ba 100644
>> --- a/drivers/scsi/cxlflash/superpipe.c
>> +++ b/drivers/scsi/cxlflash/superpipe.c
>> @@ -296,7 +296,7 @@ static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
>> 	int rc = 0;
>> 	int result = 0;
>> 	int retry_cnt = 0;
>> -	u32 tout = (MC_DISCOVERY_TIMEOUT * HZ);
>> +	u32 to = (CMD_TIMEOUT * HZ);
> 
> In V3 please remove the parenthesis here^

Sure.

> 
>> 
>> retry:
>> 	cmd_buf = kzalloc(CMD_BUFSIZE, GFP_KERNEL);
> ...
> 
>> @@ -1376,8 +1375,8 @@ out_attach:
>> 	attach->block_size = gli->blk_len;
>> 	attach->mmio_size = sizeof(afu->afu_map->hosts[0].harea);
>> 	attach->last_lba = gli->max_lba;
>> -	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT) /
>> -		gli->blk_len;
>> +	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT);
> 
> and here^ too.

done.
diff mbox

Patch

diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index 7df985d..fa513ba 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -296,7 +296,7 @@  static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
 	int rc = 0;
 	int result = 0;
 	int retry_cnt = 0;
-	u32 tout = (MC_DISCOVERY_TIMEOUT * HZ);
+	u32 to = (CMD_TIMEOUT * HZ);
 
 retry:
 	cmd_buf = kzalloc(CMD_BUFSIZE, GFP_KERNEL);
@@ -315,8 +315,7 @@  retry:
 		retry_cnt ? "re" : "", scsi_cmd[0]);
 
 	result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
-			      CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
-			      0, NULL);
+			      CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
 
 	if (driver_byte(result) == DRIVER_SENSE) {
 		result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
@@ -1376,8 +1375,8 @@  out_attach:
 	attach->block_size = gli->blk_len;
 	attach->mmio_size = sizeof(afu->afu_map->hosts[0].harea);
 	attach->last_lba = gli->max_lba;
-	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT) /
-		gli->blk_len;
+	attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT);
+	attach->max_xfer /= gli->blk_len;
 
 out:
 	attach->adap_fd = fd;
diff --git a/drivers/scsi/cxlflash/superpipe.h b/drivers/scsi/cxlflash/superpipe.h
index 3f7856b..fffb179 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -28,7 +28,7 @@  extern struct cxlflash_global global;
 */
 #define MC_CHUNK_SIZE     (1 << MC_RHT_NMASK)	/* in LBAs */
 
-#define MC_DISCOVERY_TIMEOUT 5  /* 5 secs */
+#define CMD_TIMEOUT 30  /* 30 secs */
 #define CMD_RETRIES 5   /* 5 retries for scsi_execute */
 
 #define MAX_SECTOR_UNIT  512 /* max_sector is in 512 byte multiples */
diff --git a/drivers/scsi/cxlflash/vlun.c b/drivers/scsi/cxlflash/vlun.c
index 6d6608b..68994c4 100644
--- a/drivers/scsi/cxlflash/vlun.c
+++ b/drivers/scsi/cxlflash/vlun.c
@@ -414,7 +414,7 @@  static int write_same16(struct scsi_device *sdev,
 	int ws_limit = SISLITE_MAX_WS_BLOCKS;
 	u64 offset = lba;
 	int left = nblks;
-	u32 tout = sdev->request_queue->rq_timeout;
+	u32 to = sdev->request_queue->rq_timeout;
 	struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)sdev->host->hostdata;
 	struct device *dev = &cfg->dev->dev;
 
@@ -434,7 +434,7 @@  static int write_same16(struct scsi_device *sdev,
 				   &scsi_cmd[10]);
 
 		result = scsi_execute(sdev, scsi_cmd, DMA_TO_DEVICE, cmd_buf,
-				      CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
+				      CMD_BUFSIZE, sense_buf, to, CMD_RETRIES,
 				      0, NULL);
 		if (result) {
 			dev_err_ratelimited(dev, "%s: command failed for "