Skip to contentSkip to conversation

fc transport: correct default value for fc_dev_loss_tmo

PATCH NAK Andreas Herrmann 2 participants 2 messages May 11, 2006 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fc transport: correct default value for fc_dev_loss_tmo

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

drivers/scsi/scsi_transport_fc.c +1 -1
index 95c5478..a1cb05e 100644
@@ -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,
#1 lore raw

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

Quoted 36 lines · Andreas Herrmann

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

drivers/scsi/scsi_transport_fc.c +1 -1
index 95c5478..a1cb05e 100644
@@ -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,