On 07/03/13 17:14, David Dillow wrote:
On Wed, 2013-07-03 at 14:54 +0200, Bart Van Assche wrote:+int srp_tmo_valid(int fast_io_fail_tmo, int dev_loss_tmo) +{ + return (fast_io_fail_tmo < 0 || dev_loss_tmo < 0 || + fast_io_fail_tmo < dev_loss_tmo) && + fast_io_fail_tmo <= SCSI_DEVICE_BLOCK_MAX_TIMEOUT && + dev_loss_tmo < LONG_MAX / HZ ? 0 : -EINVAL; +} +EXPORT_SYMBOL_GPL(srp_tmo_valid);This would have been more readable: int srp_tmo_valid(int fast_io_fail_tmp, int dev_loss_tmo) { /* Fast IO fail must be off, or no greater than the max timeout */ if (fast_io_fail_tmo > SCSI_DEVICE_BLOCK_MAX_TIMEOUT) return -EINVAL; /* Device timeout must be off, or fit into jiffies */ if (dev_loss_tmo >= LONG_MAX / HZ) return -EINVAL; /* Fast IO must trigger before device loss, or one of the * timeouts must be disabled. */ if (fast_io_fail_tmo < 0 || dev_loss_tmo < 0) return 0; if (fast_io_fail < dev_loss_tmo) return 0; return -EINVAL; }
Isn't that a matter of personal taste which of the above two is more clear ? It might also depend on the number of mathematics courses in someones educational background :-)
Though, now that I've unpacked it -- I don't think it is OK for dev_loss_tmo to be off, but fast IO to be on? That drops another conditional.
The combination of dev_loss_tmo off and reconnect_delay > 0 worked fine in my tests. An I/O failure was detected shortly after the cable to the target was pulled. I/O resumed shortly after the cable to the target was reinserted.
Also, FC caps dev_loss_tmo at SCSI_DEVICE_BLOCK_MAX_TIMEOUT if fail_io_fast_tmo is off; I agree with your reasoning about leaving it unlimited if fast fail is on, but does that still hold if it is off?
I think setting dev_loss_tmo to a large value only makes sense if the value of reconnect_delay is not too large. Setting both to a large value would result in slow recovery after a transport layer failure has been corrected.
Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
