[PATCH v3 0/2] nvme-tcp: NIC topology aware I/O queue scaling and queue info export

[PATCH v3 0/2] nvme-tcp: NIC topology aware I/O queue scaling and queue info export

[Nilay Shroff]

This patch series has been revised based on feedback from the netdev
community [1].

Following the feedback, the logic for determining the number of fabric I/O
queues (nr-io-queues) has been moved to userspace in nvme-cli [2]. As a
result, the first two patches from the previous series, which implemented
this logic in the kernel driver, have been dropped. This series contains
the remaining two patches, which export NVMe/TCP I/O queue information
through debugfs to facilitate host-side network tuning.

The first patch introduces debugfs helper functions that can be used by
drivers to export debugfs attributes. 

The second patch exports NVMe/TCP I/O queue and TCP flow information through
debugfs. For each I/O queue, it provides:
   - queue id (qid)
   - CPU affinity
   - TCP flow (src/dst IP and ports)

This enables userspace tools to configure:
   - IRQ affinity
   - RPS/XPS
   - ntuple steering
   - or any other scaling as deemed feasible

Together with tunning, these changes allow better alignment of:
   flow -> NIC queue -> IRQ -> CPU -> NVMe/TCP I/O worker

Improving this alignment can enhance flow-to-CPU locality, potentially
improving performance and reducing I/O latency. Performance evaluation
results comparing the baseline with the tuned configuration are
available in [3].

Thanks!

[1]: https://lore.kernel.org/all/20260807160904.2eb1b3d1@kernel.org/
[2]: https://github.com/linux-nvme/nvme-cli/pull/4140
[3]: https://lore.kernel.org/all/20260731073918.614014-1-nilay@linux.ibm.com/

Changes from v2
  - Dropped patch #1 and #2 from v2
  - Added new debugfs attribute parameter "is_visible" which could be useful
    to hide the attribute for certain configuration or transport. 
Link to v2: https://lore.kernel.org/all/20260731073918.614014-1-nilay@linux.ibm.com/

Nilay Shroff (2):
  nvme: add debugfs helpers for NVMe drivers
  nvme: expose queue information via debugfs

 drivers/nvme/host/Makefile  |   2 +-
 drivers/nvme/host/core.c    |   3 +
 drivers/nvme/host/debugfs.c | 207 ++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h    |  12 +++
 drivers/nvme/host/tcp.c     |  54 ++++++++++
 5 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/nvme/host/debugfs.c

-- 
2.53.0

[PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers

[Nilay Shroff]

Introduce helper APIs that allow NVMe drivers to register and unregister
debugfs entries, along with a reusable attribute structure for defining
new debugfs files.

The implementation uses seq_file interfaces to safely expose per-
namespace or per-path statistics, while supporting both simple show
callbacks and full seq_operations.

This will be used by subsequent patches to expose NVMe-TCP queue
and flow information for tuning NVMe TCP I/O workqueue and network stack
components.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/Makefile  |   2 +-
 drivers/nvme/host/debugfs.c | 133 ++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h    |  10 +++
 3 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 drivers/nvme/host/debugfs.c

diff --git a/drivers/nvme/host/Makefile b/drivers/nvme/host/Makefile
index 67563a69f7dc..314d3439406b 100644
--- a/drivers/nvme/host/Makefile
+++ b/drivers/nvme/host/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_NVME_FC)			+= nvme-fc.o
 obj-$(CONFIG_NVME_TCP)			+= nvme-tcp.o
 obj-$(CONFIG_NVME_APPLE)		+= nvme-apple.o
 
-nvme-core-y				+= core.o ioctl.o sysfs.o pr.o
+nvme-core-y				+= core.o ioctl.o sysfs.o pr.o debugfs.o
 nvme-core-$(CONFIG_NVME_VERBOSE_ERRORS)	+= constants.o
 nvme-core-$(CONFIG_TRACING)		+= trace.o
 nvme-core-$(CONFIG_NVME_MULTIPATH)	+= multipath.o
diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
new file mode 100644
index 000000000000..798fec4730f0
--- /dev/null
+++ b/drivers/nvme/host/debugfs.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 IBM Corporation
+ *	Nilay Shroff <nilay@linux.ibm.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+
+#include "nvme.h"
+
+struct nvme_debugfs_attr {
+	const char *name;
+	umode_t mode;
+	int (*show)(void *data, struct seq_file *m);
+	const struct seq_operations *seq_ops;
+	bool (*get)(void *data);
+	void (*put)(void *data);
+	bool (*is_visible)(void *data);
+};
+
+struct nvme_debugfs_ctx {
+	void *data;
+	struct nvme_debugfs_attr *attr;
+};
+
+static int nvme_debugfs_show(struct seq_file *m, void *v)
+{
+	struct nvme_debugfs_ctx *ctx = m->private;
+	void *data = ctx->data;
+	struct nvme_debugfs_attr *attr = ctx->attr;
+
+	return attr->show(data, m);
+}
+
+static int nvme_debugfs_open(struct inode *inode, struct file *file)
+{
+	void *data = inode->i_private;
+	struct nvme_debugfs_attr *attr = debugfs_get_aux(file);
+	struct nvme_debugfs_ctx *ctx;
+	struct seq_file *m;
+	int ret;
+
+	if (attr->get && !attr->get(data))
+		return -ENODEV;
+
+	ctx = kzalloc_obj(*ctx);
+	if (!ctx) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ctx->data = data;
+	ctx->attr = attr;
+
+	if (attr->seq_ops) {
+		ret = seq_open(file, attr->seq_ops);
+		if (ret)
+			goto out_free;
+
+		m = file->private_data;
+		m->private = ctx;
+		return ret;
+	}
+
+	if (WARN_ON_ONCE(!attr->show)) {
+		ret = -EPERM;
+		goto out_free;
+	}
+
+	ret = single_open(file, nvme_debugfs_show, ctx);
+	if (!ret)
+		return ret;
+
+out_free:
+	kfree(ctx);
+out:
+	if (attr->put)
+		attr->put(data);
+	return ret;
+}
+
+static int nvme_debugfs_release(struct inode *inode, struct file *file)
+{
+	struct seq_file *m = file->private_data;
+	struct nvme_debugfs_ctx *ctx = m->private;
+	struct nvme_debugfs_attr *attr = ctx->attr;
+	int ret;
+
+	if (attr->seq_ops)
+		ret = seq_release(inode, file);
+	else
+		ret = single_release(inode, file);
+
+	if (attr->put)
+		attr->put(ctx->data);
+
+	kfree(ctx);
+	return ret;
+}
+
+static const struct file_operations nvme_debugfs_fops = {
+	.owner   = THIS_MODULE,
+	.open    = nvme_debugfs_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = nvme_debugfs_release,
+};
+
+static const struct nvme_debugfs_attr nvme_debugfs_attrs[] = {
+	{},
+};
+
+static void nvme_debugfs_create_files(struct request_queue *q,
+		const struct nvme_debugfs_attr *attr, void *data)
+{
+	if (WARN_ON_ONCE(!q->debugfs_dir))
+		return;
+
+	for (; attr->name; attr++) {
+		if (attr->is_visible && !attr->is_visible(data))
+			continue;
+		debugfs_create_file_aux(attr->name, attr->mode, q->debugfs_dir,
+				data, (void *)attr, &nvme_debugfs_fops);
+	}
+}
+
+void nvme_debugfs_register(struct gendisk *disk)
+{
+	nvme_debugfs_create_files(disk->queue, nvme_debugfs_attrs,
+			disk->private_data);
+}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a8ef62b8e34b..db2ab1469fd0 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -483,6 +483,16 @@ struct nvme_ctrl {
 	u16			awupf; /* 0's based value. */
 };
 
