[PATCH] scsi/arcmsr: Add timeout module parameter

[PATCH] scsi/arcmsr: Add timeout module parameter

[Ari Sundholm]

From: Ari Sundholm <asundhol@cs.hut.fi>

Backported from Areca's current version of the driver.

Signed-off-by: Ari Sundholm <asundhol@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);
 #define	ARCMSR_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

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

[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.

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

[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
asundhol@cs.hut.fi

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

[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.