Linux block layer
 help / color / mirror / Atom feed
* [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