Re: [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path?

[yu kuai]

Hi,

在 2026/8/14 15:14, Zhichao Huang 写道:
> To: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>,
>      Jens Axboe <axboe@xxxxxxxxx>,
>      Ming Lei <ming.lei@xxxxxxxxxx>,
>      linux-block@xxxxxxxxxxxxxxx,
>      linux-kernel@xxxxxxxxxxxxxxx
>
> Hi all,
>
> This is a question / RFC, not a patch -- I'd like your opinion before
> proposing anything, because the code in question is a deliberate,
> stable-tagged UAF fix and I don't want to weaken it.
>
> Background / motivation
> -----------------------
> We run large numbers of Firecracker microVMs whose block devices are
> backed by NBD. On the VM-create hot path each microVM does an initial
> NBD connect (netlink NBD_CMD_CONNECT), and we launch many of them
> concurrently on the same many-core host. Profiling the create path
> shows a large chunk of kernel time is spent in blk_mq_freeze_queue()
> during the connect: each freeze waits for a full RCU grace period, which
> on these many-core hosts we measure at roughly 35-50ms. Because several
> connects run concurrently, these grace-period waits stack up and become
> a visible tail-latency contributor on VM start.

There is already a reviewed set to kill queue freeze for nbd:

[PATCH v8 0/8] nbd: eliminate queue freeze/unfreeze overhead in 
connection setup - Yang Erkun 
<https://lore.kernel.org/all/20260805122930.57647-1-yangerkun@xxxxxxxxxx/>

>
> One of the freezes on that path is in nbd_add_socket(), added by
>
>    b98e762e3d71 ("nbd: freeze the queue while we're adding connections")
>
> with the comment "We need to make sure we don't get any errant requests
> while we're reallocating the ->socks array." I understand this freeze
> fences the krealloc() of config->socks[] against a concurrent
> nbd_queue_rq() -> nbd_handle_cmd(), which dereferences
> config->socks[index] and config->num_connections -- i.e. it prevents a
> use-after-free / out-of-bounds read, and it was Cc: stable. I am *not*
> questioning that this is needed while the device is live.
>
> Observation about the *initial* connect
> ----------------------------------------
> On the initial connect, before the device is started (nbd->pid == 0),
> nbd_add_socket() runs while:
>
>    - capacity is still 0. set_capacity_and_notify() is only reached in
>      nbd_set_size() after the "if (!nbd->pid) return 0;" early return,
>      and on both the netlink and ioctl paths nbd_add_socket() runs
>      strictly before nbd_start_device() sets nbd->pid; and
>
>    - the queue advertises no write cache / discard / write-zeroes yet
>      (those queue_limits features are also only set in the nbd->pid
>      branch of nbd_set_size()).
>
> Walking submit_bio_noacct() with capacity == 0 and no features, every
> I/O op seems to be rejected before it can become a request that reaches
> nbd_handle_cmd():
>
>    - READ/WRITE with sectors: bio_check_eod() -> -EIO (maxsector == 0);
>    - flush (REQ_PREFLUSH): !bdev_write_cache() path strips the flush and
>      completes a zero-sector bio with BLK_STS_OK without dispatch;
>    - DISCARD / WRITE_ZEROES / SECURE_ERASE / ZONE_*: not_supported since
>      the corresponding limits are 0 / not set;
>    - passthrough (DRV_IN/OUT): not_supported on the bio submit path, and
>      nbd issues none itself.
>
> The only theoretical gap I can see is a zero-length, non-flush data bio
> (bio_check_eod() skips the check when nr_sectors == 0), but the VFS /
> direct-IO layers don't actually submit zero-length data bios.
>
> The question
> ------------
> Given the above, is the nbd_add_socket() freeze effectively redundant on
> the *initial*, pre-start (!nbd->pid) connect, where no I/O can reach the
> driver? If so, would any of the following be acceptable, or is the
> freeze intentionally kept unconditional for robustness?
>
>    1) Skip the freeze only when (!nbd->pid && get_capacity(disk) == 0),
>       falling back to the stock freeze otherwise (fail-shut);
>    2) Some cheaper barrier than a full-queue-freeze RCU grace period for
>       this "no I/O possible yet" case;
>    3) Leave it as-is -- the "no I/O reaches the driver" property relies on
>       a non-local invariant spanning bio_check_eod(), the flush filter,
>       the pre-start queue_limits, and the set_capacity() ordering, and you
>       consider that too fragile to build on.
>
> My instinct is that (3) is a legitimate answer and that the invariant is
> fragile, which is exactly why I'm asking rather than sending a patch. If
> there's a direction you'd be willing to take, I'm happy to do the work
> and the testing.
>
> Thanks,
> Zhichao Huang <huangzhichao.1@xxxxxxxxxxxxx>
>
-- 
Thanks,
Kuai