linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fc transport: correct default value for fc_dev_loss_tmo
@ 2006-05-11  9:23 Andreas Herrmann
  2006-05-11 14:37 ` James Smart
  0 siblings, 1 reply; 2+ messages in thread
From: Andreas Herrmann @ 2006-05-11  9:23 UTC (permalink / raw)
  To: James Bottomley, James Smart; +Cc: Linux SCSI

In current kernels initialization of fc_dev_loss_tmo is wrong.
See fc_remote_port_delete():

    int timeout = rport->dev_loss_tmo;
...
    fc_queue_devloss_work(shost, &rport->dev_loss_work, timeout * HZ);

So actually a timeout of ((HZ*60)*HZ) is used or with James S's recent
patch we have a timeout of ((HZ*120)*HZ). I think this was not
intentional, so I suggest the following patch to correct this.


Regards,

Andreas


fc transport: correct default value for fc_dev_loss_tmo

Signed-off-by: Andreas Herrmann <aherrman@de.ibm.com>


diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 95c5478..a1cb05e 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -368,7 +368,7 @@ static DECLARE_TRANSPORT_CLASS(fc_rport_
  *   should insulate the loss of a remote port.
  *   The maximum will be capped by the value of SCSI_DEVICE_BLOCK_MAX_TIMEOUT.
  */
-static unsigned int fc_dev_loss_tmo = SCSI_DEVICE_BLOCK_MAX_TIMEOUT;
+static unsigned int fc_dev_loss_tmo = SCSI_DEVICE_BLOCK_MAX_TIMEOUT / HZ;
 
 module_param_named(dev_loss_tmo, fc_dev_loss_tmo, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(dev_loss_tmo,

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

* Re: [PATCH] fc transport: correct default value for fc_dev_loss_tmo
  2006-05-11  9:23 [PATCH] fc transport: correct default value for fc_dev_loss_tmo Andreas Herrmann
@ 2006-05-11 14:37 ` James Smart
  0 siblings, 0 replies; 2+ messages in thread
From: James Smart @ 2006-05-11 14:37 UTC (permalink / raw)
  To: Andreas Herrmann; +Cc: James Bottomley, Linux SCSI

NACK.

The real problem is in the define for SCSI_DEVICE_BLOCK_MAX_TIMEOUT.
If you look at every use within the fc transport, including the set
function and when it actually sets the timeout, it expects dev_loss_tmo
to be in units of seconds. Thus, SCSI_DEVICE_BLOCK_MAX_TIMEOUT should have
been the same thing.

I'll update my patch shortly.

-- james s

Andreas Herrmann wrote:
> In current kernels initialization of fc_dev_loss_tmo is wrong.
> See fc_remote_port_delete():
> 
>     int timeout = rport->dev_loss_tmo;
> ...
>     fc_queue_devloss_work(shost, &rport->dev_loss_work, timeout * HZ);
> 
> So actually a timeout of ((HZ*60)*HZ) is used or with James S's recent
> patch we have a timeout of ((HZ*120)*HZ). I think this was not
> intentional, so I suggest the following patch to correct this.
> 
> 
> Regards,
> 
> Andreas
> 
> 
> fc transport: correct default value for fc_dev_loss_tmo
> 
> Signed-off-by: Andreas Herrmann <aherrman@de.ibm.com>
> 
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 95c5478..a1cb05e 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -368,7 +368,7 @@ static DECLARE_TRANSPORT_CLASS(fc_rport_
>   *   should insulate the loss of a remote port.
>   *   The maximum will be capped by the value of SCSI_DEVICE_BLOCK_MAX_TIMEOUT.
>   */
> -static unsigned int fc_dev_loss_tmo = SCSI_DEVICE_BLOCK_MAX_TIMEOUT;
> +static unsigned int fc_dev_loss_tmo = SCSI_DEVICE_BLOCK_MAX_TIMEOUT / HZ;
>  
>  module_param_named(dev_loss_tmo, fc_dev_loss_tmo, int, S_IRUGO|S_IWUSR);
>  MODULE_PARM_DESC(dev_loss_tmo,
> 

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

end of thread, other threads:[~2006-05-11 14:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-11  9:23 [PATCH] fc transport: correct default value for fc_dev_loss_tmo Andreas Herrmann
2006-05-11 14:37 ` James Smart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).