On Wed, Sep 09, 2026 at 01:04:03PM +0200, Christian König wrote: > Since AMDs mail servers blocked me from sending this out, here once more from > my gmail account. > > Regards, > Christian. > > On 9/4/26 15:00, Christian König wrote: > > On 9/4/26 12:44, Thierry Reding wrote: > >> From: Thierry Reding <[email protected]> > >> > >> Add a callback to struct dma_heap_ops that heap providers can implement > >> to show information about the state of the heap in debugfs. A top-level > >> directory named "dma_heap" is created in debugfs and individual files > >> will be named after the heaps. > >> > >> Reviewed-by: Maxime Ripard <[email protected]> > >> Signed-off-by: Thierry Reding <[email protected]> > > > > One minimal nit pick below, with that taken care of Reviewed-by: Christian > > König <[email protected]>. > > > >> --- > >> Changes in v5: > >> - fix failure handling during debugfs root directory creation > >> - add more cleanup to the newly introduced dma_heap_exit() > >> --- > >> drivers/dma-buf/dma-heap.c | 52 > >> ++++++++++++++++++++++++++++++++++++++++++++++ > >> include/linux/dma-heap.h | 2 ++ > >> 2 files changed, 54 insertions(+) > >> > >> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > >> index 8a6c2b6e0cc6..65c80dac6500 100644 > >> --- a/drivers/dma-buf/dma-heap.c > >> +++ b/drivers/dma-buf/dma-heap.c > >> @@ -7,6 +7,7 @@ > >> */ > >> > >> #include <linux/cdev.h> > >> +#include <linux/debugfs.h> > >> #include <linux/device.h> > >> #include <linux/dma-buf.h> > >> #include <linux/dma-heap.h> > >> @@ -225,6 +226,40 @@ const char *dma_heap_get_name(struct dma_heap *heap) > >> } > >> EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP"); > >> > >> +#ifdef CONFIG_DEBUG_FS > >> +static int dma_heap_debug_show(struct seq_file *s, void *unused) > >> +{ > >> + struct dma_heap *heap = s->private; > >> + int err = 0; > >> + > >> + if (heap->ops && heap->ops->show) > >> + err = heap->ops->show(s, heap); > >> + > >> + return err; > >> +} > >> +DEFINE_SHOW_ATTRIBUTE(dma_heap_debug); > >> + > >> +static struct dentry *dma_heap_debugfs_dir; > >> + > >> +static void dma_heap_init_debugfs(void) > >> +{ > >> + dma_heap_debugfs_dir = debugfs_create_dir("dma_heap", NULL); > >> +} > >> + > >> +static void dma_heap_exit_debugfs(void) > >> +{ > >> + debugfs_remove_recursive(dma_heap_debugfs_dir); > >> +} > >> +#else > >> +static void dma_heap_init_debugfs(void) > >> +{ > >> +} > >> + > >> +static void dma_heap_exit_debugfs(void) > >> +{ > >> +} > >> +#endif > >> + > >> /** > >> * dma_heap_add - adds a heap to dmabuf heaps > >> * @exp_info: information needed to register this heap > >> @@ -299,6 +334,13 @@ struct dma_heap *dma_heap_add(const struct > >> dma_heap_export_info *exp_info) > >> > >> /* Add heap to the list */ > >> list_add(&heap->list, &heap_list); > >> + > >> +#ifdef CONFIG_DEBUG_FS > >> + if (heap->ops && heap->ops->show) > >> + debugfs_create_file(heap->name, 0444, dma_heap_debugfs_dir, > >> + heap, &dma_heap_debug_fops); > >> +#endif > >> + > >> mutex_unlock(&heap_list_lock); > >> > >> return heap; > >> @@ -335,6 +377,16 @@ static int dma_heap_init(void) > >> } > >> dma_heap_class->devnode = dma_heap_devnode; > >> > >> + dma_heap_init_debugfs(); > >> + > >> return 0; > >> } > >> subsys_initcall(dma_heap_init); > >> + > >> +static void __exit dma_heap_exit(void) > >> +{ > >> + dma_heap_exit_debugfs(); > >> + class_destroy(dma_heap_class); > >> + unregister_chrdev_region(dma_heap_devt, NUM_HEAP_MINORS); > >> +} > >> +__exitcall(dma_heap_exit); > >> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h > >> index 648328a64b27..1c9bed1f4dde 100644 > >> --- a/include/linux/dma-heap.h > >> +++ b/include/linux/dma-heap.h > >> @@ -12,6 +12,7 @@ > >> #include <linux/types.h> > >> > >> struct dma_heap; > >> +struct seq_file; > >> > >> /** > >> * struct dma_heap_ops - ops to operate on a given heap > >> @@ -24,6 +25,7 @@ struct dma_heap_ops { > >> unsigned long len, > >> u32 fd_flags, > >> u64 heap_flags); > >> + int (*show)(struct seq_file *s, struct dma_heap *heap); > > > > I found it good coding practice for objects ops pointer tables to have the > > object the function works with as first parameter. > > > > Absolutely not a must have, but just looks a little bit more consistent.
Agreed, I generally prefer that too. However, in this case I thought it more important to model this after how pretty much every debugfs file operation works, which is to take the seq_file as first parameter. It's the same order as for the main dma_heap_debug_show() entry point that this is called from, so it's consistent that way. I guess an alternative way to think about it is that in this case the object that we're really working with is the seq_file and the dma_heap is what gets written to that seq_file. Thierry
signature.asc
Description: PGP signature
