On Tue, Dec 1, 2020 at 4:13 PM Minchan Kim minchan@kernel.org wrote:
On Tue, Dec 01, 2020 at 03:38:14PM -0800, John Stultz wrote:
On Tue, Dec 1, 2020 at 2:55 PM Minchan Kim minchan@kernel.org wrote:
On Tue, Dec 01, 2020 at 11:48:15AM -0800, John Stultz wrote:
On Tue, Dec 1, 2020 at 9:51 AM Minchan Kim minchan@kernel.org wrote:
Thanks for reworking and resending this!
...
+static int __init chunk_heap_init(void) +{
struct cma *default_cma = dev_get_cma_area(NULL);struct dma_heap_export_info exp_info;struct chunk_heap *chunk_heap;if (!default_cma)return 0;chunk_heap = kzalloc(sizeof(*chunk_heap), GFP_KERNEL);if (!chunk_heap)return -ENOMEM;chunk_heap->order = CHUNK_HEAP_ORDER;chunk_heap->cma = default_cma;exp_info.name = cma_get_name(default_cma);So, this would create a chunk heap name with the default CMA name, which would be indistinguishable from the heap name used for the plain CMA heap.
Probably a good idea to prefix it with "chunk-" so the heap device names are unique?
That will give an impression to user that they are using different CMA area but that's not true. IMHO, let's be honest at this moment.
I disagree. The dmabuf heaps provide an abstraction for allocating a type of memory, and while your heap is pulling from CMA, you aren't "just" allocating CMA as the existing CMA heap would suffice for that.
Since you need a slightly different method to allocate high order pages in bulk, we really should have a unique way to name the allocator interface. That's why I'd suggest the "chunk-" prefix to the heap name.
Got it. How about this?
diff --git a/drivers/dma-buf/heaps/chunk_heap.c b/drivers/dma-buf/heaps/chunk_heap.c index 0277707a93a9..36e189d0b73d 100644 --- a/drivers/dma-buf/heaps/chunk_heap.c +++ b/drivers/dma-buf/heaps/chunk_heap.c @@ -410,7 +410,7 @@ static int __init chunk_heap_init(void) chunk_heap->order = CHUNK_HEAP_ORDER; chunk_heap->cma = default_cma;
exp_info.name = cma_get_name(default_cma);
exp_info.name = "cma-chunk-heap";
That's still a bit general for the default cma (which can be named differently). I think including cma name is important, just adding the chunk prefix might be best.
So something like sprintf(buf, "chunk-%s", cma_get_name(default_cma)); exp_info.name = buf;
thanks -john