Chunk Heap Support on DMA-HEAP

The upcoming patch will introduce __GFP_NORETRY semantic in alloc_contig_range which is a failfast mode of the API. Instead of adding a additional parameter for gfp, replace no_warn with gfp flag.

To keep old behaviors, it follows the rule below.

no_warn gfp_flags

  false                         GFP_KERNEL
  true                          GFP_KERNEL|__GFP_NOWARN
  gfp & __GFP_NOWARN            GFP_KERNEL | (gfp & __GFP_NOWARN)

Signed-off-by Minchan Kim <[email protected]>

---
 drivers/dma-buf/heaps/cma_heap.c |  2 +-
 drivers/s390/char/vmcp.c         |  2 +-
 include/linux/cma.h              |  2 +-
 kernel/dma/contiguous.c          |  3 ++-
 mm/cma.c                         | 12 ++++++------
 mm/cma_debug.c                   |  2 +-
 mm/hugetlb.c                     |  6 ++++--
 mm/secretmem.c                   |  3 ++-
 8 files changed, 18 insertions(+), 14 deletions(-)

drivers/dma-buf/heaps/cma_heap.c +1 -1

index 364fc2f3e499..0afc1907887a 100644
@@ -298,7 +298,7 @@ static int cma_heap_allocate(struct dma_heap *heap,
        if (align > CONFIG_CMA_ALIGNMENT)
                align = CONFIG_CMA_ALIGNMENT;
 
-       cma_pages = cma_alloc(cma_heap->cma, pagecount, align, false);
+       cma_pages = cma_alloc(cma_heap->cma, pagecount, align, GFP_KERNEL);
        if (!cma_pages)
                goto free_buffer;
 

drivers/s390/char/vmcp.c +1 -1

index 9e066281e2d0..78f9adf56456 100644
@@ -70,7 +70,7 @@ static void vmcp_response_alloc(struct vmcp_session *session)
         * anymore the system won't work anyway.
         */
        if (order > 2)
-               page = cma_alloc(vmcp_cma, nr_pages, 0, false);
+               page = cma_alloc(vmcp_cma, nr_pages, 0, GFP_KERNEL);
        if (page) {
                session->response = (char *)page_to_phys(page);
                session->cma_alloc = 1;

include/linux/cma.h +1 -1

index 217999c8a762..d6c02d08ddbc 100644
@@ -45,7 +45,7 @@ extern int cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
                                        const char *name,
                                        struct cma **res_cma);
 extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
-                             bool no_warn);
+                             gfp_t gfp_mask);
 extern bool cma_release(struct cma *cma, const struct page *pages, unsigned int count);
 
 extern int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data);

kernel/dma/contiguous.c +2 -1

index 3d63d91cba5c..552ed531c018 100644
@@ -260,7 +260,8 @@ struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
        if (align > CONFIG_CMA_ALIGNMENT)
                align = CONFIG_CMA_ALIGNMENT;
 
-       return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
+       return cma_alloc(dev_get_cma_area(dev), count, align, GFP_KERNEL |
+                       (no_warn ? __GFP_NOWARN : 0));
 }
 
 /**

mm/cma.c +6 -6

index 0ba69cd16aeb..35053b82aedc 100644
@@ -419,13 +419,13 @@ static inline void cma_debug_show_areas(struct cma *cma) { }
  * @cma:   Contiguous memory region for which the allocation is performed.
  * @count: Requested number of pages.
  * @align: Requested alignment of pages (in PAGE_SIZE order).
- * @no_warn: Avoid printing message about failed allocation
+ * @gfp_mask: GFP mask to use during during the cma allocation.
  *
  * This function allocates part of contiguous memory on specific
  * contiguous memory area.
  */
 struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
-                      bool no_warn)
+                      gfp_t gfp_mask)
 {
        unsigned long mask, offset;
        unsigned long pfn = -1;

@@ -438,8 +438,8 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
        if (!cma || !cma->count || !cma->bitmap)
                return NULL;
 
-       pr_debug("%s(cma %p, count %zu, align %d)\n", __func__, (void *)cma,
-                count, align);
+       pr_debug("%s(cma %p, count %zu, align %d gfp_mask 0x%x)\n", __func__,
+                       (void *)cma, count, align, gfp_mask);
 
        if (!count)
                return NULL;

@@ -471,7 +471,7 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
 
                pfn = cma->base_pfn + (bitmap_no << cma->order_per_bit);
                ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA,
-                                    GFP_KERNEL | (no_warn ? __GFP_NOWARN : 0));
+                                               gfp_mask);
 
                if (ret == 0) {
                        page = pfn_to_page(pfn);

@@ -500,7 +500,7 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
                        page_kasan_tag_reset(page + i);
        }
 
-       if (ret && !no_warn) {
+       if (ret && !(gfp_mask & __GFP_NOWARN)) {
                pr_err("%s: alloc failed, req-size: %zu pages, ret: %d\n",
                        __func__, count, ret);
                cma_debug_show_areas(cma);

mm/cma_debug.c +1 -1

index d5bf8aa34fdc..00170c41cf81 100644
@@ -137,7 +137,7 @@ static int cma_alloc_mem(struct cma *cma, int count)
        if (!mem)
                return -ENOMEM;
 
-       p = cma_alloc(cma, count, 0, false);
+       p = cma_alloc(cma, count, 0, GFP_KERNEL);
        if (!p) {
                kfree(mem);
                return -ENOMEM;

mm/hugetlb.c +4 -2

index 737b2dce19e6..695af33aa66c 100644
@@ -1266,7 +1266,8 @@ static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
 
                if (hugetlb_cma[nid]) {
                        page = cma_alloc(hugetlb_cma[nid], nr_pages,
-                                       huge_page_order(h), true);
+                                       huge_page_order(h),
+                                       GFP_KERNEL | __GFP_NOWARN);
                        if (page)
                                return page;
                }

@@ -1277,7 +1278,8 @@ static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
                                        continue;
 
                                page = cma_alloc(hugetlb_cma[node], nr_pages,
-                                               huge_page_order(h), true);
+                                               huge_page_order(h),
+                                               GFP_KERNEL | __GFP_NOWARN);
                                if (page)
                                        return page;
                        }

mm/secretmem.c +2 -2

index b8a32954ac68..585d55b9f9d8 100644
@@ -86,7 +86,8 @@ static int secretmem_pool_increase(struct secretmem_ctx *ctx, gfp_t gfp)
        struct page *page;
        int err;
 
-       page = cma_alloc(secretmem_cma, nr_pages, PMD_SIZE, gfp & __GFP_NOWARN);
+       page = cma_alloc(secretmem_cma, nr_pages, PMD_SIZE,
+                               GFP_KERNEL | (gfp & __GFP_NOWARN));
        if (!page)
                return -ENOMEM;
 
--

2.30.0.284.gd98b1dd5eaa7-goog

Contiguous memory allocation can be stalled due to waiting on page writeback and/or page lock which causes unpredictable delay. It's a unavoidable cost for the requestor to get *big* contiguous memory but it's expensive for *small* contiguous memory(e.g., order-4) because caller could retry the request in diffrent range where would have easy migratable pages without stalling.

This patch introduce __GFP_NORETRY as compaction gfp_mask in alloc_contig_range so it will fail fast without blocking when it encounters pages needed waitting.

Signed-off-by Minchan Kim <[email protected]>

---
 mm/page_alloc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

mm/page_alloc.c +5 -1

index 5b3923db9158..ff41ceb4db51 100644
@@ -8489,12 +8489,16 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 	unsigned int nr_reclaimed;
 	unsigned long pfn = start;
 	unsigned int tries = 0;
+	unsigned int max_tries = 5;
 	int ret = 0;
 	struct migration_target_control mtc = {
 		.nid = zone_to_nid(cc->zone),
 		.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
 	};
 
+	if (cc->alloc_contig && cc->mode == MIGRATE_ASYNC)
+		max_tries = 1;
+
 	migrate_prep();
 
 	while (pfn < end || !list_empty(&cc->migratepages)) {

@@ -8511,7 +8515,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 				break;
 			}
 			tries = 0;
-		} else if (++tries == 5) {
+		} else if (++tries == max_tries) {
 			ret = ret < 0 ? ret : -EBUSY;
 			break;
 		}

Quoted 7 lines · Minchan Kim

Contiguous memory allocation can be stalled due to waiting on page writeback and/or page lock which causes unpredictable delay. It's a unavoidable cost for the requestor to get *big* contiguous memory but it's expensive for *small* contiguous memory(e.g., order-4) because caller could retry the request in diffrent range where would have easy migratable pages without stalling.

s/diffrent/different/

Quoted 4 lines

This patch introduce __GFP_NORETRY as compaction gfp_mask in alloc_contig_range so it will fail fast without blocking when it encounters pages needed waitting.

s/waitting/waiting/

Quoted 39 lines

Signed-off-by Minchan Kim <[email protected]>

---
 mm/page_alloc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

mm/page_alloc.c +5 -1

index 5b3923db9158..ff41ceb4db51 100644
@@ -8489,12 +8489,16 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 	unsigned int nr_reclaimed;
 	unsigned long pfn = start;
 	unsigned int tries = 0;
+	unsigned int max_tries = 5;
 	int ret = 0;
 	struct migration_target_control mtc = {
 		.nid = zone_to_nid(cc->zone),
 		.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
 	};
 
+	if (cc->alloc_contig && cc->mode == MIGRATE_ASYNC)
+		max_tries = 1;
+
 	migrate_prep();
 
 	while (pfn < end || !list_empty(&cc->migratepages)) {

@@ -8511,7 +8515,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 				break;
 			}
 			tries = 0;
-		} else if (++tries == 5) {
+		} else if (++tries == max_tries) {
 			ret = ret < 0 ? ret : -EBUSY;
 			break;
 		}

mm/page_alloc.c int alloc_contig_range(unsigned long start, unsigned long end, · line 8572

Quoted 8 lines

mm/page_alloc.c +1 -1

@@ -8562,7 +8566,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 		.nr_migratepages = 0,
 		.order = -1,
 		.zone = page_zone(pfn_to_page(start)),
-		.mode = MIGRATE_SYNC,
+		.mode = gfp_mask & __GFP_NORETRY ? MIGRATE_ASYNC : MIGRATE_SYNC,
 		.ignore_skip_hint = true,
 		.no_set_skip_hint = true,
 		.gfp_mask = current_gfp_context(gfp_mask),

I'm fine with using gfp flags (e.g., __GFP_NORETRY) as long as they don't enable other implicit behavior (e.g., move draining X to the caller) that's hard to get from the flag name.

IMHO, if we ever want to move draining to the caller, or change the behavior of alloc_contig_range() in different ways (e.g., disable PCP), we won't get around introducing a separate set of flags for alloc_contig_range().

Let's see what Michal thinks. Thanks!

Signature · 3 lines

Thanks,

David / dhildenb

From: Hyesoo Yu <[email protected]>

Document devicetree binding for chunk cma heap on dma heap framework.

The DMA chunk heap supports the bulk allocation of higher order pages.

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Change-Id I8fb231e5a8360e2d8f65947e155b12aa664dde01

---
 .../reserved-memory/dma_heap_chunk.yaml       | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml

Quoted 6 lines · Minchan Kim

From: Hyesoo Yu <[email protected]>

Document devicetree binding for chunk cma heap on dma heap framework.

The DMA chunk heap supports the bulk allocation of higher order pages.

Quoted 9 lines

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Change-Id I8fb231e5a8360e2d8f65947e155b12aa664dde01

---
 .../reserved-memory/dma_heap_chunk.yaml       | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml:58:1: [warning] too many blank lines (2 > 1) (empty-lines)

dtschema/dtc warnings/errors:

See https://patchwork.ozlabs.org/patch/1425577

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

From: Hyesoo Yu <[email protected]>

Document devicetree binding for chunk cma heap on dma heap framework.

The DMA chunk heap supports the bulk allocation of higher order pages.

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Change-Id I8fb231e5a8360e2d8f65947e155b12aa664dde01

---
 .../reserved-memory/dma_heap_chunk.yaml       | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml new file +7 -0

new file mode 100644
index 000000000000..3e7fed5fb006
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/dma_heap_chunk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device tree binding for chunk heap on DMA HEAP FRAMEWORK

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml new file +35 -0

+
+  chunk-order:
+    description: |
+            order of pages that will get allocated from the chunk DMA heap.
+    maxItems: 1
+
+  size:
+    maxItems: 1
+
+  alignment:
+    maxItems: 1
+
+required:
+  - compatible
+  - size
+  - alignment
+  - chunk-order
+
+additionalProperties: false
+
+examples:
+  - |
+    reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <1>;
+
+        chunk_memory: chunk_memory {
+            compatible = "dma_heap,chunk";
+            size = <0x3000000>;
+            alignment = <0x0 0x00010000>;
+            chunk-order = <4>;
+        };
+    };
+
+

Signature · 1 line

2.30.0.284.gd98b1dd5eaa7-goog

Quoted 2 lines · Minchan Kim

From: Hyesoo Yu <[email protected]>

Quoted 3 lines

Document devicetree binding for chunk cma heap on dma heap framework.

The DMA chunk heap supports the bulk allocation of higher order pages.

Why do we need this? What does this do that CMA doesn't?

With a CMA area I can believe a carve out is a common, OS independent thing. This looks too closely tied to some Linux thing to go into DT.

Quoted 6 lines

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Change-Id I8fb231e5a8360e2d8f65947e155b12aa664dde01

Drop this.

Quoted 18 lines

---
 .../reserved-memory/dma_heap_chunk.yaml       | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml new file +7 -0

new file mode 100644
index 000000000000..3e7fed5fb006
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/dma_heap_chunk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device tree binding for chunk heap on DMA HEAP FRAMEWORK

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml line 17

Quoted 10 lines

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml new file +10 -0

+
+description: |
+  The DMA chunk heap is backed by the Contiguous Memory Allocator (CMA) and
+  supports bulk allocation of fixed size pages.
+
+maintainers:
+  - Hyesoo Yu <[email protected]>
+  - John Stultz <[email protected]>
+  - Minchan Kim <[email protected]>
+  - Hridya Valsaraju<[email protected]>

space ^

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml line 23

Quoted 6 lines

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml new file +6 -0

+
+
+properties:
+  compatible:
+    enum:
+      - dma_heap,chunk

The format is <vendor>,<something> and 'dma_heap' is not a vendor.

Quoted 38 lines

Documentation/devicetree/bindings/reserved-memory/dma_heap_chunk.yaml new file +35 -1

+
+  chunk-order:
+    description: |
+            order of pages that will get allocated from the chunk DMA heap.
+    maxItems: 1
+
+  size:
+    maxItems: 1
+
+  alignment:
+    maxItems: 1
+
+required:
+  - compatible
+  - size
+  - alignment
+  - chunk-order
+
+additionalProperties: false
+
+examples:
+  - |
+    reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <1>;
+
+        chunk_memory: chunk_memory {
+            compatible = "dma_heap,chunk";
+            size = <0x3000000>;
+            alignment = <0x0 0x00010000>;
+            chunk-order = <4>;
+        };
+    };
+
+
--

2.30.0.284.gd98b1dd5eaa7-goog

Hi Minchan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20210112]
[cannot apply to s390/features robh/for-next linux/master linus/master hnaz-linux-mm/master v5.11-rc3 v5.11-rc2 v5.11-rc1 v5.11-rc3] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/Chunk-Heap-Support-on-DMA-HEAP/20210113-092747
base:    df869cab4b3519d603806234861aa0a39df479c0
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/531ebc21d3c2584784d44714e3b4f1df46b80eee
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Minchan-Kim/Chunk-Heap-Support-on-DMA-HEAP/20210113-092747 git checkout 531ebc21d3c2584784d44714e3b4f1df46b80eee
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/dma-buf/heaps/chunk_heap.c: In function 'chunk_heap_do_vmap':
drivers/dma-buf/heaps/chunk_heap.c:215:24: error: implicit declaration of function 'vmalloc'; did you mean 'kvmalloc'? [-Werror=implicit-function-declaration]

     215 |  struct page **pages = vmalloc(sizeof(struct page *) * npages);
         |                        ^~~~~~~
         |                        kvmalloc

