Re: Re: [PATCH] Remove capping from dev_loss_tmo

Mike Christie
2009-12-17T02:43:43Z

Mike Christie wrote:
> Hannes Reinecke wrote:
>> Mike Christie wrote:
>>> Hannes Reinecke wrote:
>>>> diff --git a/drivers/scsi/scsi_transport_fc.c
>>>> b/drivers/scsi/scsi_transport_fc.c
>>>> index 573ce21..6f39bf4 100644
>>>> --- a/drivers/scsi/scsi_transport_fc.c
>>>> +++ b/drivers/scsi/scsi_transport_fc.c
>>>> @@ -475,7 +475,8 @@ MODULE_PARM_DESC(dev_loss_tmo,
>>>> "Maximum number of seconds that the FC transport should"
>>>> " insulate the loss of a remote port. Once this value is"
>>>> " exceeded, the scsi target is removed. Value should be"
>>>> - " between 1 and SCSI_DEVICE_BLOCK_MAX_TIMEOUT.");
>>>> + " between 1 and SCSI_DEVICE_BLOCK_MAX_TIMEOUT if"
>>>> + " fast_io_fail_tmo is not set.");
>>>>
>>> What was the largest timeout value you tried? I think I am hitting a bug
>>> with iscsi where if you pass queue_delayed_work a large enough value it
>>> will overflow and instead of getting 100 minutes you get 1.
>>
>> Well, 0x7fffffff works, 0xffffffff is rejected with -EINVAL.
>> And so should every other value with the top bit set.
>>
>
> If someone did 0x7fffffff, then we do 0x7fffffff * HZ in
>
> fc_queue_devloss_work(shost, &rport->dev_loss_work, timeout * HZ);
>
> on a 32bit box, it won't overflow to the value the user wanted will it?
> I think it will not happen normally, but users will try it like they
> have with iscsi and they will end up not knowing how to set it really high.
>

Oh yeah, I think the idea of the patch is right. If I am reading the
code right, I was just worried about how to handle that issue.