* [PATCH 1/4] nbd: add set cmd timeout helper
2019-08-13 16:39 [PATCH 0/4] nbd: cmd timeout fixes V2 Mike Christie
@ 2019-08-13 16:39 ` Mike Christie
2019-08-13 16:39 ` [PATCH 2/4] nbd: add function to convert blk req op to nbd cmd Mike Christie
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mike Christie @ 2019-08-13 16:39 UTC (permalink / raw)
To: axboe, josef, linux-block; +Cc: Mike Christie
Add a helper to set the cmd timeout. It does not really do a lot now,
but will be more useful in the next patches.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/block/nbd.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index e21d2ded732b..69d0e5260e1d 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1246,6 +1246,12 @@ static bool nbd_is_valid_blksize(unsigned long blksize)
return true;
}
+static void nbd_set_cmd_timeout(struct nbd_device *nbd, u64 timeout)
+{
+ nbd->tag_set.timeout = timeout * HZ;
+ blk_queue_rq_timeout(nbd->disk->queue, timeout * HZ);
+}
+
/* Must be called with config_lock held */
static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
unsigned int cmd, unsigned long arg)
@@ -1276,10 +1282,8 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
nbd_size_set(nbd, config->blksize, arg);
return 0;
case NBD_SET_TIMEOUT:
- if (arg) {
- nbd->tag_set.timeout = arg * HZ;
- blk_queue_rq_timeout(nbd->disk->queue, arg * HZ);
- }
+ if (arg)
+ nbd_set_cmd_timeout(nbd, arg);
return 0;
case NBD_SET_FLAGS:
@@ -1799,11 +1803,9 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
if (ret)
goto out;
- if (info->attrs[NBD_ATTR_TIMEOUT]) {
- u64 timeout = nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]);
- nbd->tag_set.timeout = timeout * HZ;
- blk_queue_rq_timeout(nbd->disk->queue, timeout * HZ);
- }
+ if (info->attrs[NBD_ATTR_TIMEOUT])
+ nbd_set_cmd_timeout(nbd,
+ nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]));
if (info->attrs[NBD_ATTR_DEAD_CONN_TIMEOUT]) {
config->dead_conn_timeout =
nla_get_u64(info->attrs[NBD_ATTR_DEAD_CONN_TIMEOUT]);
@@ -1971,11 +1973,9 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info)
if (ret)
goto out;
- if (info->attrs[NBD_ATTR_TIMEOUT]) {
- u64 timeout = nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]);
- nbd->tag_set.timeout = timeout * HZ;
- blk_queue_rq_timeout(nbd->disk->queue, timeout * HZ);
- }
+ if (info->attrs[NBD_ATTR_TIMEOUT])
+ nbd_set_cmd_timeout(nbd,
+ nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]));
if (info->attrs[NBD_ATTR_DEAD_CONN_TIMEOUT]) {
config->dead_conn_timeout =
nla_get_u64(info->attrs[NBD_ATTR_DEAD_CONN_TIMEOUT]);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] nbd: add function to convert blk req op to nbd cmd
2019-08-13 16:39 [PATCH 0/4] nbd: cmd timeout fixes V2 Mike Christie
2019-08-13 16:39 ` [PATCH 1/4] nbd: add set cmd timeout helper Mike Christie
@ 2019-08-13 16:39 ` Mike Christie
2019-08-13 16:39 ` [PATCH 3/4] nbd: add missing config put Mike Christie
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mike Christie @ 2019-08-13 16:39 UTC (permalink / raw)
To: axboe, josef, linux-block; +Cc: Mike Christie
This adds a helper function to convert a block req op to a nbd cmd type.
It will be used in the last patch to log the type in the timeout
handler.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/block/nbd.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 69d0e5260e1d..c6ff8f922fd7 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -344,6 +344,22 @@ static void sock_shutdown(struct nbd_device *nbd)
dev_warn(disk_to_dev(nbd->disk), "shutting down sockets\n");
}
+static u32 req_to_nbd_cmd_type(struct request *req)
+{
+ switch (req_op(req)) {
+ case REQ_OP_DISCARD:
+ return NBD_CMD_TRIM;
+ case REQ_OP_FLUSH:
+ return NBD_CMD_FLUSH;
+ case REQ_OP_WRITE:
+ return NBD_CMD_WRITE;
+ case REQ_OP_READ:
+ return NBD_CMD_READ;
+ default:
+ return U32_MAX;
+ }
+}
+
static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
bool reserved)
{
@@ -480,22 +496,9 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd, int index)
iov_iter_kvec(&from, WRITE, &iov, 1, sizeof(request));
- switch (req_op(req)) {
- case REQ_OP_DISCARD:
- type = NBD_CMD_TRIM;
- break;
- case REQ_OP_FLUSH:
- type = NBD_CMD_FLUSH;
- break;
- case REQ_OP_WRITE:
- type = NBD_CMD_WRITE;
- break;
- case REQ_OP_READ:
- type = NBD_CMD_READ;
- break;
- default:
+ type = req_to_nbd_cmd_type(req);
+ if (type == U32_MAX)
return -EIO;
- }
if (rq_data_dir(req) == WRITE &&
(config->flags & NBD_FLAG_READ_ONLY)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] nbd: add missing config put
2019-08-13 16:39 [PATCH 0/4] nbd: cmd timeout fixes V2 Mike Christie
2019-08-13 16:39 ` [PATCH 1/4] nbd: add set cmd timeout helper Mike Christie
2019-08-13 16:39 ` [PATCH 2/4] nbd: add function to convert blk req op to nbd cmd Mike Christie
@ 2019-08-13 16:39 ` Mike Christie
2019-08-13 16:39 ` [PATCH 4/4] nbd: fix zero cmd timeout handling v2 Mike Christie
2019-08-20 18:44 ` [PATCH 0/4] nbd: cmd timeout fixes V2 Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Mike Christie @ 2019-08-13 16:39 UTC (permalink / raw)
To: axboe, josef, linux-block; +Cc: Mike Christie
Fix bug added with the patch:
commit 8f3ea35929a0806ad1397db99a89ffee0140822a
Author: Josef Bacik <josef@toxicpanda.com>
Date: Mon Jul 16 12:11:35 2018 -0400
nbd: handle unexpected replies better
where if the timeout handler runs when the completion path is and we fail
to grab the mutex in the timeout handler we will leave a config reference
and cannot free the config later.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/block/nbd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c6ff8f922fd7..ebc98cf76365 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -373,8 +373,10 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
}
config = nbd->config;
- if (!mutex_trylock(&cmd->lock))
+ if (!mutex_trylock(&cmd->lock)) {
+ nbd_config_put(nbd);
return BLK_EH_RESET_TIMER;
+ }
if (config->num_connections > 1) {
dev_err_ratelimited(nbd_to_dev(nbd),
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] nbd: fix zero cmd timeout handling v2
2019-08-13 16:39 [PATCH 0/4] nbd: cmd timeout fixes V2 Mike Christie
` (2 preceding siblings ...)
2019-08-13 16:39 ` [PATCH 3/4] nbd: add missing config put Mike Christie
@ 2019-08-13 16:39 ` Mike Christie
2019-08-20 18:44 ` [PATCH 0/4] nbd: cmd timeout fixes V2 Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Mike Christie @ 2019-08-13 16:39 UTC (permalink / raw)
To: axboe, josef, linux-block; +Cc: Mike Christie
This fixes a regression added in 4.9 with commit:
commit 0eadf37afc2500e1162c9040ec26a705b9af8d47
Author: Josef Bacik <jbacik@fb.com>
Date: Thu Sep 8 12:33:40 2016 -0700
nbd: allow block mq to deal with timeouts
where before the patch userspace would set the timeout to 0 to disable
it. With the above patch, a zero timeout tells the block layer to use
the default value of 30 seconds. For setups where commands can take a
long time or experience transient issues like network disruptions this
then results in IO errors being sent to the application.
To fix this, the patch still uses the common block layer timeout
framework, but if zero is set, nbd just logs a message and then resets
the timer when it expires.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
V2 - We used to allow apps to reset the timeout to 0. The first version
of the patch only allowed it to be initialized to 0, so this adds the
reset feature back.
drivers/block/nbd.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index ebc98cf76365..98c618e5732c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -121,6 +121,7 @@ struct nbd_cmd {
struct mutex lock;
int index;
int cookie;
+ int retries;
blk_status_t status;
unsigned long flags;
u32 cmd_cookie;
@@ -407,10 +408,25 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
nbd_config_put(nbd);
return BLK_EH_DONE;
}
- } else {
- dev_err_ratelimited(nbd_to_dev(nbd),
- "Connection timed out\n");
}
+
+ if (!nbd->tag_set.timeout) {
+ /*
+ * Userspace sets timeout=0 to disable socket disconnection,
+ * so just warn and reset the timer.
+ */
+ cmd->retries++;
+ dev_info(nbd_to_dev(nbd), "Possible stuck request %p: control (%s@%llu,%uB). Runtime %u seconds\n",
+ req, nbdcmd_to_ascii(req_to_nbd_cmd_type(req)),
+ (unsigned long long)blk_rq_pos(req) << 9,
+ blk_rq_bytes(req), (req->timeout / HZ) * cmd->retries);
+
+ mutex_unlock(&cmd->lock);
+ nbd_config_put(nbd);
+ return BLK_EH_RESET_TIMER;
+ }
+
+ dev_err_ratelimited(nbd_to_dev(nbd), "Connection timed out\n");
set_bit(NBD_TIMEDOUT, &config->runtime_flags);
cmd->status = BLK_STS_IOERR;
mutex_unlock(&cmd->lock);
@@ -531,6 +547,7 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd, int index)
}
cmd->index = index;
cmd->cookie = nsock->cookie;
+ cmd->retries = 0;
request.type = htonl(type | nbd_cmd_flags);
if (type != NBD_CMD_FLUSH) {
request.from = cpu_to_be64((u64)blk_rq_pos(req) << 9);
@@ -1254,7 +1271,8 @@ static bool nbd_is_valid_blksize(unsigned long blksize)
static void nbd_set_cmd_timeout(struct nbd_device *nbd, u64 timeout)
{
nbd->tag_set.timeout = timeout * HZ;
- blk_queue_rq_timeout(nbd->disk->queue, timeout * HZ);
+ if (timeout)
+ blk_queue_rq_timeout(nbd->disk->queue, timeout * HZ);
}
/* Must be called with config_lock held */
@@ -1287,8 +1305,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
nbd_size_set(nbd, config->blksize, arg);
return 0;
case NBD_SET_TIMEOUT:
- if (arg)
- nbd_set_cmd_timeout(nbd, arg);
+ nbd_set_cmd_timeout(nbd, arg);
return 0;
case NBD_SET_FLAGS:
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] nbd: cmd timeout fixes V2
2019-08-13 16:39 [PATCH 0/4] nbd: cmd timeout fixes V2 Mike Christie
` (3 preceding siblings ...)
2019-08-13 16:39 ` [PATCH 4/4] nbd: fix zero cmd timeout handling v2 Mike Christie
@ 2019-08-20 18:44 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2019-08-20 18:44 UTC (permalink / raw)
To: Mike Christie, josef, linux-block
On 8/13/19 10:39 AM, Mike Christie wrote:
> The following patches fix bugs related to the nbd cmd timeout feature.
>
> The patches were made over Linus's current tree and also apply over
> this nbd patch:
>
> https://marc.info/?l=linux-block&m=156494582914495&w=2
>
> V2:
> - Fix patch4 so it allows resetting of the nbd cmd timeout to 0 as well
> initializing it to 0.
> - Add Josef's revewied-by tag.
Applied for 5.4, thanks Mike.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread