* [PATCH v2 0/2] nvme-tcp: parallelize I/O queue connect
@ 2026-08-24 22:56 Surabhi Gogte
2026-08-24 22:56 ` [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path Surabhi Gogte
2026-08-24 22:56 ` [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup Surabhi Gogte
0 siblings, 2 replies; 15+ messages in thread
From: Surabhi Gogte @ 2026-08-24 22:56 UTC (permalink / raw)
To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, linux-kernel, mkhalfella, randyj, adailey,
Surabhi Gogte
Similar to commit 2a8513091d2f ("nvme-rdma: parallelize I/O queue
allocation and startup"), I/O queue allocation and start steps
(including authentication) can be parallelized for nvme-tcp as well.
Patch 1 is a preparatory refactor: it splits the queue-count
negotiation and TLS PSK checks into helpers so that the individual
steps can be called directly from nvme_tcp_configure_io_queues(). No
functional change.
Patch 2 has the async implementation: it fans the per-queue alloc and
start out over an async domain, serializes the per-cpu queue-count in
nvme_tcp_set_queue_io_cpu() that can now run concurrently, and
switches sock_create_kern() to init_net.
Testing on a 64-core host with 64 IO-queues shows nvme-tcp connection
time reduced from 61ms to 11ms.
---
v2:
- Resending the entire series to fix missing numbering in the
original patch 2 subject line. No functional code changes.
---
Surabhi Gogte (2):
nvme-tcp: refactor I/O queue setup path
nvme-tcp: parallelize I/O queue allocation and startup
drivers/nvme/host/tcp.c | 139 +++++++++++++++++++++++++++-------------
1 file changed, 93 insertions(+), 46 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path 2026-08-24 22:56 [PATCH v2 0/2] nvme-tcp: parallelize I/O queue connect Surabhi Gogte @ 2026-08-24 22:56 ` Surabhi Gogte 2026-08-30 21:56 ` Sagi Grimberg 2026-08-24 22:56 ` [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup Surabhi Gogte 1 sibling, 1 reply; 15+ messages in thread From: Surabhi Gogte @ 2026-08-24 22:56 UTC (permalink / raw) To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg Cc: linux-nvme, linux-kernel, mkhalfella, randyj, adailey, Surabhi Gogte Split the I/O queue setup helpers apart so that the individual steps can be called directly from nvme_tcp_configure_io_queues(): - Queue count negotiation moves out of nvme_tcp_alloc_io_queues() into a new nvme_tcp_io_queue_count(), leaving the allocator with just the per-queue allocation loop. - TLS PSK validation moves out of __nvme_tcp_alloc_io_queues() into a new nvme_tcp_tls_check_psk(). - nvme_tcp_configure_io_queues() now calls the three steps directly in the same order as before. Signed-off-by: Surabhi Gogte <sgogte@purestorage.com> --- drivers/nvme/host/tcp.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 5fda9661bdb7..354668ad29ac 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2171,10 +2171,8 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl) return ret; } -static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) +static int nvme_tcp_tls_check_psk(struct nvme_ctrl *ctrl) { - int i, ret; - if (nvme_tcp_tls_configured(ctrl)) { if (ctrl->opts->concat) { /* @@ -2196,6 +2194,13 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) } } + return 0; +} + +static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) +{ + int i, ret; + for (i = 1; i < ctrl->queue_count; i++) { ret = nvme_tcp_alloc_queue(ctrl, i, ctrl->tls_pskid); @@ -2212,7 +2217,7 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) return ret; } -static int nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) +static int nvme_tcp_io_queue_count(struct nvme_ctrl *ctrl) { unsigned int nr_io_queues; int ret; @@ -2234,14 +2239,22 @@ static int nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) nvmf_set_io_queues(ctrl->opts, nr_io_queues, to_tcp_ctrl(ctrl)->io_queues); - return __nvme_tcp_alloc_io_queues(ctrl); + return 0; } static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new) { int ret, nr_queues; - ret = nvme_tcp_alloc_io_queues(ctrl); + ret = nvme_tcp_io_queue_count(ctrl); + if (ret) + return ret; + + ret = nvme_tcp_tls_check_psk(ctrl); + if (ret) + return ret; + + ret = __nvme_tcp_alloc_io_queues(ctrl); if (ret) return ret; -- 2.55.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path 2026-08-24 22:56 ` [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path Surabhi Gogte @ 2026-08-30 21:56 ` Sagi Grimberg 2026-09-02 21:25 ` Surabhi Gogte (she/her) 0 siblings, 1 reply; 15+ messages in thread From: Sagi Grimberg @ 2026-08-30 21:56 UTC (permalink / raw) To: Surabhi Gogte, Keith Busch, Jens Axboe, Christoph Hellwig Cc: linux-nvme, linux-kernel, mkhalfella, randyj, adailey On 25/08/2026 1:56, Surabhi Gogte wrote: > Split the I/O queue setup helpers apart so that the individual steps can > be called directly from nvme_tcp_configure_io_queues(): > > - Queue count negotiation moves out of nvme_tcp_alloc_io_queues() into a > new nvme_tcp_io_queue_count(), leaving the allocator with just the > per-queue allocation loop. > - TLS PSK validation moves out of __nvme_tcp_alloc_io_queues() into a > new nvme_tcp_tls_check_psk(). > - nvme_tcp_configure_io_queues() now calls the three steps directly in > the same order as before. > > Signed-off-by: Surabhi Gogte <sgogte@purestorage.com> > --- > drivers/nvme/host/tcp.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c > index 5fda9661bdb7..354668ad29ac 100644 > --- a/drivers/nvme/host/tcp.c > +++ b/drivers/nvme/host/tcp.c > @@ -2171,10 +2171,8 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl) > return ret; > } > > -static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) > +static int nvme_tcp_tls_check_psk(struct nvme_ctrl *ctrl) > { > - int i, ret; > - > if (nvme_tcp_tls_configured(ctrl)) { > if (ctrl->opts->concat) { > /* > @@ -2196,6 +2194,13 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) > } > } > > + return 0; > +} > + > +static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) > +{ > + int i, ret; > + > for (i = 1; i < ctrl->queue_count; i++) { > ret = nvme_tcp_alloc_queue(ctrl, i, > ctrl->tls_pskid); > @@ -2212,7 +2217,7 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) > return ret; > } > > -static int nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) > +static int nvme_tcp_io_queue_count(struct nvme_ctrl *ctrl) Strange function name... maybe nvme_tcp_set_io_queue_count? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path 2026-08-30 21:56 ` Sagi Grimberg @ 2026-09-02 21:25 ` Surabhi Gogte (she/her) 0 siblings, 0 replies; 15+ messages in thread From: Surabhi Gogte (she/her) @ 2026-09-02 21:25 UTC (permalink / raw) To: Sagi Grimberg Cc: Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, mkhalfella, randyj, adailey On Sun, Aug 30, 2026 at 2:56 PM Sagi Grimberg <sagi@grimberg.me> wrote: > > > > On 25/08/2026 1:56, Surabhi Gogte wrote: > > Split the I/O queue setup helpers apart so that the individual steps can > > be called directly from nvme_tcp_configure_io_queues(): > > > > - Queue count negotiation moves out of nvme_tcp_alloc_io_queues() into a > > new nvme_tcp_io_queue_count(), leaving the allocator with just the > > per-queue allocation loop. > > - TLS PSK validation moves out of __nvme_tcp_alloc_io_queues() into a > > new nvme_tcp_tls_check_psk(). > > - nvme_tcp_configure_io_queues() now calls the three steps directly in > > the same order as before. > > > > Signed-off-by: Surabhi Gogte <sgogte@purestorage.com> > > --- > > } > > > > -static int nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) > > +static int nvme_tcp_io_queue_count(struct nvme_ctrl *ctrl) > > Strange function name... maybe nvme_tcp_set_io_queue_count? Yeah, that works better. will update in v3. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-24 22:56 [PATCH v2 0/2] nvme-tcp: parallelize I/O queue connect Surabhi Gogte 2026-08-24 22:56 ` [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path Surabhi Gogte @ 2026-08-24 22:56 ` Surabhi Gogte 2026-08-24 23:58 ` Randy Jennings 2026-08-30 22:00 ` Sagi Grimberg 1 sibling, 2 replies; 15+ messages in thread From: Surabhi Gogte @ 2026-08-24 22:56 UTC (permalink / raw) To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg Cc: linux-nvme, linux-kernel, mkhalfella, randyj, adailey, Surabhi Gogte Similar to commit 2a8513091d2f ("nvme-rdma: parallelize I/O queue allocation and startup"), refactor nvme tcp I/O queue setup to use async API, combining allocation and startup into a single parallel operation per queue. This reduces connection and reconnection setup time when there are delays in establishing connections, which is especially important for high-core-count hosts. Key changes: - Use async API to facilitate parallel calls for io queue setup. - Add nvme_tcp_setup_ctx for propagating errors from async workers. - Remove nvme_tcp_start_io_queues() and __nvme_tcp_alloc_io_queues(); their logic is folded into nvme_tcp_setup_io_queues() and nvme_tcp_configure_io_queues(). - Allocate the io tag set before the queues so that the queue range is known, and only set up the reconnect grow case if the queue count actually increased. - Serialize the cpu scan and claim in nvme_tcp_set_queue_io_cpu() with a spinlock, as concurrent callers would otherwise select the same cpu. The per-cpu counters no longer need to be atomics. - Use init_net in nvme_tcp_alloc_queue() instead of the namespace of current, which is no longer the connecting task once the allocation runs from a worker. A controller is not guaranteed to be tied to a namespace, as the reconnect and error recovery paths already run from a workqueue in init_net. Testing on a 64-core host with 64 IO-queues shows nvme-tcp connection time reduced from 61ms to 11ms. Signed-off-by: Surabhi Gogte <sgogte@purestorage.com> --- drivers/nvme/host/tcp.c | 126 +++++++++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 46 deletions(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 354668ad29ac..30fe4c5abe0b 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -7,6 +7,7 @@ #include <linux/module.h> #include <linux/init.h> #include <linux/slab.h> +#include <linux/async.h> #include <linux/err.h> #include <linux/crc32.h> #include <linux/nvme-tcp.h> @@ -54,7 +55,8 @@ MODULE_PARM_DESC(tls_handshake_timeout, "nvme TLS handshake timeout in seconds (default 10)"); #endif -static atomic_t nvme_tcp_cpu_queues[NR_CPUS]; +static int nvme_tcp_cpu_queues[NR_CPUS]; +static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock); enum nvme_tcp_send_state { NVME_TCP_SEND_CMD_PDU = 0, @@ -154,6 +156,12 @@ struct nvme_tcp_queue { static DEFINE_MUTEX(nvme_tcp_ctrl_mutex); static LIST_HEAD_GUARDED(nvme_tcp_ctrl_list, nvme_tcp_ctrl_mutex); +struct nvme_tcp_setup_ctx { + struct nvme_ctrl *ctrl; + int qid; + int *err; +}; + struct nvme_tcp_ctrl { /* read only in the hot path */ struct nvme_tcp_queue *queues; @@ -1718,9 +1726,10 @@ static void nvme_tcp_set_queue_io_cpu(struct nvme_tcp_queue *queue) goto out; /* Search for the least used cpu from the mq_map */ + spin_lock(&nvme_tcp_cpu_queues_lock); io_cpu = WORK_CPU_UNBOUND; for_each_online_cpu(cpu) { - int num_queues = atomic_read(&nvme_tcp_cpu_queues[cpu]); + int num_queues = nvme_tcp_cpu_queues[cpu]; if (mq_map[cpu] != qid) continue; @@ -1731,9 +1740,10 @@ static void nvme_tcp_set_queue_io_cpu(struct nvme_tcp_queue *queue) } if (io_cpu != WORK_CPU_UNBOUND) { queue->io_cpu = io_cpu; - atomic_inc(&nvme_tcp_cpu_queues[io_cpu]); + nvme_tcp_cpu_queues[io_cpu]++; set_bit(NVME_TCP_Q_IO_CPU_SET, &queue->flags); } + spin_unlock(&nvme_tcp_cpu_queues_lock); out: dev_dbg(ctrl->ctrl.device, "queue %d: using cpu %d\n", qid, queue->io_cpu); @@ -1846,7 +1856,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, queue->cmnd_capsule_len = sizeof(struct nvme_command) + NVME_TCP_ADMIN_CCSZ; - ret = sock_create_kern(current->nsproxy->net_ns, + ret = sock_create_kern(&init_net, ctrl->addr.ss_family, SOCK_STREAM, IPPROTO_TCP, &queue->sock); if (ret) { @@ -2010,8 +2020,11 @@ static void nvme_tcp_stop_queue_nowait(struct nvme_ctrl *nctrl, int qid) if (!test_bit(NVME_TCP_Q_ALLOCATED, &queue->flags)) return; - if (test_and_clear_bit(NVME_TCP_Q_IO_CPU_SET, &queue->flags)) - atomic_dec(&nvme_tcp_cpu_queues[queue->io_cpu]); + if (test_and_clear_bit(NVME_TCP_Q_IO_CPU_SET, &queue->flags)) { + spin_lock(&nvme_tcp_cpu_queues_lock); + nvme_tcp_cpu_queues[queue->io_cpu]--; + spin_unlock(&nvme_tcp_cpu_queues_lock); + } mutex_lock(&queue->queue_lock); if (test_and_clear_bit(NVME_TCP_Q_LIVE, &queue->flags)) @@ -2118,25 +2131,6 @@ static void nvme_tcp_stop_io_queues(struct nvme_ctrl *ctrl) nvme_tcp_wait_queue(ctrl, i); } -static int nvme_tcp_start_io_queues(struct nvme_ctrl *ctrl, - int first, int last) -{ - int i, ret; - - for (i = first; i < last; i++) { - ret = nvme_tcp_start_queue(ctrl, i); - if (ret) - goto out_stop_queues; - } - - return 0; - -out_stop_queues: - for (i--; i >= first; i--) - nvme_tcp_stop_queue(ctrl, i); - return ret; -} - static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl) { int ret; @@ -2197,22 +2191,64 @@ static int nvme_tcp_tls_check_psk(struct nvme_ctrl *ctrl) return 0; } -static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) +static void nvme_tcp_setup_queue_async(void *data, async_cookie_t cookie) { - int i, ret; + struct nvme_tcp_setup_ctx *ctx = data; + struct nvme_ctrl *ctrl = ctx->ctrl; + int ret; - for (i = 1; i < ctrl->queue_count; i++) { - ret = nvme_tcp_alloc_queue(ctrl, i, - ctrl->tls_pskid); - if (ret) - goto out_free_queues; + ret = nvme_tcp_alloc_queue(ctrl, ctx->qid, ctrl->tls_pskid); + if (ret) + goto out_err; + + ret = nvme_tcp_start_queue(ctrl, ctx->qid); + if (ret) + goto out_err; + + return; + +out_err: + WRITE_ONCE(*ctx->err, ret); +} + +static int nvme_tcp_setup_io_queues(struct nvme_ctrl *ctrl, unsigned int first, + unsigned int last) +{ + ASYNC_DOMAIN_EXCLUSIVE(queue_domain); + struct nvme_tcp_setup_ctx *ctxs; + int nr_queues = last - first; + int err = 0, i, ret; + + ctxs = kmalloc_objs(*ctxs, nr_queues); + if (!ctxs) + return -ENOMEM; + + for (i = 0; i < nr_queues; i++) { + ctxs[i].ctrl = ctrl; + ctxs[i].qid = first + i; + ctxs[i].err = &err; + async_schedule_domain(nvme_tcp_setup_queue_async, &ctxs[i], + &queue_domain); } + async_synchronize_full_domain(&queue_domain); + kfree(ctxs); + + ret = READ_ONCE(err); + if (ret) + goto out_free_queues; + return 0; out_free_queues: - for (i--; i >= 1; i--) - nvme_tcp_free_queue(ctrl, i); + for (i = first; i < last; i++) { + struct nvme_tcp_queue *queue = &to_tcp_ctrl(ctrl)->queues[i]; + + if (test_bit(NVME_TCP_Q_LIVE, &queue->flags)) + nvme_tcp_stop_queue(ctrl, i); + if (test_bit(NVME_TCP_Q_ALLOCATED, &queue->flags)) + nvme_tcp_free_queue(ctrl, i); + } return ret; } @@ -2254,10 +2290,6 @@ static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new) if (ret) return ret; - ret = __nvme_tcp_alloc_io_queues(ctrl); - if (ret) - return ret; - if (new) { ret = nvme_alloc_io_tag_set(ctrl, &to_tcp_ctrl(ctrl)->tag_set, &nvme_tcp_mq_ops, @@ -2268,12 +2300,12 @@ static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new) } /* - * Only start IO queues for which we have allocated the tagset + * Only setup IO queues for which we have allocated the tagset * and limited it to the available queues. On reconnects, the * queue number might have changed. */ nr_queues = min(ctrl->tagset->nr_hw_queues + 1, ctrl->queue_count); - ret = nvme_tcp_start_io_queues(ctrl, 1, nr_queues); + ret = nvme_tcp_setup_io_queues(ctrl, 1, nr_queues); if (ret) goto out_cleanup_connect_q; @@ -2297,12 +2329,14 @@ static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new) /* * If the number of queues has increased (reconnect case) - * start all new queues now. + * setup all new queues now. */ - ret = nvme_tcp_start_io_queues(ctrl, nr_queues, - ctrl->tagset->nr_hw_queues + 1); - if (ret) - goto out_wait_freeze_timed_out; + if (ctrl->tagset->nr_hw_queues + 1 > nr_queues) { + ret = nvme_tcp_setup_io_queues(ctrl, nr_queues, + ctrl->tagset->nr_hw_queues + 1); + if (ret) + goto out_wait_freeze_timed_out; + } return 0; @@ -3140,7 +3174,7 @@ static int __init nvme_tcp_init_module(void) return -ENOMEM; for_each_possible_cpu(cpu) - atomic_set(&nvme_tcp_cpu_queues[cpu], 0); + nvme_tcp_cpu_queues[cpu] = 0; nvmf_register_transport(&nvme_tcp_transport); return 0; -- 2.55.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-24 22:56 ` [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup Surabhi Gogte @ 2026-08-24 23:58 ` Randy Jennings 2026-08-30 22:00 ` Sagi Grimberg 1 sibling, 0 replies; 15+ messages in thread From: Randy Jennings @ 2026-08-24 23:58 UTC (permalink / raw) To: Surabhi Gogte Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, linux-nvme, linux-kernel, mkhalfella, adailey On Mon, Aug 24, 2026 at 3:57 PM Surabhi Gogte <sgogte@purestorage.com> wrote: > > Similar to commit 2a8513091d2f ("nvme-rdma: parallelize I/O queue > allocation and startup"), refactor nvme tcp I/O queue setup to use async > API, combining allocation and startup into a single parallel operation > per queue. This reduces connection and reconnection setup time when > there are delays in establishing connections, which is especially > important for high-core-count hosts. > > Key changes: > - Use async API to facilitate parallel calls for io queue setup. > - Add nvme_tcp_setup_ctx for propagating errors from async workers. > - Remove nvme_tcp_start_io_queues() and __nvme_tcp_alloc_io_queues(); > their logic is folded into nvme_tcp_setup_io_queues() and > nvme_tcp_configure_io_queues(). > - Allocate the io tag set before the queues so that the queue range is > known, and only set up the reconnect grow case if the queue count > actually increased. > - Serialize the cpu scan and claim in nvme_tcp_set_queue_io_cpu() with a > spinlock, as concurrent callers would otherwise select the same cpu. > The per-cpu counters no longer need to be atomics. > - Use init_net in nvme_tcp_alloc_queue() instead of the namespace of > current, which is no longer the connecting task once the allocation > runs from a worker. A controller is not guaranteed to be tied to a > namespace, as the reconnect and error recovery paths already run from > a workqueue in init_net. > > Testing on a 64-core host with 64 IO-queues shows nvme-tcp connection > time reduced from 61ms to 11ms. > > Signed-off-by: Surabhi Gogte <sgogte@purestorage.com> :~$ Reviewed-by: Randy Jennings <randyj@purestorage.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-24 22:56 ` [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup Surabhi Gogte 2026-08-24 23:58 ` Randy Jennings @ 2026-08-30 22:00 ` Sagi Grimberg 2026-08-30 22:25 ` Mohamed Khalfella 2026-09-04 16:05 ` Surabhi Gogte (she/her) 1 sibling, 2 replies; 15+ messages in thread From: Sagi Grimberg @ 2026-08-30 22:00 UTC (permalink / raw) To: Surabhi Gogte, Keith Busch, Jens Axboe, Christoph Hellwig Cc: linux-nvme, linux-kernel, mkhalfella, randyj, adailey On 25/08/2026 1:56, Surabhi Gogte wrote: > Similar to commit 2a8513091d2f ("nvme-rdma: parallelize I/O queue > allocation and startup"), refactor nvme tcp I/O queue setup to use async > API, combining allocation and startup into a single parallel operation > per queue. This reduces connection and reconnection setup time when > there are delays in establishing connections, which is especially > important for high-core-count hosts. > > Key changes: > - Use async API to facilitate parallel calls for io queue setup. > - Add nvme_tcp_setup_ctx for propagating errors from async workers. > - Remove nvme_tcp_start_io_queues() and __nvme_tcp_alloc_io_queues(); > their logic is folded into nvme_tcp_setup_io_queues() and > nvme_tcp_configure_io_queues(). > - Allocate the io tag set before the queues so that the queue range is > known, and only set up the reconnect grow case if the queue count > actually increased. > - Serialize the cpu scan and claim in nvme_tcp_set_queue_io_cpu() with a > spinlock, as concurrent callers would otherwise select the same cpu. > The per-cpu counters no longer need to be atomics. > - Use init_net in nvme_tcp_alloc_queue() instead of the namespace of > current, which is no longer the connecting task once the allocation > runs from a worker. A controller is not guaranteed to be tied to a > namespace, as the reconnect and error recovery paths already run from > a workqueue in init_net. Well, I think this is breaking 1be52169c3488ef98582ed553ab35cefa3978817 > > Testing on a 64-core host with 64 IO-queues shows nvme-tcp connection > time reduced from 61ms to 11ms. > > Signed-off-by: Surabhi Gogte <sgogte@purestorage.com> > --- > drivers/nvme/host/tcp.c | 126 +++++++++++++++++++++++++--------------- > 1 file changed, 80 insertions(+), 46 deletions(-) > > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c > index 354668ad29ac..30fe4c5abe0b 100644 > --- a/drivers/nvme/host/tcp.c > +++ b/drivers/nvme/host/tcp.c > @@ -7,6 +7,7 @@ > #include <linux/module.h> > #include <linux/init.h> > #include <linux/slab.h> > +#include <linux/async.h> > #include <linux/err.h> > #include <linux/crc32.h> > #include <linux/nvme-tcp.h> > @@ -54,7 +55,8 @@ MODULE_PARM_DESC(tls_handshake_timeout, > "nvme TLS handshake timeout in seconds (default 10)"); > #endif > > -static atomic_t nvme_tcp_cpu_queues[NR_CPUS]; > +static int nvme_tcp_cpu_queues[NR_CPUS]; > +static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock); > > enum nvme_tcp_send_state { > NVME_TCP_SEND_CMD_PDU = 0, > @@ -154,6 +156,12 @@ struct nvme_tcp_queue { > static DEFINE_MUTEX(nvme_tcp_ctrl_mutex); > static LIST_HEAD_GUARDED(nvme_tcp_ctrl_list, nvme_tcp_ctrl_mutex); > > +struct nvme_tcp_setup_ctx { nvme_tcp_queue_setup_ctx? > + struct nvme_ctrl *ctrl; > + int qid; > + int *err; > +}; > + > struct nvme_tcp_ctrl { > /* read only in the hot path */ > struct nvme_tcp_queue *queues; > @@ -1718,9 +1726,10 @@ static void nvme_tcp_set_queue_io_cpu(struct nvme_tcp_queue *queue) > goto out; > > /* Search for the least used cpu from the mq_map */ > + spin_lock(&nvme_tcp_cpu_queues_lock); > io_cpu = WORK_CPU_UNBOUND; > for_each_online_cpu(cpu) { > - int num_queues = atomic_read(&nvme_tcp_cpu_queues[cpu]); > + int num_queues = nvme_tcp_cpu_queues[cpu]; > > if (mq_map[cpu] != qid) > continue; > @@ -1731,9 +1740,10 @@ static void nvme_tcp_set_queue_io_cpu(struct nvme_tcp_queue *queue) > } > if (io_cpu != WORK_CPU_UNBOUND) { > queue->io_cpu = io_cpu; > - atomic_inc(&nvme_tcp_cpu_queues[io_cpu]); > + nvme_tcp_cpu_queues[io_cpu]++; > set_bit(NVME_TCP_Q_IO_CPU_SET, &queue->flags); > } > + spin_unlock(&nvme_tcp_cpu_queues_lock); > out: > dev_dbg(ctrl->ctrl.device, "queue %d: using cpu %d\n", > qid, queue->io_cpu); > @@ -1846,7 +1856,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, > queue->cmnd_capsule_len = sizeof(struct nvme_command) + > NVME_TCP_ADMIN_CCSZ; > > - ret = sock_create_kern(current->nsproxy->net_ns, > + ret = sock_create_kern(&init_net, I don't think we can just change this... ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-30 22:00 ` Sagi Grimberg @ 2026-08-30 22:25 ` Mohamed Khalfella 2026-08-30 22:44 ` Sagi Grimberg 2026-09-04 16:05 ` Surabhi Gogte (she/her) 1 sibling, 1 reply; 15+ messages in thread From: Mohamed Khalfella @ 2026-08-30 22:25 UTC (permalink / raw) To: Sagi Grimberg Cc: Surabhi Gogte, Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, randyj, adailey On Mon 2026-08-31 01:00:52 +0300, Sagi Grimberg wrote: > > > On 25/08/2026 1:56, Surabhi Gogte wrote: > > - ret = sock_create_kern(current->nsproxy->net_ns, > > + ret = sock_create_kern(&init_net, > > I don't think we can just change this... Why we can not change it? Today the error recovery code running in worker thread will be using init_net anyway, no? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-30 22:25 ` Mohamed Khalfella @ 2026-08-30 22:44 ` Sagi Grimberg 2026-08-30 22:59 ` Mohamed Khalfella 0 siblings, 1 reply; 15+ messages in thread From: Sagi Grimberg @ 2026-08-30 22:44 UTC (permalink / raw) To: Mohamed Khalfella Cc: Surabhi Gogte, Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, randyj, adailey On 31/08/2026 1:25, Mohamed Khalfella wrote: > On Mon 2026-08-31 01:00:52 +0300, Sagi Grimberg wrote: >> >> On 25/08/2026 1:56, Surabhi Gogte wrote: >>> - ret = sock_create_kern(current->nsproxy->net_ns, >>> + ret = sock_create_kern(&init_net, >> I don't think we can just change this... > Why we can not change it? Today the error recovery code running in > worker thread will be using init_net anyway, no? See commit: 1be52169c3488ef98582ed553ab35cefa3978817 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-30 22:44 ` Sagi Grimberg @ 2026-08-30 22:59 ` Mohamed Khalfella 2026-09-04 16:04 ` Surabhi Gogte (she/her) 0 siblings, 1 reply; 15+ messages in thread From: Mohamed Khalfella @ 2026-08-30 22:59 UTC (permalink / raw) To: Sagi Grimberg Cc: Surabhi Gogte, Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, randyj, adailey On Mon 2026-08-31 01:44:32 +0300, Sagi Grimberg wrote: > > > On 31/08/2026 1:25, Mohamed Khalfella wrote: > > On Mon 2026-08-31 01:00:52 +0300, Sagi Grimberg wrote: > >> > >> On 25/08/2026 1:56, Surabhi Gogte wrote: > >>> - ret = sock_create_kern(current->nsproxy->net_ns, > >>> + ret = sock_create_kern(&init_net, > >> I don't think we can just change this... > > Why we can not change it? Today the error recovery code running in > > worker thread will be using init_net anyway, no? > See commit: 1be52169c3488ef98582ed553ab35cefa3978817 Thanks for the pointer. I was not aware of that. What about error recovery case? I _think_ it will be using init_net, no? Also, Will the change to sock_create_kern(&init_net) break the fix in commit 1be52169c348 ("nvme-tcp: fix selinux denied when calling sock_sendmsg")? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-30 22:59 ` Mohamed Khalfella @ 2026-09-04 16:04 ` Surabhi Gogte (she/her) 2026-09-05 22:29 ` Sagi Grimberg 0 siblings, 1 reply; 15+ messages in thread From: Surabhi Gogte (she/her) @ 2026-09-04 16:04 UTC (permalink / raw) To: Mohamed Khalfella Cc: Sagi Grimberg, Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, randyj, adailey On Sun, Aug 30, 2026 at 3:59 PM Mohamed Khalfella <mkhalfella@purestorage.com> wrote: > > On Mon 2026-08-31 01:44:32 +0300, Sagi Grimberg wrote: > > > > > > On 31/08/2026 1:25, Mohamed Khalfella wrote: > > > On Mon 2026-08-31 01:00:52 +0300, Sagi Grimberg wrote: > > >> > > >> On 25/08/2026 1:56, Surabhi Gogte wrote: > > >>> - ret = sock_create_kern(current->nsproxy->net_ns, > > >>> + ret = sock_create_kern(&init_net, > > >> I don't think we can just change this... > > > Why we can not change it? Today the error recovery code running in > > > worker thread will be using init_net anyway, no? > > See commit: 1be52169c3488ef98582ed553ab35cefa3978817 > > Thanks for the pointer. I was not aware of that. What about error > recovery case? I _think_ it will be using init_net, no? > From what I gathered, reconnect and error recovery run on kworkers, so those paths were already resolving to init_net even before this change. If we pin the caller's netns to the ctrl, we can keep netns consistent throughout, if that is desired. > Also, Will the change to sock_create_kern(&init_net) break the fix in > commit 1be52169c348 ("nvme-tcp: fix selinux denied when calling > sock_sendmsg")? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-09-04 16:04 ` Surabhi Gogte (she/her) @ 2026-09-05 22:29 ` Sagi Grimberg 0 siblings, 0 replies; 15+ messages in thread From: Sagi Grimberg @ 2026-09-05 22:29 UTC (permalink / raw) To: Surabhi Gogte (she/her), Mohamed Khalfella Cc: Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, randyj, adailey On 04/09/2026 19:04, Surabhi Gogte (she/her) wrote: > On Sun, Aug 30, 2026 at 3:59 PM Mohamed Khalfella > <mkhalfella@purestorage.com> wrote: >> On Mon 2026-08-31 01:44:32 +0300, Sagi Grimberg wrote: >>> >>> On 31/08/2026 1:25, Mohamed Khalfella wrote: >>>> On Mon 2026-08-31 01:00:52 +0300, Sagi Grimberg wrote: >>>>> On 25/08/2026 1:56, Surabhi Gogte wrote: >>>>>> - ret = sock_create_kern(current->nsproxy->net_ns, >>>>>> + ret = sock_create_kern(&init_net, >>>>> I don't think we can just change this... >>>> Why we can not change it? Today the error recovery code running in >>>> worker thread will be using init_net anyway, no? >>> See commit: 1be52169c3488ef98582ed553ab35cefa3978817 >> Thanks for the pointer. I was not aware of that. What about error >> recovery case? I _think_ it will be using init_net, no? >> > From what I gathered, reconnect and error recovery run on kworkers, > so those paths were already resolving to init_net even before this change. > If we pin the caller's netns to the ctrl, we can keep netns consistent > throughout, > if that is desired. We probably should. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-08-30 22:00 ` Sagi Grimberg 2026-08-30 22:25 ` Mohamed Khalfella @ 2026-09-04 16:05 ` Surabhi Gogte (she/her) 2026-09-05 22:30 ` Sagi Grimberg 1 sibling, 1 reply; 15+ messages in thread From: Surabhi Gogte (she/her) @ 2026-09-04 16:05 UTC (permalink / raw) To: Sagi Grimberg Cc: Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, mkhalfella, randyj, adailey On Sun, Aug 30, 2026 at 3:00 PM Sagi Grimberg <sagi@grimberg.me> wrote: > > > > On 25/08/2026 1:56, Surabhi Gogte wrote: > > Similar to commit 2a8513091d2f ("nvme-rdma: parallelize I/O queue > > allocation and startup"), refactor nvme tcp I/O queue setup to use async > > API, combining allocation and startup into a single parallel operation > > per queue. This reduces connection and reconnection setup time when > > there are delays in establishing connections, which is especially > > important for high-core-count hosts. > > > > Key changes: > > - Use async API to facilitate parallel calls for io queue setup. > > - Add nvme_tcp_setup_ctx for propagating errors from async workers. > > - Remove nvme_tcp_start_io_queues() and __nvme_tcp_alloc_io_queues(); > > their logic is folded into nvme_tcp_setup_io_queues() and > > nvme_tcp_configure_io_queues(). > > - Allocate the io tag set before the queues so that the queue range is > > known, and only set up the reconnect grow case if the queue count > > actually increased. > > - Serialize the cpu scan and claim in nvme_tcp_set_queue_io_cpu() with a > > spinlock, as concurrent callers would otherwise select the same cpu. > > The per-cpu counters no longer need to be atomics. > > - Use init_net in nvme_tcp_alloc_queue() instead of the namespace of > > current, which is no longer the connecting task once the allocation > > runs from a worker. A controller is not guaranteed to be tied to a > > namespace, as the reconnect and error recovery paths already run from > > a workqueue in init_net. > > Well, I think this is breaking 1be52169c3488ef98582ed553ab35cefa3978817 > Trying to understand how this breaks the commit 1be52169c348. Added my understanding later in the thread. > > @@ -154,6 +156,12 @@ struct nvme_tcp_queue { > > static DEFINE_MUTEX(nvme_tcp_ctrl_mutex); > > static LIST_HEAD_GUARDED(nvme_tcp_ctrl_list, nvme_tcp_ctrl_mutex); > > > > +struct nvme_tcp_setup_ctx { > > nvme_tcp_queue_setup_ctx? > Yeah, can rename it. > > + spin_unlock(&nvme_tcp_cpu_queues_lock); > > out: > > dev_dbg(ctrl->ctrl.device, "queue %d: using cpu %d\n", > > qid, queue->io_cpu); > > @@ -1846,7 +1856,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, > > queue->cmnd_capsule_len = sizeof(struct nvme_command) + > > NVME_TCP_ADMIN_CCSZ; > > > > - ret = sock_create_kern(current->nsproxy->net_ns, > > + ret = sock_create_kern(&init_net, > > I don't think we can just change this... Passing init_net explicitly does change behavior on the synchronous connect path. However, with queue allocation now moved into an async worker, current->nsproxy->net_ns resolves to init_net in that context anyway, so the initial connect path already loses the caller's netns regardless of which argument is passed. If the goal is to actually preserve the caller's netns through the full ctrl lifecycle - initial connect, reconnect, and error recovery; it can be pinned at ctrl creation like: nvme_tcp_alloc_ctrl(): to_tcp_ctrl(ctrl)->net = get_net(current->nsproxy->net_ns); nvme_tcp_free_ctrl(): put_net(to_tcp_ctrl(ctrl)->net); nvme_tcp_alloc_queue(): sock_create_kern(to_tcp_ctrl(ctrl)->net, ...); This captures the caller's netns while still in userspace context, then carries it through all async paths — so reconnect and error recovery also honor the original netns rather than falling back to the kworker's init_net. Is this approach preferred? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-09-04 16:05 ` Surabhi Gogte (she/her) @ 2026-09-05 22:30 ` Sagi Grimberg 2026-09-10 20:32 ` Surabhi Gogte (she/her) 0 siblings, 1 reply; 15+ messages in thread From: Sagi Grimberg @ 2026-09-05 22:30 UTC (permalink / raw) To: Surabhi Gogte (she/her) Cc: Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, mkhalfella, randyj, adailey >>> qid, queue->io_cpu); >>> @@ -1846,7 +1856,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, >>> queue->cmnd_capsule_len = sizeof(struct nvme_command) + >>> NVME_TCP_ADMIN_CCSZ; >>> >>> - ret = sock_create_kern(current->nsproxy->net_ns, >>> + ret = sock_create_kern(&init_net, >> I don't think we can just change this... > Passing init_net explicitly does change behavior on the synchronous connect > path. However, with queue allocation now moved into an async worker, > current->nsproxy->net_ns resolves to init_net in that context anyway, so > the initial connect path already loses the caller's netns regardless of > which argument is passed. > > If the goal is to actually preserve the caller's netns through the full > ctrl lifecycle - initial connect, reconnect, and error recovery; it can > be pinned at ctrl creation like: > > nvme_tcp_alloc_ctrl(): > to_tcp_ctrl(ctrl)->net = get_net(current->nsproxy->net_ns); > > nvme_tcp_free_ctrl(): > put_net(to_tcp_ctrl(ctrl)->net); > > nvme_tcp_alloc_queue(): > sock_create_kern(to_tcp_ctrl(ctrl)->net, ...); > > This captures the caller's netns while still in userspace context, then > carries it through all async paths — so reconnect and error recovery > also honor the original netns rather than falling back to the kworker's > init_net. > > Is this approach preferred? Yes I think so ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup 2026-09-05 22:30 ` Sagi Grimberg @ 2026-09-10 20:32 ` Surabhi Gogte (she/her) 0 siblings, 0 replies; 15+ messages in thread From: Surabhi Gogte (she/her) @ 2026-09-10 20:32 UTC (permalink / raw) To: Sagi Grimberg Cc: Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, linux-kernel, mkhalfella, randyj, adailey On Sat, Sep 5, 2026 at 3:30 PM Sagi Grimberg <sagi@grimberg.me> wrote: > > >>> qid, queue->io_cpu); > >>> @@ -1846,7 +1856,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, > >>> queue->cmnd_capsule_len = sizeof(struct nvme_command) + > >>> NVME_TCP_ADMIN_CCSZ; > >>> > >>> - ret = sock_create_kern(current->nsproxy->net_ns, > >>> + ret = sock_create_kern(&init_net, > >> I don't think we can just change this... > > Passing init_net explicitly does change behavior on the synchronous connect > > path. However, with queue allocation now moved into an async worker, > > current->nsproxy->net_ns resolves to init_net in that context anyway, so > > the initial connect path already loses the caller's netns regardless of > > which argument is passed. > > > > If the goal is to actually preserve the caller's netns through the full > > ctrl lifecycle - initial connect, reconnect, and error recovery; it can > > be pinned at ctrl creation like: > > > > nvme_tcp_alloc_ctrl(): > > to_tcp_ctrl(ctrl)->net = get_net(current->nsproxy->net_ns); > > > > nvme_tcp_free_ctrl(): > > put_net(to_tcp_ctrl(ctrl)->net); > > > > nvme_tcp_alloc_queue(): > > sock_create_kern(to_tcp_ctrl(ctrl)->net, ...); > > > > This captures the caller's netns while still in userspace context, then > > carries it through all async paths — so reconnect and error recovery > > also honor the original netns rather than falling back to the kworker's > > init_net. > > > > Is this approach preferred? > > Yes I think so Alright, sent the patch with the implementation in v3 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-09-10 20:32 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-24 22:56 [PATCH v2 0/2] nvme-tcp: parallelize I/O queue connect Surabhi Gogte 2026-08-24 22:56 ` [PATCH v2 1/2] nvme-tcp: refactor I/O queue setup path Surabhi Gogte 2026-08-30 21:56 ` Sagi Grimberg 2026-09-02 21:25 ` Surabhi Gogte (she/her) 2026-08-24 22:56 ` [PATCH v2 2/2] nvme-tcp: parallelize I/O queue allocation and startup Surabhi Gogte 2026-08-24 23:58 ` Randy Jennings 2026-08-30 22:00 ` Sagi Grimberg 2026-08-30 22:25 ` Mohamed Khalfella 2026-08-30 22:44 ` Sagi Grimberg 2026-08-30 22:59 ` Mohamed Khalfella 2026-09-04 16:04 ` Surabhi Gogte (she/her) 2026-09-05 22:29 ` Sagi Grimberg 2026-09-04 16:05 ` Surabhi Gogte (she/her) 2026-09-05 22:30 ` Sagi Grimberg 2026-09-10 20:32 ` Surabhi Gogte (she/her)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox