Re: [PATCH] scsi/arcmsr: Add timeout module parameter

2014-08-19 Thread Christoph Hellwig 
On Tue, Aug 05, 2014 at 04:10:08PM +0300, Ari Sundholm wrote:
 
 On Mon, 4 Aug 2014, Christoph Hellwig wrote:
 
  To modify the timeout on a queue please use blk_queue_rq_timeout in
  the slave_configure method instead of poking directly into the block
  timer, which won't work e.g. for the blk-mq path.
 
 I see. It seems the way the driver Areca offers on its download page does 
 this is wrong, then, as all I did was I ported the feature from there.
  
  But this really needs an explanation on why you'd need a configurable
  timeout to start with.
 
 A colleague of mine has experienced problems with disks not supporting 
 TLER performing lengthy error recovery, hitting a timeout. He has 
 mitigated this problem by configuring the timeout to be long enough to 
 allow for the error recovery to finish. And again, the driver offered by 
 Areca supports this feature, so it seems they have their reasons as well.

Can you test this patch:

http://git.infradead.org/users/hch/scsi-queue.git/commitdiff/3a7fcf9e5f57469fab78d6b6db81fbef6fa48a0d

which is part of the latest Areca code drop?  I don't really like the
timeout parameter, but if there's no better work around for you we'll
probablyneed to merge something.  In that case I'd love a patch ontop of
the branch that the above patches is in, which used the timeout
manipulation method I mentioned in my previous mail.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi/arcmsr: Add timeout module parameter

2014-08-05 Thread Ari Sundholm 

On Mon, 4 Aug 2014, Christoph Hellwig wrote:

 To modify the timeout on a queue please use blk_queue_rq_timeout in
 the slave_configure method instead of poking directly into the block
 timer, which won't work e.g. for the blk-mq path.

I see. It seems the way the driver Areca offers on its download page does 
this is wrong, then, as all I did was I ported the feature from there.
 
 But this really needs an explanation on why you'd need a configurable
 timeout to start with.

A colleague of mine has experienced problems with disks not supporting 
TLER performing lengthy error recovery, hitting a timeout. He has 
mitigated this problem by configuring the timeout to be long enough to 
allow for the error recovery to finish. And again, the driver offered by 
Areca supports this feature, so it seems they have their reasons as well.

Best regards,
Ari Sundholm
asund...@cs.hut.fi

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi/arcmsr: Add timeout module parameter

2014-08-04 Thread Christoph Hellwig 
To modify the timeout on a queue please use blk_queue_rq_timeout in
the slave_configure method instead of poking directly into the block
timer, which won't work e.g. for the blk-mq path.

But this really needs an explanation on why you'd need a configurable
timeout to start with.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi/arcmsr: Add timeout module parameter

2014-08-02 Thread Ari Sundholm 
From: Ari Sundholm asund...@cs.hut.fi

Backported from Areca's current version of the driver.

Signed-off-by: Ari Sundholm asund...@cs.hut.fi
---
 drivers/scsi/arcmsr/arcmsr_hba.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 652b41b..e79b62b 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -79,6 +79,9 @@ MODULE_VERSION(ARCMSR_DRIVER_VERSION);
 #defineARCMSR_RETRYCOUNT   12
 
 wait_queue_head_t wait_q;
+static int timeout;
+module_param(timeout, int, 0644);
+MODULE_PARM_DESC(timeout,  scsi cmd timeout value (0 - 120 sec.));
 static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
struct scsi_cmnd *cmd);
 static int arcmsr_iop_confirm(struct AdapterControlBlock *acb);
@@ -1151,6 +1154,14 @@ static int arcmsr_build_ccb(struct AdapterControlBlock 
*acb,
if (pcmd-sc_data_direction == DMA_TO_DEVICE)
arcmsr_cdb-Flags |= ARCMSR_CDB_FLAG_WRITE;
ccb-arc_cdb_size = arccdbsize;
+
+   if (timeout) {
+   pcmd-request-deadline = jiffies + timeout * HZ;
+   if (pcmd-device-request_queue-timeout.function)
+   mod_timer(pcmd-device-request_queue-timeout,
+ pcmd-request-deadline);
+   }
+
return SUCCESS;
 }
 
-- 
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html