Re: [PATCH v3 3/6] nbd: remove queue freeze in nbd_add_socket
yangerkun <[email protected]>
| Newsgroups | org.kernel.vger.linux-block |
|---|---|
| Message-ID | <[email protected]> |
在 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 <[email protected]> >>>> --- >>>> 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; >>>> } >> > >