[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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



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: