Re: [PATCH v3 3/3] nvme-tcp: parallelize I/O queue allocation and startup

[Surabhi Gogte (she/her)]

On Fri, Sep 11, 2026 at 2:27 PM Sagi Grimberg <sagi@xxxxxxxxxxx> wrote:
>
>
>
> On 10/09/2026 23:28, 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_queue_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.
> >
> > 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@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/nvme/host/tcp.c | 124 +++++++++++++++++++++++++---------------
> >   1 file changed, 79 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> > index fe0e581608b4..0fc244e676f9 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_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;
> > @@ -1719,9 +1727,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;
> > @@ -1732,9 +1741,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);
> > @@ -2011,8 +2021,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))
> > @@ -2119,25 +2132,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;
> > @@ -2198,22 +2192,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_queue_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_queue_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;
> >   }
> > @@ -2255,10 +2291,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,
> > @@ -2269,12 +2301,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;
> >
> > @@ -2298,12 +2330,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;
> > +     }
>
> This if condition can probably move into nvme_tcp_setup_io_queues() and
> exit early if both are the same. Other than that, patch looks solid. Can
> you please also run blktests to see that nothing broke. There may be
> some tests that have subtle race conditions. rdma and tcp btw.

Thanks. Sending a v4 version with this change. Also, I ran blktests
for tcp and rdma. The blktests nvme/062 "Create TLS-encrypted
connections" and nvme/063 "Create authenticated TCP connections with
secure concatenation" related to tls need ktls-utils-1.4.0 (c63c04b)
(tlshd) linked against gnutls 3.8.5. rdma also passed fine.