Re: [RFC PATCH v2] block: add io timeout to sysfs
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
>> ---
>>
>> 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, );
>> +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
Re: [RFC PATCH v2] block: add io timeout to sysfs
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
> ---
>
> 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.
> +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, );
> + 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?
Thanks,
Bart.
Re: [RFC PATCH v2] block: add io timeout to sysfs
On 11/28/18 7:52 AM, Weiping Zhang wrote: > Hi Jens, > > It's useful if user want a short timeout when a disk doesn't work normally. > Would you give some comments for this patch, I'm fine with the patch, in fact I've posted a similar/identical one in the past but just never got around to merging it. I think we should expose the value in seconds or msecs, however, not in jiffies. That makes it hard to use for people. So modify it to do a msecs_to_jiffies() on the value passed in. -- Jens Axboe
Re: [RFC PATCH v2] block: add io timeout to sysfs
Hi Jens,
It's useful if user want a short timeout when a disk doesn't work normally.
Would you give some comments for this patch,
Thanks a lot
Weiping Zhang 于2018年11月19日周一 下午10:30写道：
>
> Give a interface to adjust io timeout by device.
>
> Signed-off-by: Weiping Zhang
> ---
>
> Changes since v1:
> * make sure timeout > 0
>
> block/blk-sysfs.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 80eef48fddc8..90a927514d30 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -417,6 +417,26 @@ static ssize_t queue_poll_store(struct request_queue *q,
> const char *page,
> return ret;
> }
>
> +static ssize_t queue_io_timeout_show(struct request_queue *q, char *page)
> +{
> + return sprintf(page, "%u\n", q->rq_timeout);
> +}
> +
> +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, );
> + if (err || val == 0)
> + return -EINVAL;
> +
> + blk_queue_rq_timeout(q, val);
> +
> + return count;
> +}
> +
> static ssize_t queue_wb_lat_show(struct request_queue *q, char *page)
> {
> if (!wbt_rq_qos(q))
> @@ -685,6 +705,12 @@ static struct queue_sysfs_entry queue_dax_entry = {
> .show = queue_dax_show,
> };
>
> +static struct queue_sysfs_entry queue_io_timeout_entry = {
> + .attr = {.name = "io_timeout", .mode = 0644 },
> + .show = queue_io_timeout_show,
> + .store = queue_io_timeout_store,
> +};
> +
> static struct queue_sysfs_entry queue_wb_lat_entry = {
> .attr = {.name = "wbt_lat_usec", .mode = 0644 },
> .show = queue_wb_lat_show,
> @@ -734,6 +760,7 @@ static struct attribute *default_attrs[] = {
> _dax_entry.attr,
> _wb_lat_entry.attr,
> _poll_delay_entry.attr,
> + _io_timeout_entry.attr,
> #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
> _sample_time_entry.attr,
> #endif
> --
> 2.14.1
>
[RFC PATCH v2] block: add io timeout to sysfs
Give a interface to adjust io timeout by device.
Signed-off-by: Weiping Zhang
---
Changes since v1:
* make sure timeout > 0
block/blk-sysfs.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 80eef48fddc8..90a927514d30 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -417,6 +417,26 @@ static ssize_t queue_poll_store(struct request_queue *q,
const char *page,
return ret;
}
+static ssize_t queue_io_timeout_show(struct request_queue *q, char *page)
+{
+ return sprintf(page, "%u\n", q->rq_timeout);
+}
+
+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, );
+ if (err || val == 0)
+ return -EINVAL;
+
+ blk_queue_rq_timeout(q, val);
+
+ return count;
+}
+
static ssize_t queue_wb_lat_show(struct request_queue *q, char *page)
{
if (!wbt_rq_qos(q))
@@ -685,6 +705,12 @@ static struct queue_sysfs_entry queue_dax_entry = {
.show = queue_dax_show,
};
+static struct queue_sysfs_entry queue_io_timeout_entry = {
+ .attr = {.name = "io_timeout", .mode = 0644 },
+ .show = queue_io_timeout_show,
+ .store = queue_io_timeout_store,
+};
+
static struct queue_sysfs_entry queue_wb_lat_entry = {
.attr = {.name = "wbt_lat_usec", .mode = 0644 },
.show = queue_wb_lat_show,
@@ -734,6 +760,7 @@ static struct attribute *default_attrs[] = {
_dax_entry.attr,
_wb_lat_entry.attr,
_poll_delay_entry.attr,
+ _io_timeout_entry.attr,
#ifdef CONFIG_BLK_DEV_THROTTLING_LOW
_sample_time_entry.attr,
#endif
--
2.14.1
