Re: [PATCH v5 2/2] nvme-tcp: allow setting per-queue io_cpu through sysfs

[Saravanan D]

On Sun, Oct 18, 2026 at 01:15:14AM +0000, sashiko-bot@xxxxxxxxxx wrote:
[...]
> [Severity: Medium]
> In nvme_tcp_setup_queue_async(), nvme_tcp_alloc_queue() runs before
> nvme_tcp_start_queue(), where nvme_tcp_set_queue_io_cpu() is called:
>
> nvme_tcp_setup_queue_async()
>   nvme_tcp_alloc_queue()
>     if (!test_bit(NVME_TCP_Q_IO_CPU_USER, &queue->flags))
>       nvme_tcp_queue_change_io_cpu(queue, WORK_CPU_UNBOUND);
>     ...
>     nvme_tcp_add_queue_sysfs(queue);
>   nvme_tcp_start_queue()
>     if (idx) {
>       nvme_tcp_set_queue_io_cpu(queue);
>       ret = nvmf_connect_io_queue(nctrl, idx);
>     }
>
> Because nvme_tcp_add_queue_sysfs() publishes the sysfs attributes in
> nvme_tcp_alloc_queue() before nvme_tcp_set_queue_io_cpu() runs in
> nvme_tcp_start_queue(), can a userspace control plane reading io_cpu and
> managed as soon as the queue directory appears observe io_cpu as -1
> (unbound) with managed=1 even when wq_unbound is false?

Agreed. v6 adds the queue directory in nvme_tcp_start_queue(), right
after nvme_tcp_set_queue_io_cpu(), instead of at the end of
nvme_tcp_alloc_queue(), so io_cpu and managed only become visible once
the selection has run. The directory is still removed in
nvme_tcp_free_queue(), which the setup error path calls for every
allocated queue, so a failed connect does not leave it behind.

> Also, during parallel reconnects in nvme_tcp_setup_io_queues(), a queue
> with NVME_TCP_Q_IO_CPU_USER set does not re-increment
> nvme_tcp_cpu_queues[io_cpu] until it reaches nvme_tcp_set_queue_io_cpu()
> after completing the blocking TCP, TLS, and ICReq handshakes in
> nvme_tcp_alloc_queue(). If a driver-managed queue finishes its handshake
> first, could nvme_tcp_set_queue_io_cpu() pick a CPU without accounting for
> the user-assigned queue's load?

This window exists for every queue, not only user assigned ones,
since a queue is counted only once its handshakes complete and it
reaches its own selection. The selection is best effort, and the user
assigned queue keeps its io_cpu regardless, so I'd rather not add
accounting to nvme_tcp_alloc_queue() and its error path for it.

Thanks,
Saravanan D.