Re: [PATCH v3 3/6] nbd: remove queue freeze in nbd_add_socket

[yangerkun <yangerkun@huawei.com>]

在 2026/7/22 16:45, yangerkun 写道:
> 
> 
> 在 2026/7/22 14:35, yu kuai 写道:
>> 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.
> 
> nbd can only accept request from bio, so bio_check_eod will reject them 
> all?

Sorry, for read/write bio, bio_check_eod will reject them, for flush 
only bio, if (!bdev_write_cache(bdev)) in submit_bio_noacct will reject 
it. So it seems safe here.

> 
>>
>>>
>>>> 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;
>>>>     }
>>
> 
>