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

Hi,

在 2026/7/22 11:38, yu kuai 写道:
> 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?

OK, I will try it.

> 
>> +
>>    /* 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.

Yes, this patch indeed only addresses the freeze/unfreeze issue when 
creating a nbd device for the first time through netlink. The later 
patch addresses the nbds_max case. I will update the commit message 
later to make it more accurate.

> 
>>    
>