+void nvme_debugfs_register(struct gendisk *disk);
+static inline void nvme_debugfs_unregister(struct gendisk *disk)
+{
+	/*
+	 * Nothing to do for now. When the request queue is unregistered,
+	 * all files under q->debugfs_dir are recursively deleted.
+	 * This is just a placeholder; the compiler will optimize it out.
+	 */
+}
+
 static inline enum nvme_ctrl_state nvme_ctrl_state(struct nvme_ctrl *ctrl)
 {
 	return READ_ONCE(ctrl->state);
-- 
2.53.0

[PATCH v3 2/2] nvme: expose queue information via debugfs

[Nilay Shroff]

Add a new debugfs attribute "io_queue_info" to expose per-queue
information for NVMe controllers. For NVMe-TCP, this includes the
CPU handling each I/O queue and the associated TCP flow (source and
destination address/port).

This information can be useful for understanding and tuning the
interaction between NVMe-TCP I/O queues and network stack components,
such as IRQ affinity, RPS/RFS, XPS, or NIC flow steering (ntuple).

The data is exported using seq_file interfaces to allow iteration
over all controller queues.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/host/core.c    |  3 ++
 drivers/nvme/host/debugfs.c | 76 ++++++++++++++++++++++++++++++++++++-
 drivers/nvme/host/nvme.h    |  2 +
 drivers/nvme/host/tcp.c     | 54 ++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 531f9bb14bab..15334521c314 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4316,6 +4316,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
 	if (device_add_disk(ctrl->device, ns->disk, nvme_ns_attr_groups))
 		goto out_cleanup_ns_from_list;
 
+	nvme_debugfs_register(ns->disk);
+
 	if (!nvme_ns_head_multipath(ns->head))
 		nvme_add_ns_cdev(ns);
 
@@ -4399,6 +4401,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 
 	nvme_mpath_remove_sysfs_link(ns);
 
+	nvme_debugfs_unregister(ns->disk);
 	del_gendisk(ns->disk);
 
 	mutex_lock(&ns->ctrl->namespaces_lock);
diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
index 798fec4730f0..8ea0413f7b75 100644
--- a/drivers/nvme/host/debugfs.c
+++ b/drivers/nvme/host/debugfs.c
@@ -25,6 +25,61 @@ struct nvme_debugfs_ctx {
 	struct nvme_debugfs_attr *attr;
 };
 
+static void *nvme_io_queue_info_start(struct seq_file *m, loff_t *pos)
+{
+	struct nvme_debugfs_ctx *ctx = m->private;
+	struct nvme_ns *ns = ctx->data;
+	struct nvme_ctrl *ctrl = ns->ctrl;
+
+	/*
+	 * IO queues starts at offset 1.
+	 */
+	return (++*pos < ctrl->queue_count) ? pos : NULL;
+}
+
+static void *nvme_io_queue_info_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	struct nvme_debugfs_ctx *ctx = m->private;
+	struct nvme_ns *ns = ctx->data;
+	struct nvme_ctrl *ctrl = ns->ctrl;
+
+	return (++*pos < ctrl->queue_count) ? pos : NULL;
+}
+
+static void nvme_io_queue_info_stop(struct seq_file *m, void *v)
+{
+	/* nothing to be done */
+}
+
+static int nvme_io_queue_info_show(struct seq_file *m, void *v)
+{
+	struct nvme_debugfs_ctx *ctx = m->private;
+	struct nvme_ns *ns = ctx->data;
+	struct nvme_ctrl *ctrl = ns->ctrl;
+
+	if (ctrl->ops->print_io_queue_info)
+		return ctrl->ops->print_io_queue_info(m, ctrl, *(loff_t *)v);
+
+	return 0;
+}
+
+const struct seq_operations nvme_io_queue_info_seq_ops = {
+	.start = nvme_io_queue_info_start,
+	.next = nvme_io_queue_info_next,
+	.stop = nvme_io_queue_info_stop,
+	.show = nvme_io_queue_info_show
+};
+
+static bool nvme_debugfs_get_ns(void *data)
+{
+	return nvme_get_ns(data);
+}
+
+static void nvme_debugfs_put_ns(void *data)
+{
+	nvme_put_ns(data);
+}
+
 static int nvme_debugfs_show(struct seq_file *m, void *v)
 {
 	struct nvme_debugfs_ctx *ctx = m->private;
@@ -100,6 +155,17 @@ static int nvme_debugfs_release(struct inode *inode, struct file *file)
 	return ret;
 }
 
+static bool nvme_debugfs_io_queue_info_visible(void *data)
+{
+	struct nvme_ns *ns = data;
+	struct nvme_ctrl *ctrl = ns->ctrl;
+
+	if (ctrl->ops->print_io_queue_info)
+		return true;
+
+	return false;
+}
+
 static const struct file_operations nvme_debugfs_fops = {
 	.owner   = THIS_MODULE,
 	.open    = nvme_debugfs_open,
@@ -109,7 +175,15 @@ static const struct file_operations nvme_debugfs_fops = {
 };
 
 static const struct nvme_debugfs_attr nvme_debugfs_attrs[] = {
-	{},
+	{
+		.name		= "io_queue_info",
+		.mode		= 0400,
+		.seq_ops	= &nvme_io_queue_info_seq_ops,
+		.get		= nvme_debugfs_get_ns,
+		.put		= nvme_debugfs_put_ns,
+		.is_visible	= nvme_debugfs_io_queue_info_visible,
+	},
+	{}
 };
 
 static void nvme_debugfs_create_files(struct request_queue *q,
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index db2ab1469fd0..48995bc6669c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -679,6 +679,8 @@ struct nvme_ctrl_ops {
 	void (*print_device_info)(struct nvme_ctrl *ctrl);
 	bool (*supports_pci_p2pdma)(struct nvme_ctrl *ctrl);
 	unsigned long (*get_virt_boundary)(struct nvme_ctrl *ctrl, bool is_admin);
+	int (*print_io_queue_info)(struct seq_file *m, struct nvme_ctrl *ctrl,
+			int qid);
 };
 
 /*
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index f876375dffd2..2cb697d33c02 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2701,6 +2701,59 @@ static void nvme_tcp_free_ctrl(struct nvme_ctrl *nctrl)
 	kfree(ctrl);
 }
 
+static int nvme_tcp_print_io_queue_info(struct seq_file *m,
+		struct nvme_ctrl *ctrl, int qid)
+{
+	int cpu;
+	struct sockaddr_storage src, dst;
+	struct nvme_tcp_ctrl *tctrl = to_tcp_ctrl(ctrl);
+	struct nvme_tcp_queue *queue = &tctrl->queues[qid];
+	int ret = -EINVAL;
+
+	if (!qid || qid >= ctrl->queue_count)
+		return -EINVAL;
+
+	if (!test_bit(NVME_TCP_Q_LIVE, &queue->flags))
+		return 0;
+
+	mutex_lock(&queue->queue_lock);
+	if (!queue->sock)
+		goto unlock;
+
+	ret = kernel_getsockname(queue->sock, (struct sockaddr *)&src);
+	if (ret <= 0)
+		goto unlock;
+
+	ret = kernel_getpeername(queue->sock, (struct sockaddr *)&dst);
+	if (ret <= 0)
+		goto unlock;
+
+	cpu = (queue->io_cpu == WORK_CPU_UNBOUND) ? -1 : queue->io_cpu;
+
+	if (src.ss_family == AF_INET) {
+		struct sockaddr_in *sip = (struct sockaddr_in *)&src;
+		struct sockaddr_in *dip = (struct sockaddr_in *)&dst;
+
+		seq_printf(m, "qid=%d cpu=%d src_ip=%pI4 src_port=%u dst_ip=%pI4 dst_port=%u\n",
+				qid, cpu,
+				&sip->sin_addr.s_addr, ntohs(sip->sin_port),
+				&dip->sin_addr.s_addr, ntohs(dip->sin_port));
+		ret = 0;
+	} else if (src.ss_family == AF_INET6) {
+		struct sockaddr_in6 *sip6 = (struct sockaddr_in6 *)&src;
+		struct sockaddr_in6 *dip6 = (struct sockaddr_in6 *)&dst;
+
+		seq_printf(m, "qid=%d cpu=%d src_ip=%pI6c src_port=%u dst_ip=%pI6c dst_port=%u\n",
+				qid, cpu,
+				&sip6->sin6_addr, ntohs(sip6->sin6_port),
+				&dip6->sin6_addr, ntohs(dip6->sin6_port));
+		ret = 0;
+	}
+unlock:
+	mutex_unlock(&queue->queue_lock);
+	return ret;
+}
+
 static void nvme_tcp_set_sg_null(struct nvme_command *c)
 {
 	struct nvme_sgl_desc *sg = &c->common.dptr.sgl;
@@ -3002,6 +3055,7 @@ static const struct nvme_ctrl_ops nvme_tcp_ctrl_ops = {
 	.get_address		= nvme_tcp_get_address,
 	.stop_ctrl		= nvme_tcp_stop_ctrl,
 	.get_virt_boundary	= nvmf_get_virt_boundary,
+	.print_io_queue_info	= nvme_tcp_print_io_queue_info,
 };
 
 static bool
-- 
2.53.0

Re: [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme: missing debugfs_mutex and NOIO scope in nvme_debugfs_create_files()
--

commit cd2cc953bf8eddec233eaabc47f820bf78836863
Author: Nilay Shroff <nilay@linux.ibm.com>

nvme: add debugfs helpers for NVMe drivers

This patch introduces helper APIs and an attribute structure for NVMe
drivers to register and unregister seq_file-based debugfs entries on a
request queue's debugfs directory.

> diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
> new file mode 100644
> index 0000000000000..798fec4730f0a
> --- /dev/null
> +++ b/drivers/nvme/host/debugfs.c
> @@ -0,0 +1,133 @@

[ ... ]

> +static void nvme_debugfs_create_files(struct request_queue *q,
> +		const struct nvme_debugfs_attr *attr, void *data)
> +{
> +	if (WARN_ON_ONCE(!q->debugfs_dir))
> +		return;
> +
> +	for (; attr->name; attr++) {
> +		if (attr->is_visible && !attr->is_visible(data))
> +			continue;
> +		debugfs_create_file_aux(attr->name, attr->mode, q->debugfs_dir,
> +				data, (void *)attr, &nvme_debugfs_fops);
> +	}
> +}

[Severity: Medium]
Should nvme_debugfs_create_files() hold q->debugfs_mutex and run under a
memalloc_noio_save() scope when accessing q->debugfs_dir and creating
debugfs files?

In include/linux/blkdev.h, struct request_queue documents that
debugfs_mutex serializes all debugfs metadata operations on debugfs_dir:

	struct dentry		*debugfs_dir;
	struct dentry		*sched_debugfs_dir;
	struct dentry		*rqos_debugfs_dir;
	/*
	 * Serializes all debugfs metadata operations using the above dentries.
	 */
	struct mutex		debugfs_mutex;

Furthermore, block/blk.h notes in blk_debugfs_lock_nomemsave() why NOIO
context is needed alongside debugfs_mutex (as introduced in commit
dfe48ea17973 ("blk-mq: use NOIO context to prevent deadlock during debugfs
creation")):

	/*
	 * debugfs directory and file creation can trigger fs reclaim, which can enter
	 * back into the block layer request_queue. This can cause deadlock if the
	 * queue is frozen. Use NOIO context together with debugfs_mutex to prevent fs
	 * reclaim from triggering block I/O.
	 */

When nvme_alloc_ns() calls nvme_debugfs_register(ns->disk) after
device_add_disk(), the namespace is already on ctrl->namespaces and the
disk is live in sysfs, while blk_register_queue() has already dropped
blk_debugfs_lock(q). If the queue is concurrently frozen (for example, by
a controller reset calling nvme_start_freeze() or a sysfs write calling
blk_mq_freeze_queue()), could GFP_KERNEL allocations inside
debugfs_create_file_aux() trigger direct filesystem reclaim into the
frozen queue and deadlock, or race with concurrent blktrace, scheduler,
rqos, or queue teardown debugfs operations on q->debugfs_dir?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026123411.1988949-4-nilay@linux.ibm.com?part=1

Re: [PATCH v3 2/2] nvme: expose queue information via debugfs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] nvme-tcp: race with queue teardown in nvme_tcp_print_io_queue_info()
- [Medium] nvme: debugfs: nvme_io_queue_info_start() skips queues on resumed reads
- [Low] nvme: debugfs: missing static qualifier on nvme_io_queue_info_seq_ops
--

commit 0f7d7347ae3aacfc41bb3e9a63226f587e2b9ffd
Author: Nilay Shroff <nilay@linux.ibm.com>

nvme: expose queue information via debugfs

This commit adds an io_queue_info debugfs attribute to expose per-queue
information for NVMe controllers using seq_file iteration. For NVMe-TCP,
it outputs each I/O queue's bound CPU and TCP socket source and
destination address and port.

> diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
> index 798fec4730f0a..8ea0413f7b753 100644
> --- a/drivers/nvme/host/debugfs.c
> +++ b/drivers/nvme/host/debugfs.c
> @@ -25,6 +25,61 @@ struct nvme_debugfs_ctx {
>  	struct nvme_debugfs_attr *attr;
>  };
>  
> +static void *nvme_io_queue_info_start(struct seq_file *m, loff_t *pos)
> +{
> +	struct nvme_debugfs_ctx *ctx = m->private;
> +	struct nvme_ns *ns = ctx->data;
> +	struct nvme_ctrl *ctrl = ns->ctrl;
> +
> +	/*
> +	 * IO queues starts at offset 1.
> +	 */
> +	return (++*pos < ctrl->queue_count) ? pos : NULL;
> +}

[Severity: Medium]
Will unconditionally pre-incrementing *pos in nvme_io_queue_info_start()
cause queues to be skipped when seq_read_iter() resumes reading on a
subsequent read() call or after a seq_file buffer overflow?

In fs/seq_file.c:seq_read_iter(), the Fill loop advances m->index via
m->op->next() before checking whether the user buffer is full or whether
m->op->show() overflowed the seq_file buffer:

	p = m->op->next(m, p, &m->index);
	...
	if (!p || IS_ERR(p))	// no next record for us
		break;
	if (m->count >= iov_iter_count(iter))
		break;
	err = m->op->show(m, p);
	if (err > 0) {		// ->show() says "skip it"
		m->count = offs;
	} else if (err || seq_has_overflowed(m)) {
		m->count = offs;
		break;
	}

When the loop stops because the seq_file buffer overflowed or the caller's
buffer was filled, m->index has already been incremented by
nvme_io_queue_info_next() to the next unshown queue ID. On the next read,
seq_read_iter() calls nvme_io_queue_info_start(m, &m->index), which
executes ++*pos a second time and skips that queue.

Should nvme_io_queue_info_start() only advance *pos when *pos is 0?

> +
> +static void *nvme_io_queue_info_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> +	struct nvme_debugfs_ctx *ctx = m->private;
> +	struct nvme_ns *ns = ctx->data;
> +	struct nvme_ctrl *ctrl = ns->ctrl;
> +
> +	return (++*pos < ctrl->queue_count) ? pos : NULL;
> +}
> +
> +static void nvme_io_queue_info_stop(struct seq_file *m, void *v)
> +{
> +	/* nothing to be done */
> +}
> +
> +static int nvme_io_queue_info_show(struct seq_file *m, void *v)
> +{
> +	struct nvme_debugfs_ctx *ctx = m->private;
> +	struct nvme_ns *ns = ctx->data;
> +	struct nvme_ctrl *ctrl = ns->ctrl;
> +
> +	if (ctrl->ops->print_io_queue_info)
> +		return ctrl->ops->print_io_queue_info(m, ctrl, *(loff_t *)v);
> +
> +	return 0;
> +}
> +
> +const struct seq_operations nvme_io_queue_info_seq_ops = {
> +	.start = nvme_io_queue_info_start,
> +	.next = nvme_io_queue_info_next,
> +	.stop = nvme_io_queue_info_stop,
> +	.show = nvme_io_queue_info_show
> +};

[Severity: Low]
Should nvme_io_queue_info_seq_ops be marked static since it is only used
within drivers/nvme/host/debugfs.c?

[ ... ]

> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 7190eb10c2481..a59169629d56b 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2691,6 +2691,59 @@ static void nvme_tcp_free_ctrl(struct nvme_ctrl *nctrl)
>  	kfree(ctrl);
>  }
>  
> +static int nvme_tcp_print_io_queue_info(struct seq_file *m,
> +		struct nvme_ctrl *ctrl, int qid)
> +{
> +	int cpu;
> +	struct sockaddr_storage src, dst;
> +	struct nvme_tcp_ctrl *tctrl = to_tcp_ctrl(ctrl);
> +	struct nvme_tcp_queue *queue = &tctrl->queues[qid];
> +	int ret = -EINVAL;
> +
> +	if (!qid || qid >= ctrl->queue_count)
> +		return -EINVAL;
> +
> +	if (!test_bit(NVME_TCP_Q_LIVE, &queue->flags))
> +		return 0;
> +
> +	mutex_lock(&queue->queue_lock);
> +	if (!queue->sock)
> +		goto unlock;
> +
> +	ret = kernel_getsockname(queue->sock, (struct sockaddr *)&src);
> +	if (ret <= 0)
> +		goto unlock;
> +
> +	ret = kernel_getpeername(queue->sock, (struct sockaddr *)&dst);
> +	if (ret <= 0)
> +		goto unlock;

[Severity: High]
Can nvme_tcp_print_io_queue_info() race with queue teardown during
controller reset or error recovery?

Holding a reference on struct nvme_ns keeps the namespace and controller
allocated, but does not prevent nvme_reset_ctrl_work() or
nvme_tcp_error_recovery_work() from running nvme_tcp_teardown_io_queues()
while io_queue_info is being read.

In nvme_tcp_stop_queue_nowait(), NVME_TCP_Q_LIVE is cleared under
queue->queue_lock while queue->sock remains non-NULL:

	mutex_lock(&queue->queue_lock);
	if (test_and_clear_bit(NVME_TCP_Q_LIVE, &queue->flags))
		__nvme_tcp_stop_queue(queue);
	/* Stopping the queue will disable TLS */
	queue->tls_enabled = false;
	mutex_unlock(&queue->queue_lock);

Shortly after, nvme_tcp_free_io_queues() -> nvme_tcp_free_queue() runs
without holding queue->queue_lock, frees queue->sock before clearing the
pointer, and destroys queue->queue_lock:

	__fput_sync(queue->sock->file);
	queue->sock = NULL;
	memalloc_noio_restore(noio_flag);

	kfree(queue->pdu);
	mutex_destroy(&queue->send_mutex);
	mutex_destroy(&queue->queue_lock);
	mutex_destroy(&queue->pf_cache_lock);

If nvme_tcp_print_io_queue_info() checks test_bit(NVME_TCP_Q_LIVE,
&queue->flags) just before teardown, it can acquire queue->queue_lock
after nvme_tcp_stop_queue_nowait() unlocks it, see queue->sock != NULL
because NVME_TCP_Q_LIVE is not re-checked under the lock, and dereference
queue->sock in kernel_getsockname() and kernel_getpeername() concurrently
with __fput_sync(queue->sock->file), or touch queue->queue_lock while
nvme_tcp_free_queue() destroys it (and reconnect re-initializes it in
nvme_tcp_alloc_queue()).

In addition, if !queue->sock is taken under queue->queue_lock (where ret
is still -EINVAL) or if kernel_getpeername() returns -ENOTCONN on a
shut-down socket, returning a negative error code from the seq_file show
callback aborts the entire seq_read_iter() read instead of skipping the
stopped queue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026123411.1988949-4-nilay@linux.ibm.com?part=2