Re: [PATCH 3/3] nbd: introduce a client flag to do zero timeout handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2020/9/22 2:03 AM, Josef Bacik wrote:
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@xxxxxxxxxxxxx>

I like this idea in general, just a few comments below.

Thanks,
Hou


---
  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.

Thanks. Please see v2 latter.


  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

Thanks. Will fix this in v2.


Thanks,
Hou




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux