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

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



[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