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