On Mon, Jan 23, 2023 at 4:38 AM Christian König ckoenig.leichtzumerken@gmail.com wrote:
This allows device drivers to specify a DMA-heap where they want their buffers to be allocated from. This information is then exposed as sysfs link between the device and the DMA-heap.
Useful for userspace when in need to decide from which provider to allocate a buffer.
Signed-off-by: Christian König christian.koenig@amd.com
Hey Christian! Thanks so much for sending this out and also thanks for including me (Adding TJ as well)!
This looks like a really interesting approach, but I have a few thoughts below.
drivers/dma-buf/dma-heap.c | 41 ++++++++++++++++++++++++++++++-------- include/linux/dma-heap.h | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index c9e41e8a9e27..0f7cf713c22f 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -31,6 +31,7 @@
- @heap_devt heap device node
- @list list head connecting to list of heaps
- @heap_cdev heap char device
*/
- @dev: heap device in sysfs
- Represents a heap of memory from which buffers can be made.
@@ -41,6 +42,7 @@ struct dma_heap { dev_t heap_devt; struct list_head list; struct cdev heap_cdev;
struct device *dev;};
static LIST_HEAD(heap_list); @@ -49,6 +51,33 @@ static dev_t dma_heap_devt; static struct class *dma_heap_class; static DEFINE_XARRAY_ALLOC(dma_heap_minors);
+int dma_heap_create_device_link(struct device *dev, const char *heap) +{
struct dma_heap *h;/* check the name is valid */mutex_lock(&heap_list_lock);list_for_each_entry(h, &heap_list, list) {if (!strcmp(h->name, heap))break;}mutex_unlock(&heap_list_lock);if (list_entry_is_head(h, &heap_list, list)) {pr_err("dma_heap: Link to invalid heap requested %s\n", heap);return -EINVAL;}return sysfs_create_link(&dev->kobj, &h->dev->kobj, DEVNAME);+}
So, one concern with this (if I'm reading this right) is it seems like a single heap link may be insufficient.
There may be multiple heaps that a driver could work with (especially if the device isn't very constrained), so when sharing a buffer with a second device that is more constrained we'd have the same problem we have now where userspace just needs to know which device is the more constrained one to allocate for.
So it might be useful to have a sysfs "dma_heaps" directory with the supported heaps linked from there? That way userland could find across the various devices the heap-link that was common.
This still has the downside that every driver needs to be aware of every heap, and when new heaps are added, it may take awhile before folks might be able to assess if a device is compatible or not to update it, so I suspect we'll have eventually some loose constraint-based helpers to register links. But I think this at least moves us in a workable direction, so again, I'm really glad to see you send this out!
thanks -john