Quoted 1 line

Quoted 1 line

drivers/dma-buf/heaps/chunk_heap.c:215:24: warning: initialization of 'struct page **' from 'int' makes pointer from integer without a cast [-Wint-conversion]

drivers/dma-buf/heaps/chunk_heap.c:228:10: error: implicit declaration of function 'vmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration]

     228 |  vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
         |          ^~~~
         |          kmap

drivers/dma-buf/heaps/chunk_heap.c:228:30: error: 'VM_MAP' undeclared (first use in this function); did you mean 'VM_MTE'?

     228 |  vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
         |                              ^~~~~~
         |                              VM_MTE

drivers/dma-buf/heaps/chunk_heap.c:228:30: note: each undeclared identifier is reported only once for each function it appears in drivers/dma-buf/heaps/chunk_heap.c:229:2: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]

     229 |  vfree(pages);
         |  ^~~~~
         |  kvfree
   drivers/dma-buf/heaps/chunk_heap.c: In function 'chunk_heap_vunmap':
   drivers/dma-buf/heaps/chunk_heap.c:268:3: error: implicit declaration of function 'vunmap'; did you mean 'kunmap'? [-Werror=implicit-function-declaration]
     268 |   vunmap(buffer->vaddr);
         |   ^~~~~~
         |   kunmap

cc1: some warnings being treated as errors

vim +215 drivers/dma-buf/heaps/chunk_heap.c

   210	
   211	static void *chunk_heap_do_vmap(struct chunk_heap_buffer *buffer)
   212	{
   213		struct sg_table *table = &buffer->sg_table;
   214		int npages = PAGE_ALIGN(buffer->len) / PAGE_SIZE;

Quoted 1 line

215 struct page **pages = vmalloc(sizeof(struct page *) * npages);

   216		struct page **tmp = pages;
   217		struct sg_page_iter piter;
   218		void *vaddr;
   219	
   220		if (!pages)
   221			return ERR_PTR(-ENOMEM);
   222	
   223		for_each_sgtable_page(table, &piter, 0) {
   224			WARN_ON(tmp - pages >= npages);
   225			*tmp++ = sg_page_iter_page(&piter);
   226		}
   227	
   228		vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
   229		vfree(pages);
   230	
   231		if (!vaddr)
   232			return ERR_PTR(-ENOMEM);
   233	
   234		return vaddr;
   235	}
   236	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

drivers/dma-buf/heaps/Kconfig config DMABUF_HEAPS_CMA · line 22

Quoted 7 lines · Minchan Kim

drivers/dma-buf/heaps/Kconfig +7 -0

+config DMABUF_HEAPS_CHUNK
+	bool "DMA-BUF CHUNK Heap"
+	depends on DMABUF_HEAPS && DMA_CMA
+	help
+	  Choose this option to enable dma-buf CHUNK heap. This heap is backed
+	  by the Contiguous Memory Allocator (CMA) and allocates the buffers that
+	  arranged into a list of fixed size chunks taken from CMA.

maybe:
are arranged into

From: Hyesoo Yu <[email protected]>

This patch supports chunk heap that allocates the buffers that arranged into a list a fixed size chunks taken from CMA.

The chunk heap driver is bound directly to a reserved_memory node by following Rob Herring's suggestion in [1].

[1] https://lore.kernel.org/lkml/[email protected]/T/#m3dc63acd33fea269a584f43bb799a876f0b2b45d

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

From: Hyesoo Yu <[email protected]>

This patch supports chunk heap that allocates the buffers that arranged into a list a fixed size chunks taken from CMA.

The chunk heap driver is bound directly to a reserved_memory node by following Rob Herring's suggestion in [1].

[1] https://lore.kernel.org/lkml/[email protected]/T/#m3dc63acd33fea269a584f43bb799a876f0b2b45d

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---

Quoted 15 lines · Minchan Kim

From: Hyesoo Yu <[email protected]>

This patch supports chunk heap that allocates the buffers that arranged into a list a fixed size chunks taken from CMA.

The chunk heap driver is bound directly to a reserved_memory node by following Rob Herring's suggestion in [1].

