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

[yangerkun <yangerkun@huawei.com>]

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

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.

> >