From: Prasanna Kumar Kalever <[email protected]>
Hi,
This series has changes to reset the queue/io_timeout for nbd devices and
a cleanup patch.
Thank you!
Prasanna Kumar Kalever (2):
block: cleanup: define default command timeout and use it
nbd: reset the queue/io_timeout to default on disconnect
block/blk-mq.c | 2 +-
drivers/block/nbd.c | 9 +++++++--
include/linux/blkdev.h | 2 ++
3 files changed, 10 insertions(+), 3 deletions(-)
--
2.31.1
From: Prasanna Kumar Kalever <[email protected]>
defined BLK_DEFAULT_CMD_TIMEOUT and reuse it everywhere else.
Signed-off-by: Prasanna Kumar Kalever <[email protected]>
---
block/blk-mq.c | 2 +-
drivers/block/nbd.c | 2 +-
include/linux/blkdev.h | 2 ++
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c4ac51e54eb..1cba91eca6ee 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3289,7 +3289,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
goto err_hctxs;
INIT_WORK(&q->timeout_work, blk_mq_timeout_work);
- blk_queue_rq_timeout(q, set->timeout ? set->timeout : 30 * HZ);
+ blk_queue_rq_timeout(q, set->timeout ? set->timeout : BLK_DEFAULT_CMD_TIMEOUT);
q->tag_set = set;
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c38317979f74..16a1a14b1fd1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1376,7 +1376,7 @@ static void nbd_set_cmd_timeout(struct nbd_device *nbd, u64 timeout)
if (timeout)
blk_queue_rq_timeout(nbd->disk->queue, timeout * HZ);
else
- blk_queue_rq_timeout(nbd->disk->queue, 30 * HZ);
+ blk_queue_rq_timeout(nbd->disk->queue, BLK_DEFAULT_CMD_TIMEOUT);
}
/* Must be called with config_lock held */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d3afea47ade6..e50a9a5356d3 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -378,6 +378,8 @@ static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
#endif /* CONFIG_BLK_DEV_ZONED */
+#define BLK_DEFAULT_CMD_TIMEOUT (30*HZ) /* 30 seconds */
+
struct request_queue {
struct request *last_merge;
struct elevator_queue *elevator;
--
2.31.1
From: Prasanna Kumar Kalever <[email protected]>
Without any changes to NBD_ATTR_TIMEOUT (default is 30 secs),
$ rbd-nbd map rbd-pool/image0 --try-netlink
/dev/nbd0
$ cat /sys/block/nbd0/queue/io_timeout
30000
$ rbd-nbd unmap /dev/nbd0
$ cat /sys/block/nbd0/queue/io_timeout
30000
Now user sets NBD_ATTR_TIMEOUT to 60,
$ rbd-nbd map rbd-pool/image0 --try-netlink --io-timeout 60
/dev/nbd0
$ cat /sys/block/nbd0/queue/io_timeout
60000
$ rbd-nbd unmap /dev/nbd0
$ cat /sys/block/nbd0/queue/io_timeout
60000
Now user doesn't alter NBD_ATTR_TIMEOUT, but sysfs still shows it as 60,
$ rbd-nbd map rbd-pool/image0 --try-netlink
/dev/nbd0
$ cat /sys/block/nbd0/queue/io_timeout
60000
$ rbd-nbd unmap /dev/nbd0
$ cat /sys/block/nbd0/queue/io_timeout
60000
The problem exists with ioctl interface too.
Signed-off-by: Prasanna Kumar Kalever <[email protected]>
---
drivers/block/nbd.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 16a1a14b1fd1..a45aabc4914b 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -158,6 +158,7 @@ static void nbd_connect_reply(struct genl_info *info, int index);
static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info);
static void nbd_dead_link_work(struct work_struct *work);
static void nbd_disconnect_and_put(struct nbd_device *nbd);
+static void nbd_set_cmd_timeout(struct nbd_device *nbd, u64 timeout);
static inline struct device *nbd_to_dev(struct nbd_device *nbd)
{
@@ -1250,7 +1251,7 @@ static void nbd_config_put(struct nbd_device *nbd)
destroy_workqueue(nbd->recv_workq);
nbd->recv_workq = NULL;
- nbd->tag_set.timeout = 0;
+ nbd_set_cmd_timeout(nbd, 0);
nbd->disk->queue->limits.discard_granularity = 0;
nbd->disk->queue->limits.discard_alignment = 0;
blk_queue_max_discard_sectors(nbd->disk->queue, UINT_MAX);
@@ -2124,6 +2125,10 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info)
if (ret)
goto out;
+ /*
+ * On reconfigure, if NBD_ATTR_TIMEOUT is not provided, we will
+ * continue to use the cmd timeout provided with connect initially.
+ */
if (info->attrs[NBD_ATTR_TIMEOUT])
nbd_set_cmd_timeout(nbd,
nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]));
--
2.31.1
Hello,
Just wanted to bring this up again and remind you about the pending review.
Many Thanks,
--
Prasanna
On Fri, Aug 6, 2021 at 7:59 PM <[email protected]> wrote:
>
> From: Prasanna Kumar Kalever <[email protected]>
>
> Hi,
>
> This series has changes to reset the queue/io_timeout for nbd devices and
> a cleanup patch.
>
> Thank you!
>
> Prasanna Kumar Kalever (2):
> block: cleanup: define default command timeout and use it
> nbd: reset the queue/io_timeout to default on disconnect
>
> block/blk-mq.c | 2 +-
> drivers/block/nbd.c | 9 +++++++--
> include/linux/blkdev.h | 2 ++
> 3 files changed, 10 insertions(+), 3 deletions(-)
>
> --
> 2.31.1
>
On Fri, Aug 06, 2021 at 07:59:13PM +0530, [email protected] wrote:
> From: Prasanna Kumar Kalever <[email protected]>
>
> defined BLK_DEFAULT_CMD_TIMEOUT and reuse it everywhere else.
>
> Signed-off-by: Prasanna Kumar Kalever <[email protected]>
> ---
Reviewed-by: Ming Lei <[email protected]>
--
Ming
On Fri, Aug 06, 2021 at 07:59:14PM +0530, [email protected] wrote:
> From: Prasanna Kumar Kalever <[email protected]>
>
> Without any changes to NBD_ATTR_TIMEOUT (default is 30 secs),
> $ rbd-nbd map rbd-pool/image0 --try-netlink
> /dev/nbd0
> $ cat /sys/block/nbd0/queue/io_timeout
> 30000
> $ rbd-nbd unmap /dev/nbd0
> $ cat /sys/block/nbd0/queue/io_timeout
> 30000
>
> Now user sets NBD_ATTR_TIMEOUT to 60,
> $ rbd-nbd map rbd-pool/image0 --try-netlink --io-timeout 60
> /dev/nbd0
> $ cat /sys/block/nbd0/queue/io_timeout
> 60000
> $ rbd-nbd unmap /dev/nbd0
> $ cat /sys/block/nbd0/queue/io_timeout
> 60000
>
> Now user doesn't alter NBD_ATTR_TIMEOUT, but sysfs still shows it as 60,
> $ rbd-nbd map rbd-pool/image0 --try-netlink
> /dev/nbd0
> $ cat /sys/block/nbd0/queue/io_timeout
> 60000
> $ rbd-nbd unmap /dev/nbd0
> $ cat /sys/block/nbd0/queue/io_timeout
> 60000
>
> The problem exists with ioctl interface too.
>
> Signed-off-by: Prasanna Kumar Kalever <[email protected]>
> ---
> drivers/block/nbd.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 16a1a14b1fd1..a45aabc4914b 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -158,6 +158,7 @@ static void nbd_connect_reply(struct genl_info *info, int index);
> static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info);
> static void nbd_dead_link_work(struct work_struct *work);
> static void nbd_disconnect_and_put(struct nbd_device *nbd);
> +static void nbd_set_cmd_timeout(struct nbd_device *nbd, u64 timeout);
>
> static inline struct device *nbd_to_dev(struct nbd_device *nbd)
> {
> @@ -1250,7 +1251,7 @@ static void nbd_config_put(struct nbd_device *nbd)
> destroy_workqueue(nbd->recv_workq);
> nbd->recv_workq = NULL;
>
> - nbd->tag_set.timeout = 0;
> + nbd_set_cmd_timeout(nbd, 0);
> nbd->disk->queue->limits.discard_granularity = 0;
> nbd->disk->queue->limits.discard_alignment = 0;
> blk_queue_max_discard_sectors(nbd->disk->queue, UINT_MAX);
> @@ -2124,6 +2125,10 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info)
> if (ret)
> goto out;
>
> + /*
> + * On reconfigure, if NBD_ATTR_TIMEOUT is not provided, we will
> + * continue to use the cmd timeout provided with connect initially.
> + */
> if (info->attrs[NBD_ATTR_TIMEOUT])
> nbd_set_cmd_timeout(nbd,
> nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]));
> --
> 2.31.1
>
Looks fine:
Reviewed-by: Ming Lei <[email protected]>
--
Ming
On Wed, Oct 27, 2021 at 7:48 AM Prasanna Kalever <[email protected]> wrote:
>
> On Thu, Sep 16, 2021 at 1:52 PM Ming Lei <[email protected]> wrote:
> >
> > On Fri, Aug 06, 2021 at 07:59:13PM +0530, [email protected] wrote:
> > > From: Prasanna Kumar Kalever <[email protected]>
> > >
> > > defined BLK_DEFAULT_CMD_TIMEOUT and reuse it everywhere else.
> > >
> > > Signed-off-by: Prasanna Kumar Kalever <[email protected]>
> > > ---
> >
> > Reviewed-by: Ming Lei <[email protected]>
>
> Thanks for the review Ming.
>
> Attempting to bring this to the top again for more reviews/acks.
oops! please ignore, this is the wrong thread.
>
>
> BRs,
> --
> Prasanna
>
> >
> > --
> > Ming
> >
On Thu, Sep 16, 2021 at 1:53 PM Ming Lei <[email protected]> wrote:
>
> On Fri, Aug 06, 2021 at 07:59:14PM +0530, [email protected] wrote:
> > From: Prasanna Kumar Kalever <[email protected]>
> >
> > Without any changes to NBD_ATTR_TIMEOUT (default is 30 secs),
> > $ rbd-nbd map rbd-pool/image0 --try-netlink
> > /dev/nbd0
> > $ cat /sys/block/nbd0/queue/io_timeout
> > 30000
> > $ rbd-nbd unmap /dev/nbd0
> > $ cat /sys/block/nbd0/queue/io_timeout
> > 30000
> >
> > Now user sets NBD_ATTR_TIMEOUT to 60,
> > $ rbd-nbd map rbd-pool/image0 --try-netlink --io-timeout 60
> > /dev/nbd0
> > $ cat /sys/block/nbd0/queue/io_timeout
> > 60000
> > $ rbd-nbd unmap /dev/nbd0
> > $ cat /sys/block/nbd0/queue/io_timeout
> > 60000
> >
> > Now user doesn't alter NBD_ATTR_TIMEOUT, but sysfs still shows it as 60,
> > $ rbd-nbd map rbd-pool/image0 --try-netlink
> > /dev/nbd0
> > $ cat /sys/block/nbd0/queue/io_timeout
> > 60000
> > $ rbd-nbd unmap /dev/nbd0
> > $ cat /sys/block/nbd0/queue/io_timeout
> > 60000
> >
> > The problem exists with ioctl interface too.
> >
> > Signed-off-by: Prasanna Kumar Kalever <[email protected]>
> > ---
> > drivers/block/nbd.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > index 16a1a14b1fd1..a45aabc4914b 100644
> > --- a/drivers/block/nbd.c
> > +++ b/drivers/block/nbd.c
> > @@ -158,6 +158,7 @@ static void nbd_connect_reply(struct genl_info *info, int index);
> > static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info);
> > static void nbd_dead_link_work(struct work_struct *work);
> > static void nbd_disconnect_and_put(struct nbd_device *nbd);
> > +static void nbd_set_cmd_timeout(struct nbd_device *nbd, u64 timeout);
> >
> > static inline struct device *nbd_to_dev(struct nbd_device *nbd)
> > {
> > @@ -1250,7 +1251,7 @@ static void nbd_config_put(struct nbd_device *nbd)
> > destroy_workqueue(nbd->recv_workq);
> > nbd->recv_workq = NULL;
> >
> > - nbd->tag_set.timeout = 0;
> > + nbd_set_cmd_timeout(nbd, 0);
> > nbd->disk->queue->limits.discard_granularity = 0;
> > nbd->disk->queue->limits.discard_alignment = 0;
> > blk_queue_max_discard_sectors(nbd->disk->queue, UINT_MAX);
> > @@ -2124,6 +2125,10 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info)
> > if (ret)
> > goto out;
> >
> > + /*
> > + * On reconfigure, if NBD_ATTR_TIMEOUT is not provided, we will
> > + * continue to use the cmd timeout provided with connect initially.
> > + */
> > if (info->attrs[NBD_ATTR_TIMEOUT])
> > nbd_set_cmd_timeout(nbd,
> > nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]));
> > --
> > 2.31.1
> >
>
> Looks fine:
>
> Reviewed-by: Ming Lei <[email protected]>
Thanks for the review Ming.
Attempting to bring this to the top again for more reviews/acks.
Thanks!
--
Prasanna
>
> --
> Ming
>
On Thu, Sep 16, 2021 at 1:52 PM Ming Lei <[email protected]> wrote:
>
> On Fri, Aug 06, 2021 at 07:59:13PM +0530, [email protected] wrote:
> > From: Prasanna Kumar Kalever <[email protected]>
> >
> > defined BLK_DEFAULT_CMD_TIMEOUT and reuse it everywhere else.
> >
> > Signed-off-by: Prasanna Kumar Kalever <[email protected]>
> > ---
>
> Reviewed-by: Ming Lei <[email protected]>
Thanks for the review Ming.
Attempting to bring this to the top again for more reviews/acks.
BRs,
--
Prasanna
>
> --
> Ming
>
On 8/6/21 10:29 PM, [email protected] wrote:
> From: Prasanna Kumar Kalever <[email protected]>
>
> Hi,
>
> This series has changes to reset the queue/io_timeout for nbd devices and
> a cleanup patch.
>
> Thank you!
>
> Prasanna Kumar Kalever (2):
> block: cleanup: define default command timeout and use it
> nbd: reset the queue/io_timeout to default on disconnect
>
> block/blk-mq.c | 2 +-
> drivers/block/nbd.c | 9 +++++++--
> include/linux/blkdev.h | 2 ++
> 3 files changed, 10 insertions(+), 3 deletions(-)
>
This series LGTM.
Reviewed-by: Xiubo Li <[email protected]>