Re: [PATCH v7 02/15] swiotlb: Refactor swiotlb_create_debugfs

Christoph Hellwig
2021-05-27T13:24:58Z

On Tue, May 18, 2021 at 02:42:02PM +0800, Claire Chang wrote:
> struct io_tlb_mem *io_tlb_default_mem;
> +static struct dentry *debugfs_dir;
>
> /*
> * Max segment that we can provide which (if pages are contingous) will
> @@ -662,18 +663,30 @@ EXPORT_SYMBOL_GPL(is_swiotlb_active);
>
> #ifdef CONFIG_DEBUG_FS
>
> +static void swiotlb_create_debugfs(struct io_tlb_mem *mem, const char *name)
> {
> if (!mem)
> + return;

I don't think this check makes much sense here.

> +}
> +
> +static int __init swiotlb_create_default_debugfs(void)
> +{
> + struct io_tlb_mem *mem = io_tlb_default_mem;
> +
> + if (mem) {
> + swiotlb_create_debugfs(mem, "swiotlb");
> + debugfs_dir = mem->debugfs;
> + } else {
> + debugfs_dir = debugfs_create_dir("swiotlb", NULL);
> + }

This also looks rather strange. I'd much rather create move the
directory creation of out swiotlb_create_debugfs. E.g. something like:

static void swiotlb_create_debugfs_file(struct io_tlb_mem *mem)
{
debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, &mem->nslabs);
debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, &mem->used);
}

static int __init swiotlb_init_debugfs(void)
{
debugfs_dir = debugfs_create_dir("swiotlb", NULL);
if (io_tlb_default_mem) {
io_tlb_default_mem->debugfs = debugfs_dir;
swiotlb_create_debugfs_files(io_tlb_default_mem);
}
return 0;
}
late_initcall(swiotlb_init_debugfs);

...

static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
struct device *dev)
{
...
mem->debugfs = debugfs_create_dir(rmem->name, debugfs_dir);
swiotlb_create_debugfs_files(mem->debugfs);

}