[1] https://lore.kernel.org/lkml/[email protected]/T/#m3dc63acd33fea269a584f43bb799a876f0b2b45d

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---

...

drivers/dma-buf/heaps/chunk_heap.c line 410

Quoted 5 lines

drivers/dma-buf/heaps/chunk_heap.c new file +5 -0

+static int register_chunk_heap(struct chunk_heap *chunk_heap_info)
+{
+       struct dma_heap_export_info exp_info;
+
+       exp_info.name = cma_get_name(chunk_heap_info->cma);

One potential issue here, you're setting the name to the same as the CMA name. Since the CMA heap uses the CMA name, if one chunk was registered as a chunk heap but also was the default CMA area, it might be registered twice. But since both would have the same name it would be an initialization race as to which one "wins".

So maybe could you postfix the CMA name with "-chunk" or something?

Quoted 17 lines · John Stultz

Quoted 15 lines · Minchan Kim

From: Hyesoo Yu <[email protected]>

This patch supports chunk heap that allocates the buffers that arranged into a list a fixed size chunks taken from CMA.

The chunk heap driver is bound directly to a reserved_memory node by following Rob Herring's suggestion in [1].

[1] https://lore.kernel.org/lkml/[email protected]/T/#m3dc63acd33fea269a584f43bb799a876f0b2b45d

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---

...

Quoted 11 lines

drivers/dma-buf/heaps/chunk_heap.c line 410

Quoted 5 lines

drivers/dma-buf/heaps/chunk_heap.c new file +5 -0

+static int register_chunk_heap(struct chunk_heap *chunk_heap_info)
+{
+       struct dma_heap_export_info exp_info;
+
+       exp_info.name = cma_get_name(chunk_heap_info->cma);

One potential issue here, you're setting the name to the same as the CMA name. Since the CMA heap uses the CMA name, if one chunk was registered as a chunk heap but also was the default CMA area, it might be registered twice. But since both would have the same name it would be an initialization race as to which one "wins".

Good point. Maybe someone might want to use default CMA area for both cma_heap and chunk_heap. I cannot come up with ideas why we should prohibit it atm.

Quoted 1 line

Quoted 1 line

So maybe could you postfix the CMA name with "-chunk" or something?

Hyesoo, Any opinion?
Unless you have something other idea, let's fix it in next version.

Quoted 8 lines · Minchan Kim

Quoted 7 lines · John Stultz

Quoted 6 lines · Minchan Kim

From: Hyesoo Yu <[email protected]>

This patch supports chunk heap that allocates the buffers that arranged into a list a fixed size chunks taken from CMA.

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

The chunk heap driver is bound directly to a reserved_memory node by following Rob Herring's suggestion in [1].

[1] https://lore.kernel.org/lkml/[email protected]/T/#m3dc63acd33fea269a584f43bb799a876f0b2b45d

Signed-off-by Hyesoo Yu <[email protected]>

Signed-off-by Hridya Valsaraju <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 19 lines

Quoted 13 lines

Quoted 1 line

---

...

drivers/dma-buf/heaps/chunk_heap.c line 410

Quoted 5 lines

drivers/dma-buf/heaps/chunk_heap.c new file +5 -0

+static int register_chunk_heap(struct chunk_heap *chunk_heap_info)
+{
+       struct dma_heap_export_info exp_info;
+
+       exp_info.name = cma_get_name(chunk_heap_info->cma);

One potential issue here, you're setting the name to the same as the CMA name. Since the CMA heap uses the CMA name, if one chunk was registered as a chunk heap but also was the default CMA area, it might be registered twice. But since both would have the same name it would be an initialization race as to which one "wins".

Good point. Maybe someone might want to use default CMA area for both cma_heap and chunk_heap. I cannot come up with ideas why we should prohibit it atm.

Quoted 1 line

Quoted 5 lines

Quoted 1 line

So maybe could you postfix the CMA name with "-chunk" or something?

Hyesoo, Any opinion?
Unless you have something other idea, let's fix it in next version.

I agree that. It is not good to use heap name directly as cma name. Let's postfix the name with '-chunk'

Thanks,
Regards.