Re: [PATCH v3 5/6] nbd: skip queue freeze when setting size at device startup

yangerkun <[email protected]>
Newsgroups org.kernel.vger.linux-block
Message-ID <[email protected]>

在 2026/7/22 11:49, yu kuai 写道:
> Hi,
> 
> 在 2026/7/13 14:56, Yang Erkun 写道:
>> Commit 242a49e5c878 ("nbd: freeze the queue for queue limits updates")
>> introduce queue freeze/unfreeze in nbd_set_size to avoid inflight
>> commands see this inconsistent limits. However, this cannot be happened
>> when device setup since the capacity is still 0.
> 
> Same reason above, 0 capacity disk will only reject non-zero sized bio from
> bio_check_eod().

Mabye we have reached an agreement on this issue? Flush bio will be 
reject since not enable write cache.

> 
>>
>> time nbd-client --name myexport --connections 96 127.0.0.1 1234
>>
>> Before this patchset:
>> real    0m2.195s
>> user    0m0.005s
>> sys     0m0.022s
>>
>> After this patchset:
>> real    0m0.090s
>> user    0m0.004s
>> sys     0m0.018s
>>
>> Signed-off-by: Yang Erkun <[email protected]>
>> ---
>>    drivers/block/nbd.c | 21 ++++++++++++++-------
>>    1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
>> index 400f638e832e..2c7b09c70da2 100644
>> --- a/drivers/block/nbd.c
>> +++ b/drivers/block/nbd.c
>> @@ -331,7 +331,8 @@ static void nbd_mark_nsock_dead(struct nbd_device *nbd, struct nbd_sock *nsock,
>>    	nsock->sent = 0;
>>    }
>>    
>> -static int nbd_set_size(struct nbd_device *nbd, loff_t bytesize, loff_t blksize)
>> +static int nbd_set_size(struct nbd_device *nbd, loff_t bytesize, loff_t blksize,
>> +			bool freeze)
>>    {
>>    	struct queue_limits lim;
>>    	int error;
>> @@ -371,7 +372,13 @@ static int nbd_set_size(struct nbd_device *nbd, loff_t bytesize, loff_t blksize)
>>    
>>    	lim.logical_block_size = blksize;
>>    	lim.physical_block_size = blksize;
>> -	error = queue_limits_commit_update_frozen(nbd->disk->queue, &lim);
>> +
>> +	if (freeze)
>> +		error = queue_limits_commit_update_frozen(nbd->disk->queue,
>> +				&lim);
>> +	else
>> +		error = queue_limits_commit_update(nbd->disk->queue, &lim);
>> +
>>    	if (error)
>>    		return error;
>>    
>> @@ -1563,7 +1570,7 @@ static int nbd_start_device(struct nbd_device *nbd)
>>    		args->index = i;
>>    		queue_work(nbd->recv_workq, &args->work);
>>    	}
>> -	return nbd_set_size(nbd, config->bytesize, nbd_blksize(config));
>> +	return nbd_set_size(nbd, config->bytesize, nbd_blksize(config), false);
>>    }
>>    
>>    static int nbd_start_device_ioctl(struct nbd_device *nbd)
>> @@ -1631,13 +1638,13 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>>    	case NBD_SET_SOCK:
>>    		return nbd_add_socket(nbd, arg, false);
>>    	case NBD_SET_BLKSIZE:
>> -		return nbd_set_size(nbd, config->bytesize, arg);
>> +		return nbd_set_size(nbd, config->bytesize, arg, true);
>>    	case NBD_SET_SIZE:
>> -		return nbd_set_size(nbd, arg, nbd_blksize(config));
>> +		return nbd_set_size(nbd, arg, nbd_blksize(config), true);
>>    	case NBD_SET_SIZE_BLOCKS:
>>    		if (check_shl_overflow(arg, config->blksize_bits, &bytesize))
>>    			return -EINVAL;
>> -		return nbd_set_size(nbd, bytesize, nbd_blksize(config));
>> +		return nbd_set_size(nbd, bytesize, nbd_blksize(config), true);
>>    	case NBD_SET_TIMEOUT:
>>    		nbd_set_cmd_timeout(nbd, arg);
>>    		return 0;
>> @@ -2122,7 +2129,7 @@ static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd)
>>    		bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]);
>>    
>>    	if (bytes != config->bytesize || bsize != nbd_blksize(config))
>> -		return nbd_set_size(nbd, bytes, bsize);
>> +		return nbd_set_size(nbd, bytes, bsize, true);
> 
> This is still called from nbd_genl_connect().

nbd->pid check exist in nbd_set_size, and nbd_genl_connect call 
nbd_genl_size_set before nbd_start_device, which has not set nbd->pid, 
so path from nbd_genl_connect won't work. But for path from 
nbd_genl_reconnect, we need this protect...

> 
> Will it be much simpler to just check pid in nbd_set_size()?
> 
>>    	return 0;
>>    }
>>    
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.