Skip to contentSkip to conversation

block: add io timeout to sysfs

RFC PATCH Superseded Weiping Zhang 4 participants 5 messages Nov 19, 2018 – Nov 28, 2018 Block layer

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Weiping Zhang

author
#0 lore raw

RFC PATCH v2 1/1 block: add io timeout to sysfs

Give a interface to adjust io timeout by device.

Signed-off-by Weiping Zhang <[email protected]>

---

Changes since v1:
* make sure timeout > 0

 block/blk-sysfs.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
block/blk-sysfs.c +27 -0
index 80eef48fddc8..90a927514d30 100644
@@ -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, &val);
+	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[] = {
 	&queue_dax_entry.attr,
 	&queue_wb_lat_entry.attr,
 	&queue_poll_delay_entry.attr,
+	&queue_io_timeout_entry.attr,
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
 	&throtl_sample_time_entry.attr,
 #endif
Signature · 1 line
2.14.1
#1 lore raw

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

Quoted 67 lines · Weiping Zhang <[email protected]> 于2018年11月19日周一 下午10:30写道：

Give a interface to adjust io timeout by device.

Signed-off-by Weiping Zhang <[email protected]>

---

Changes since v1:
* make sure timeout > 0

 block/blk-sysfs.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
block/blk-sysfs.c +27 -1
index 80eef48fddc8..90a927514d30 100644
@@ -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, &val);
+       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[] = {
        &queue_dax_entry.attr,
        &queue_wb_lat_entry.attr,
        &queue_poll_delay_entry.attr,
+       &queue_io_timeout_entry.attr,
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
        &throtl_sample_time_entry.attr,
 #endif
--

2.14.1

maintainer
#2 lore raw
Quoted 4 lines · Weiping Zhang

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.

Signature · 1 line
Jens Axboe
#3 lore raw
In reply to Weiping Zhang
Quoted 9 lines · Weiping Zhang

Give a interface to adjust io timeout by device.

Signed-off-by Weiping Zhang <[email protected]>>---

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.

Quoted 7 lines
+-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?

Thanks,

Bart.

maintainer
#4 lore raw
Quoted 9 lines · Bart Van Assche
Quoted 8 lines · Weiping Zhang

Give a interface to adjust io timeout by device.

Signed-off-by Weiping Zhang <[email protected]>

---

Changes since v1:
* make sure timeout > 0

Quoted 6 lines
Quoted 2 lines
 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.

Quoted 11 lines
Quoted 11 lines
block/blk-sysfs.c +11 -0
+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);
Quoted 8 lines

block/blk-sysfs.c static ssize_t queue_poll_store(struct request_queue *q, const char *page, · line 438

Quoted 3 lines
block/blk-sysfs.c +3 -0
+
+	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.

Signature · 1 line
Jens Axboe