Re: [PATCH v3 3/6] nbd: remove queue freeze in nbd_add_socket
- 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 3/6] nbd: remove queue freeze in nbd_add_socket
- From: "yu kuai" <yukuai@fygo.io>
- Date: Wed, 22 Jul 2026 14:35:28 +0800
- Message-id: <[🔎] 5799ffec-f179-47ec-b685-1f992648aff5@fygo.io>
- Reply-to: yukuai@fygo.io
- In-reply-to: <[🔎] 1b8b00e4-3f6f-42f1-a52a-b4d99c9f2cc3@fygo.io>
- References: <[🔎] 20260713065644.1637594-1-yangerkun@huawei.com> <[🔎] 20260713065644.1637594-4-yangerkun@huawei.com> <[🔎] 1b8b00e4-3f6f-42f1-a52a-b4d99c9f2cc3@fygo.io>
Hi,
在 2026/7/22 11:17, yu kuai 写道:
> Hi,
>
> 在 2026/7/13 14:56, Yang Erkun 写道:
>> nbd_add_socket can never run concurrently with inflight I/O:
>>
>> - netlink path: nbd_genl_connect calls nbd_add_socket before
>> nbd_start_device, so no I/O can happened when invoking nbd_add_socket,
>> nbd_genl_reconfigure cannot too since it won't call nbd_add_socket
>>
>> - ioctl path: NBD_SET_SOCK cannot be called after NBD_DO_IT with
>> the previous commit, so capability of nbd will keep 0 while
>> invoking NBD_SET_SOCK
>>
>> Removing the freeze in nbd_add_socket to speed up nbd device startup.
> Check nbd_open(), the pid is not checked there, which means the nbd device
> can be opened after add_disk() succeed. Normally read/write IO will not pass
> bio_check_eod() checking, however, there can be special zero sized bio that
> might still be issued to nbd device. So I think there should be a pid checking
> in nbd_open() for this patch, and I'm not aware of any special IO mush be handled
> before nbd_start_device(). Otherwise, the freeze is still necessary since
> nbd_handle_cmd() will deference config->socks[] directly.
Sorry this is a mistake, nbd_open() must succeed before pid is set, because of ioctl
like NBD_SET_SOCK and NED_DO_IT :( I just forgot this simple case.
>
>> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
>> ---
>> drivers/block/nbd.c | 9 ---------
>> 1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
>> index a15553ab4b97..0755b7046ed4 100644
>> --- a/drivers/block/nbd.c
>> +++ b/drivers/block/nbd.c
>> @@ -1272,7 +1272,6 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
>> struct socket *sock;
>> struct nbd_sock **socks;
>> struct nbd_sock *nsock;
>> - unsigned int memflags;
>> int err;
>>
>> /* Arg will be cast to int, check it to avoid overflow */
>> @@ -1283,12 +1282,6 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
>> return err;
>> nbd_reclassify_socket(sock);
>>
>> - /*
>> - * We need to make sure we don't get any errant requests while we're
>> - * reallocating the ->socks array.
>> - */
>> - memflags = blk_mq_freeze_queue(nbd->disk->queue);
>> -
>> if (!netlink && !nbd->task_setup &&
>> !test_bit(NBD_RT_BOUND, &config->runtime_flags))
>> nbd->task_setup = current;
>> @@ -1335,12 +1328,10 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
>> INIT_WORK(&nsock->work, nbd_pending_cmd_work);
>> socks[config->num_connections++] = nsock;
>> atomic_inc(&config->live_connections);
>> - blk_mq_unfreeze_queue(nbd->disk->queue, memflags);
>>
>> return 0;
>>
>> put_socket:
>> - blk_mq_unfreeze_queue(nbd->disk->queue, memflags);
>> sockfd_put(sock);
>> return err;
>> }
--
Thanks,
Kuai
Reply to: