Re: [RFC PATCH v2] block: add io timeout to sysfs

[Jens Axboe <axboe@kernel.dk>]

On 11/28/18 9:13 AM, Bart Van Assche wrote:
> On Mon, 2018-11-19 at 22:11 +0800, Weiping Zhang wrote:
>> Give a interface to adjust io timeout by device.
>>
>> Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com>
>> ---
>>
>> Changes since v1:
>> * make sure timeout > 0
>>
>>  block/blk-sysfs.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
> 
> Documentation for new block layer sysfs attributes should be added in
> Documentation/ABI/testing/sysfs-block and also in Documentation/block/queue-sysfs.txt.
> Please add such documentation for this new attribute.

Yes, please send a followup patch to add the documentation.

>> +static ssize_t queue_io_timeout_store(struct request_queue *q, const char *page,
>> +				  size_t count)
>> +{
>> +	unsigned int val;
>> +	int err;
>> +
>> +	err = kstrtou32(page, 10, &val);
>> +	if (err || val == 0)
>> +		return -EINVAL;
>> +
>> +	blk_queue_rq_timeout(q, val);
>> +
>> +	return count;
>> +}
> 
> Setting the block layer timeout to a very high value (e.g. hours) may make it look
> like a request got stuck without users having an easy way of figuring out what is
> going on. I'm wondering whether this function should restrict the upper bound for
> block layer timeouts. How about limiting timeout values to ten minutes?

This is no different than folks using SG_IO/bsg and putting a high timeout
in their commands. I don't think we should impose a limit, if you set it
high, you get exactly what you asked for.

-- 
Jens Axboe