* [PATCH v3 0/2] nvme-tcp: NIC topology aware I/O queue scaling and queue info export
@ 2026-10-26 12:33 Nilay Shroff
2026-10-26 12:33 ` [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers Nilay Shroff
2026-10-26 12:33 ` [PATCH v3 2/2] nvme: expose queue information via debugfs Nilay Shroff
0 siblings, 2 replies; 5+ messages in thread
From: Nilay Shroff @ 2026-10-26 12:33 UTC (permalink / raw)
To: kbusch, dwagner, hch, hare, sagi, chaitanyak, gjoyce; +Cc: linux-nvme
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers 2026-10-26 12:33 [PATCH v3 0/2] nvme-tcp: NIC topology aware I/O queue scaling and queue info export Nilay Shroff @ 2026-10-26 12:33 ` Nilay Shroff 2026-10-26 12:52 ` sashiko-bot 2026-10-26 12:33 ` [PATCH v3 2/2] nvme: expose queue information via debugfs Nilay Shroff 1 sibling, 1 reply; 5+ messages in thread From: Nilay Shroff @ 2026-10-26 12:33 UTC (permalink / raw) To: kbusch, dwagner, hch, hare, sagi, chaitanyak, gjoyce; +Cc: linux-nvme 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers 2026-10-26 12:33 ` [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers Nilay Shroff @ 2026-10-26 12:52 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-10-26 12:52 UTC (permalink / raw) To: Nilay Shroff; +Cc: linux-nvme 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] nvme: expose queue information via debugfs 2026-10-26 12:33 [PATCH v3 0/2] nvme-tcp: NIC topology aware I/O queue scaling and queue info export Nilay Shroff 2026-10-26 12:33 ` [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers Nilay Shroff @ 2026-10-26 12:33 ` Nilay Shroff 2026-10-26 12:55 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: Nilay Shroff @ 2026-10-26 12:33 UTC (permalink / raw) To: kbusch, dwagner, hch, hare, sagi, chaitanyak, gjoyce; +Cc: linux-nvme 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] nvme: expose queue information via debugfs 2026-10-26 12:33 ` [PATCH v3 2/2] nvme: expose queue information via debugfs Nilay Shroff @ 2026-10-26 12:55 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-10-26 12:55 UTC (permalink / raw) To: Nilay Shroff; +Cc: linux-nvme 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-26 12:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-26 12:33 [PATCH v3 0/2] nvme-tcp: NIC topology aware I/O queue scaling and queue info export Nilay Shroff 2026-10-26 12:33 ` [PATCH v3 1/2] nvme: add debugfs helpers for NVMe drivers Nilay Shroff 2026-10-26 12:52 ` sashiko-bot 2026-10-26 12:33 ` [PATCH v3 2/2] nvme: expose queue information via debugfs Nilay Shroff 2026-10-26 12:55 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox