Re: [PATCH v3 4/6] nbd: set nr_hw_queues at device creation to skip queue freeze

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 <[email protected]>
> ---
>   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