Re: [PATCH 3/3] nbd: introduce a client flag to do zero timeout handling
On 9/21/20 6:57 AM, Hou Pu wrote:
Introduce a dedicated client flag NBD_RT_WAIT_ON_TIMEOUT to reset
timer in nbd_xmit_timer instead of depending on tag_set.timeout == 0.
So that the timeout value could be configured by the user to
whatever they like instead of the default 30s. A smaller timeout
value allow us to detect if a new socket is reconfigured in a
shorter time. Thus the io could be requeued more quickly.
The tag_set.timeout == 0 setting still works like before.
Signed-off-by: Hou Pu <houpu@bytedance.com>
I like this idea in general, just a few comments below.
---
drivers/block/nbd.c | 25 ++++++++++++++++++++++++-
include/uapi/linux/nbd.h | 4 ++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 4c0bbb981cbc..1d7a9de7bdcc 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -80,6 +80,7 @@ struct link_dead_args {
#define NBD_RT_BOUND 5
#define NBD_RT_DESTROY_ON_DISCONNECT 6
#define NBD_RT_DISCONNECT_ON_CLOSE 7
+#define NBD_RT_WAIT_ON_TIMEOUT 8
#define NBD_DESTROY_ON_DISCONNECT 0
#define NBD_DISCONNECT_REQUESTED 1
@@ -378,6 +379,16 @@ static u32 req_to_nbd_cmd_type(struct request *req)
}
}
+static inline bool wait_on_timeout(struct nbd_device *nbd,
+ struct nbd_config *config)
+{
+ if (test_bit(NBD_RT_WAIT_ON_TIMEOUT, &config->runtime_flags) ||
+ nbd->tag_set.timeout == 0)
+ return true;
+ else
+ return false;
+}
+
This obviously needs to be updated to match my comments from the previous patch.
static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
bool reserved)
{
@@ -400,7 +411,7 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
nbd->tag_set.timeout)
goto error_out;
- if (nbd->tag_set.timeout) {
+ if (!wait_on_timeout(nbd, config)) {
dev_err_ratelimited(nbd_to_dev(nbd),
"Connection timed out, retrying (%d/%d alive)\n",
atomic_read(&config->live_connections),
@@ -1953,6 +1964,10 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
set_bit(NBD_RT_DISCONNECT_ON_CLOSE,
&config->runtime_flags);
}
+ if (flags & NBD_CFLAG_WAIT_ON_TIMEOUT) {
+ set_bit(NBD_RT_WAIT_ON_TIMEOUT,
+ &config->runtime_flags);
+ }
Documentation/process/coding-style.rst
"Do not unnecessarily use braces where a single statement will do."
same goes for below. Thanks,
Josef
Reply to: