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

[Christoph Hellwig <hch@xxxxxx>]

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);

                        
}