* [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path?
@ 2026-08-14 7:14 Zhichao Huang
2026-08-14 7:54 ` yangerkun
2026-08-14 8:03 ` yu kuai
0 siblings, 2 replies; 4+ messages in thread
From: Zhichao Huang @ 2026-08-14 7:14 UTC (permalink / raw)
To: josef; +Cc: hch, axboe, ming.lei, linux-block, linux-kernel, huangzhichao.1
To: Josef Bacik <josef@toxicpanda.com>
Cc: Christoph Hellwig <hch@lst.de>,
Jens Axboe <axboe@kernel.dk>,
Ming Lei <ming.lei@redhat.com>,
linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org
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.
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@bytedance.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path?
2026-08-14 7:14 [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path? Zhichao Huang
@ 2026-08-14 7:54 ` yangerkun
2026-08-14 10:33 ` Zhichao Huang
2026-08-14 8:03 ` yu kuai
1 sibling, 1 reply; 4+ messages in thread
From: yangerkun @ 2026-08-14 7:54 UTC (permalink / raw)
To: Zhichao Huang, josef; +Cc: hch, axboe, ming.lei, linux-block, linux-kernel
Hi Zhichao,
Thanks for your attention to this issue. I have try fix this with this
patchset. Please check does it solve your problem.
https://lore.kernel.org/all/20260805122930.57647-1-yangerkun@huawei.com/
Thanks,
Erkun.
在 2026/8/14 15:14, Zhichao Huang 写道:
> To: Josef Bacik <josef@toxicpanda.com>
> Cc: Christoph Hellwig <hch@lst.de>,
> Jens Axboe <axboe@kernel.dk>,
> Ming Lei <ming.lei@redhat.com>,
> linux-block@vger.kernel.org,
> linux-kernel@vger.kernel.org
>
> 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.
>
> 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@bytedance.com>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path?
2026-08-14 7:14 [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path? Zhichao Huang
2026-08-14 7:54 ` yangerkun
@ 2026-08-14 8:03 ` yu kuai
1 sibling, 0 replies; 4+ messages in thread
From: yu kuai @ 2026-08-14 8:03 UTC (permalink / raw)
To: Zhichao Huang, josef, yu kuai
Cc: hch, axboe, ming.lei, linux-block, linux-kernel
Hi,
在 2026/8/14 15:14, Zhichao Huang 写道:
> To: Josef Bacik <josef@toxicpanda.com>
> Cc: Christoph Hellwig <hch@lst.de>,
> Jens Axboe <axboe@kernel.dk>,
> Ming Lei <ming.lei@redhat.com>,
> linux-block@vger.kernel.org,
> linux-kernel@vger.kernel.org
>
> 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@huawei.com/>
>
> 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@bytedance.com>
>
--
Thanks,
Kuai
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path?
2026-08-14 7:54 ` yangerkun
@ 2026-08-14 10:33 ` Zhichao Huang
0 siblings, 0 replies; 4+ messages in thread
From: Zhichao Huang @ 2026-08-14 10:33 UTC (permalink / raw)
To: yangerkun, yukuai; +Cc: josef, hch, axboe, ming.lei, linux-block, linux-kernel
Hi Erkun, Kuai,
Thank you both -- yes, [PATCH v8 0/8] is exactly what we need. It covers
the nbd_add_socket() and nbd_set_size() freezes I was asking about, and
also the blk_mq_update_nr_hw_queues() one in nbd_start_device() that I
hadn't gotten to. Good to see it's already reviewed.
Erkun, I especially like that patch 2 removes the freeze's root cause by
rejecting NBD_SET_SOCK on an active device, rather than relying on the
capacity==0 / bio_check_eod invariant I was reasoning about in my mail --
that's a cleaner and more robust basis for dropping the freeze.
I'll apply the series on our Firecracker + many-core setup and follow up
with our create-latency numbers and a Tested-by. Happy to help however
is useful to get it landed.
Thanks again,
Zhichao Huang <huangzhichao.1@bytedance.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-14 10:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 7:14 [RFC] nbd: is the add_socket queue freeze needed on the initial (pre-start) connect path? Zhichao Huang
2026-08-14 7:54 ` yangerkun
2026-08-14 10:33 ` Zhichao Huang
2026-08-14 8:03 ` yu kuai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox