[RFC PATCHv3 5/6] nvme: add generic debugfs support

[Nilay Shroff]

On 10/27/25 5:25 PM, Hannes Reinecke wrote:
> On 10/27/25 10:29, Nilay Shroff wrote:
>> Add generic infrastructure for creating and managing debugfs files in
>> the NVMe module. This introduces 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-NS and
>> per-NS-head statistics, while supporting both simple show callbacks and
>> full seq_operations.
>>
>> Reviewed-by: Hannes Reinecke <hare at suse.de>
>> Signed-off-by: Nilay Shroff <nilay at linux.ibm.com>
>> ---
>>   drivers/nvme/host/Makefile  |   2 +-
>>   drivers/nvme/host/debugfs.c | 122 ++++++++++++++++++++++++++++++++++++
>>   drivers/nvme/host/nvme.h    |  11 ++++
>>   3 files changed, 134 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/nvme/host/debugfs.c
>>
>> diff --git a/drivers/nvme/host/Makefile b/drivers/nvme/host/Makefile
>> index 6414ec968f99..7962dfc3b2ad 100644
>> --- a/drivers/nvme/host/Makefile
>> +++ b/drivers/nvme/host/Makefile
>> @@ -10,7 +10,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..5c441779554f
>> --- /dev/null
>> +++ b/drivers/nvme/host/debugfs.c
>> @@ -0,0 +1,122 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2025 IBM Corporation
>> + *    Nilay Shroff <nilay at linux.ibm.com>
>> + */
>> +
>> +#include <linux/debugfs.h>
>> +#include <linux/seq_file.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;
>> +};
>> +
>> +struct nvme_debugfs_ctx {
>> +    void *data;
>> +    struct nvme_debugfs_attr *attr;
>> +    int srcu_idx;
>> +};
>> +
>> +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;
>> +
>> +    ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> +    if (WARN_ON_ONCE(!ctx))
>> +        return -ENOMEM;
>> +
>> +    ctx->data = data;
>> +    ctx->attr = attr;
>> +
>> +    if (attr->seq_ops) {
>> +        ret = seq_open(file, attr->seq_ops);
>> +        if (ret) {
>> +            kfree(ctx);
>> +            return ret;
>> +        }
>> +        m = file->private_data;
>> +        m->private = ctx;
>> +        return ret;
>> +    }
>> +
>> +    if (WARN_ON_ONCE(!attr->show)) {
>> +        kfree(ctx);
>> +        return -EPERM;
>> +    }
>> +
>> +    return single_open(file, nvme_debugfs_show, ctx);
>> +}
>> +
>> +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);
>> +
>> +    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_mpath_debugfs_attrs[] = {
>> +    {},
>> +};
>> +
>> +static const struct nvme_debugfs_attr nvme_ns_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++)
>> +        debugfs_create_file_aux(attr->name, attr->mode, q->debugfs_dir,
>> +                data, (void *)attr, &nvme_debugfs_fops);
>> +}
>> +
>> +void nvme_debugfs_register(struct gendisk *disk)
>> +{
>> +    const struct nvme_debugfs_attr *attr;
>> +
>> +    if (nvme_disk_is_ns_head(disk))
>> +        attr = nvme_mpath_debugfs_attrs;
>> +    else
>> +        attr = nvme_ns_debugfs_attrs;
>> +
>> +    nvme_debugfs_create_files(disk->queue, attr, disk->private_data);
>> +}
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 2e58d4d6902a..d049b62b71d2 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -988,6 +988,7 @@ extern const struct attribute_group *nvme_dev_attr_groups[];
>>   extern const struct block_device_operations nvme_bdev_ops;
>>     void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
>> +
>>   struct nvme_ns *nvme_find_path(struct nvme_ns_head *head, unsigned int op_type);
>>   static inline int nvme_data_dir(const enum req_op op)
>>   {
>> @@ -999,6 +1000,16 @@ static inline int nvme_data_dir(const enum req_op op)
>>           return NVME_STAT_OTHER;
>>   }
>>   +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.
>> +     */
>> +}
>> +
>>   #ifdef CONFIG_NVME_MULTIPATH
>>   static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
>>   {
> 
> Especially as you already add a debugfs entry, so moving
> the values from the previous two patches should be easy.
> 
Okay will move those two entries under debugfs.

Thanks,
--Nilay