Re: [PATCH v3 4/6] nbd: set nr_hw_queues at device creation to skip queue freeze
- To: "Yang Erkun" <yangerkun@huawei.com>, <josef@toxicpanda.com>, <axboe@kernel.dk>, <hch@lst.de>, "yu kuai" <yukuai@fygo.io>
- Cc: <yi.zhang@huawei.com>, <chengzhihao1@huawei.com>, <echo.chenlin@huawei.com>, <leo.lilong@huaweicloud.com>, <wangkefeng.wang@huawei.com>, <linux-block@vger.kernel.org>, <nbd@other.debian.org>
- Subject: Re: [PATCH v3 4/6] nbd: set nr_hw_queues at device creation to skip queue freeze
- From: "yu kuai" <yukuai@fygo.io>
- Date: Wed, 22 Jul 2026 11:38:39 +0800
- Message-id: <[🔎] a36f609c-c861-42ad-afd5-b44e7cdcfb80@fygo.io>
- Reply-to: yukuai@fygo.io
- In-reply-to: <[🔎] 20260713065644.1637594-5-yangerkun@huawei.com>
- References: <[🔎] 20260713065644.1637594-1-yangerkun@huawei.com> <[🔎] 20260713065644.1637594-5-yangerkun@huawei.com>
Hi,
在 2026/7/13 14:56, Yang Erkun 写道:
> There still be queue freeze call when nbd_start_device invoking
> blk_mq_update_nr_hw_queues. For netlink path, we can obtain the actual
> number of connections before calling nbd_dev_add in nbd_genl_connect,
> which can helps remove this queue freeze.
>
> However, nbd devices created with a fixed nbds_max may still require
> this freezing because the real connection count is unknown.
>
> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
> ---
> drivers/block/nbd.c | 39 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 0755b7046ed4..400f638e832e 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -1930,7 +1930,8 @@ static const struct blk_mq_ops nbd_mq_ops = {
> .timeout = nbd_xmit_timeout,
> };
>
> -static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
> +static struct nbd_device *nbd_dev_add(int index, unsigned int refs,
> + int nr_hw_queues)
> {
> struct queue_limits lim = {
> .max_hw_sectors = 65536,
> @@ -1947,7 +1948,7 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
> goto out;
>
> nbd->tag_set.ops = &nbd_mq_ops;
> - nbd->tag_set.nr_hw_queues = 1;
> + nbd->tag_set.nr_hw_queues = nr_hw_queues;
> nbd->tag_set.queue_depth = 128;
> nbd->tag_set.numa_node = NUMA_NO_NODE;
> nbd->tag_set.cmd_size = sizeof(struct nbd_cmd);
> @@ -2070,6 +2071,35 @@ static const struct nla_policy nbd_sock_policy[NBD_SOCK_MAX + 1] = {
> [NBD_SOCK_FD] = { .type = NLA_U32 },
> };
>
> +/*
> + * Count the number of socket FDs in the NBD_ATTR_SOCKETS netlink attribute.
> + * This is used to determine the correct nr_hw_queues before creating the
> + * nbd device, so that blk_mq_update_nr_hw_queues (and its RCU grace period
> + * overhead) can be avoided entirely.
> + */
> +static int nbd_genl_count_sockets(struct genl_info *info)
> +{
> + struct nlattr *attr;
> + int rem, count = 0;
> +
> + if (!info->attrs[NBD_ATTR_SOCKETS])
> + return 0;
> +
> + nla_for_each_nested(attr, info->attrs[NBD_ATTR_SOCKETS], rem) {
> + struct nlattr *socks[NBD_SOCK_MAX + 1];
> +
> + if (nla_type(attr) != NBD_SOCK_ITEM)
> + continue;
nbd_genl_connect() will fail in this case.
> + if (nla_parse_nested_deprecated(socks, NBD_SOCK_MAX,
> + attr, nbd_sock_policy,
> + info->extack) != 0)
> + continue;
> + if (socks[NBD_SOCK_FD])
> + count++;
> + }
> + return count;
> +}
Personally I don't like to copy code from existed NBD_ATTR_SOCKETS handling. Do
you consider moving NBD_ATTR_SOCKETS handling forward or factor out a common helper?
> +
> /* We don't use this right now since we don't parse the incoming list, but we
> * still want it here so userspace knows what to expect.
> */
> @@ -2101,6 +2131,7 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
> struct nbd_device *nbd;
> struct nbd_config *config;
> int index = -1;
> + int num_connections = nbd_genl_count_sockets(info);
> int ret;
> bool put_dev = false;
>
> @@ -2148,7 +2179,7 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
> mutex_unlock(&nbd_index_mutex);
>
> if (!nbd) {
> - nbd = nbd_dev_add(index, 2);
> + nbd = nbd_dev_add(index, 2, num_connections);
> if (IS_ERR(nbd)) {
> pr_err("failed to add new device\n");
> return PTR_ERR(nbd);
> @@ -2715,7 +2746,7 @@ static int __init nbd_init(void)
> nbd_dbg_init();
>
> for (i = 0; i < nbds_max; i++)
> - nbd_dev_add(i, 1);
> + nbd_dev_add(i, 1, 1);
> return 0;
> }
This approach will only be useful when nbd device is created the first time.
If NBD_ATTR_INDEX is set and nbd device is found, or NBD_ATTR_IDNEX is not set
and nbd_find_get_unused() found an unused nbd device, blk_mq_update_nr_hw_queues()
is still required. As you can see, nbds_max is default to 16 and all these devices
will be created with nr_hw_queues as 1.
>
--
Thanks,
Kuai
Reply to: