On 07/19/2013 11:17 PM, Benjamin Marzinski wrote:
> On Tue, Jul 16, 2013 at 09:12:55AM +0200, Hannes Reinecke wrote:
>> There are several constraints when setting fast_io_fail and
>> dev_loss_tmo.
>> dev_loss_tmo will be capped automatically when fast_io_fail is
>> not set. And fast_io_fail can not be raised beyond dev_loss_tmo.
>>
>> So to increase dev_loss_tmo and fast_io_fail we first need
>> to increase dev_loss_tmo to the given fast_io_fail
>> setting, then set fast_io_fail, and then set dev_loss_tmo
>> to the final setting.
>
> This patch seems kind of convoluted to me, and even with the fix to
> temporarily set dev_loss_tmo to one greater than fast_io_fail_tmo,
> there is still a broken case
>
> If you currently have:
> fast_io_fail_tmo off
> dev_loss_tmo <something_less_than_600>
>
> and you want
>
> fast_io_fail_tmo 600
> dev_loss_tmo <something_greater_than_600>
>
> This will fail, since it will try to set dev_loss_tmo to 601 with
> fast_io_fail_tmo set to off (Granted, I doubt that 600 is a
> popular fast_io_fail_tmo value).
>
> But in the general case, If you aren't turning off fast_io_fail_tmo and
> the current dev_loss_tmo is greater than the target fast_io_fail_tmo
> (this seems like it is the most common case), you unecessarily go through
> the work of first setting dev_loss_tmo to its current value.
>
> if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
> mpp->fast_io_fail != MP_FAST_IO_FAIL_ZERO &&
> mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
> /* Check if we need to temporarily increase dev_loss_tmo
> * */
> ret = sysfs_attr_get_value(rport_dev, "dev_loss_tmo",
> value, 16);
>
> <SNIP>
>
> }
> if (strlen(value)) {
> ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
> value, strlen(value));
>
> I have a patch that solves this issue differently. It still checks
> dev_loss_tmo, but it always sets fast_io_fail_tmo first. If the target
> fast_io_fail_tmo is greater than or equal to the current dev_loss_tmo,
> it sets fast_io_fail_tmo to one less than the current dev_loss_tmo.
> This always works, since the lowest value possible for dev_loss_tmo is 1
> and the lowest value possible for fast_io_fail_tmo is 0. Then it sets
> dev_loss_tmo. Finally, if necessary, it sets fast_io_fail_tmo to its
> final value.
>
Ah.
So you're doing (worst case)
-> set fast_io_fail
-> set dev_loss_tmo
-> set fast_io_fail
and I'm doing
-> set dev_loss_tmo
-> set fast_io_fail
-> set dev_loss_tmo
So the only difference here is the '+- 1' offset to fast_io_fail or
dev_loss_tmo ...
Whatever.
Waiting for your patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
