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;
}
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.
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
