[PATCH] staging: ion: Fix ion_cma_heap allocations

[John Stultz]

In trying to add support for drm_hwcomposer to HiKey,
I've needed to utilize the ION CMA heap, and I've noticed
problems with allocations on newer kernels failing.

It seems back with 204f672255c2 ("ion: Use CMA APIs directly"),
the ion_cma_heap code was modified to use the CMA API, but
kept the arguments as buffer lengths rather then number of pages.

This results in errors as we don't have enough pages in CMA to
satisfy the exaggerated requests.

This patch converts the ion_cma_heap CMA API usage to properly
request pages.

It also fixes a minor issue in the allocation where in the error
path, the cma_release is called with the buffer->size value which
hasn't yet been set.

Cc: Laura Abbott 
Cc: Sumit Semwal 
Cc: Benjamin Gaignard 
Cc: Archit Taneja 
Cc: Greg KH 
Cc: Daniel Vetter 
Cc: Dmitry Shmidt 
Cc: Todd Kjos 
Cc: Amit Pundir 
Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
Acked-by: Laura Abbott 
Signed-off-by: John Stultz 
---
 drivers/staging/android/ion/ion_cma_heap.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index dd5545d..86196ff 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -39,9 +39,15 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
struct sg_table *table;
struct page *pages;
+   unsigned long size = PAGE_ALIGN(len);
+   unsigned long nr_pages = size >> PAGE_SHIFT;
+   unsigned long align = get_order(size);
int ret;
 
-   pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);
+   if (align > CONFIG_CMA_ALIGNMENT)
+   align = CONFIG_CMA_ALIGNMENT;
+
+   pages = cma_alloc(cma_heap->cma, nr_pages, align, GFP_KERNEL);
if (!pages)
return -ENOMEM;
 
@@ -53,7 +59,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
if (ret)
goto free_mem;
 
-   sg_set_page(table->sgl, pages, len, 0);
+   sg_set_page(table->sgl, pages, size, 0);
 
buffer->priv_virt = pages;
buffer->sg_table = table;
@@ -62,7 +68,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
 free_mem:
kfree(table);
 err:
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
return -ENOMEM;
 }
 
@@ -70,9 +76,10 @@ static void ion_cma_free(struct ion_buffer *buffer)
 {
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
struct page *pages = buffer->priv_virt;
+   unsigned long nr_pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
 
/* release memory */
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
/* release sg table */
sg_free_table(buffer->sg_table);
kfree(buffer->sg_table);
-- 
2.7.4

[PATCH] staging: ion: Fix ion_cma_heap allocations

[John Stultz]

In trying to add support for drm_hwcomposer to HiKey,
I've needed to utilize the ION CMA heap, and I've noticed
problems with allocations on newer kernels failing.

It seems back with 204f672255c2 ("ion: Use CMA APIs directly"),
the ion_cma_heap code was modified to use the CMA API, but
kept the arguments as buffer lengths rather then number of pages.

This results in errors as we don't have enough pages in CMA to
satisfy the exaggerated requests.

This patch converts the ion_cma_heap CMA API usage to properly
request pages.

It also fixes a minor issue in the allocation where in the error
path, the cma_release is called with the buffer->size value which
hasn't yet been set.

Cc: Laura Abbott 
Cc: Sumit Semwal 
Cc: Benjamin Gaignard 
Cc: Archit Taneja 
Cc: Greg KH 
Cc: Daniel Vetter 
Cc: Dmitry Shmidt 
Cc: Todd Kjos 
Cc: Amit Pundir 
Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
Acked-by: Laura Abbott 
Signed-off-by: John Stultz 
---
 drivers/staging/android/ion/ion_cma_heap.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index dd5545d..86196ff 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -39,9 +39,15 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
struct sg_table *table;
struct page *pages;
+   unsigned long size = PAGE_ALIGN(len);
+   unsigned long nr_pages = size >> PAGE_SHIFT;
+   unsigned long align = get_order(size);
int ret;
 
-   pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);
+   if (align > CONFIG_CMA_ALIGNMENT)
+   align = CONFIG_CMA_ALIGNMENT;
+
+   pages = cma_alloc(cma_heap->cma, nr_pages, align, GFP_KERNEL);
if (!pages)
return -ENOMEM;
 
@@ -53,7 +59,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
if (ret)
goto free_mem;
 
-   sg_set_page(table->sgl, pages, len, 0);
+   sg_set_page(table->sgl, pages, size, 0);
 
buffer->priv_virt = pages;
buffer->sg_table = table;
@@ -62,7 +68,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
 free_mem:
kfree(table);
 err:
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
return -ENOMEM;
 }
 
@@ -70,9 +76,10 @@ static void ion_cma_free(struct ion_buffer *buffer)
 {
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
struct page *pages = buffer->priv_virt;
+   unsigned long nr_pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
 
/* release memory */
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
/* release sg table */
sg_free_table(buffer->sg_table);
kfree(buffer->sg_table);
-- 
2.7.4

Re: [RFC][PATCH] staging: ion: Fix ion_cma_heap allocations

[John Stultz]

On Thu, Dec 7, 2017 at 4:55 PM, Laura Abbott  wrote:
> On 12/07/2017 03:42 PM, John Stultz wrote:
>>
>> In trying to add support for drm_hwcomposer to HiKey,
>> I've needed to utilize the ION CMA heap, and I've noticed
>> problems with allocations on newer kernels failing.
>>
>> It seems back with 204f672255c22 ("ion: Use CMA APIs directly"),
>> the ion_cma_heap code was modified to use the CMA API, but
>> kept the arguments as buffer lengths rather then number of pages.
>>
>> This results in errors as we don't have enough pages in CMA to
>> satisfy the exaggerated requests.
>>
>> This patch converts the ion_cma_heap CMA API usage to properly
>> request pages.
>>
>> It also fixes a minor issue in the allocation where in the error
>> path, the cma_release is called with the buffer->size value which
>> hasn't yet been set.
>>
>> I'm no memory expert so close review would be appreciated!
>>
>
> Yup.
>
> Acked-by: Laura Abbott 

Thanks for that!

I realized right after sending that there's a checkpatch issue to be
fixed up, so I'll resend tomorrow (including your ack) if there aren't
any other objections.

thanks
-john

Re: [RFC][PATCH] staging: ion: Fix ion_cma_heap allocations

[John Stultz]

On Thu, Dec 7, 2017 at 4:55 PM, Laura Abbott  wrote:
> On 12/07/2017 03:42 PM, John Stultz wrote:
>>
>> In trying to add support for drm_hwcomposer to HiKey,
>> I've needed to utilize the ION CMA heap, and I've noticed
>> problems with allocations on newer kernels failing.
>>
>> It seems back with 204f672255c22 ("ion: Use CMA APIs directly"),
>> the ion_cma_heap code was modified to use the CMA API, but
>> kept the arguments as buffer lengths rather then number of pages.
>>
>> This results in errors as we don't have enough pages in CMA to
>> satisfy the exaggerated requests.
>>
>> This patch converts the ion_cma_heap CMA API usage to properly
>> request pages.
>>
>> It also fixes a minor issue in the allocation where in the error
>> path, the cma_release is called with the buffer->size value which
>> hasn't yet been set.
>>
>> I'm no memory expert so close review would be appreciated!
>>
>
> Yup.
>
> Acked-by: Laura Abbott 

Thanks for that!

I realized right after sending that there's a checkpatch issue to be
fixed up, so I'll resend tomorrow (including your ack) if there aren't
any other objections.

thanks
-john

Re: [RFC][PATCH] staging: ion: Fix ion_cma_heap allocations

[Laura Abbott]

On 12/07/2017 03:42 PM, John Stultz wrote:

In trying to add support for drm_hwcomposer to HiKey,
I've needed to utilize the ION CMA heap, and I've noticed
problems with allocations on newer kernels failing.

It seems back with 204f672255c22 ("ion: Use CMA APIs directly"),
the ion_cma_heap code was modified to use the CMA API, but
kept the arguments as buffer lengths rather then number of pages.

This results in errors as we don't have enough pages in CMA to
satisfy the exaggerated requests.

This patch converts the ion_cma_heap CMA API usage to properly
request pages.

It also fixes a minor issue in the allocation where in the error
path, the cma_release is called with the buffer->size value which
hasn't yet been set.

I'm no memory expert so close review would be appreciated!



Yup.

Acked-by: Laura Abbott 


Cc: Laura Abbott 
Cc: Sumit Semwal 
Cc: Benjamin Gaignard 
Cc: Archit Taneja 
Cc: Greg KH 
Cc: Daniel Vetter 
Cc: Dmitry Shmidt 
Cc: Todd Kjos 
Cc: Amit Pundir 
Signed-off-by: John Stultz 
---
  drivers/staging/android/ion/ion_cma_heap.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index dd5545d..b0fb895 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -39,9 +39,15 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
struct sg_table *table;
struct page *pages;
+   unsigned long size = PAGE_ALIGN(len);
+   unsigned long nr_pages = size >> PAGE_SHIFT;
+   unsigned long align = get_order(size);
int ret;
  
-	pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);

+   if (align > CONFIG_CMA_ALIGNMENT)
+align = CONFIG_CMA_ALIGNMENT;
+
+   pages = cma_alloc(cma_heap->cma, nr_pages, align, GFP_KERNEL);
if (!pages)
return -ENOMEM;
  
@@ -53,7 +59,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,

if (ret)
goto free_mem;
  
-	sg_set_page(table->sgl, pages, len, 0);

+   sg_set_page(table->sgl, pages, size, 0);
  
  	buffer->priv_virt = pages;

buffer->sg_table = table;
@@ -62,7 +68,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
  free_mem:
kfree(table);
  err:
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
return -ENOMEM;
  }
  
@@ -70,9 +76,10 @@ static void ion_cma_free(struct ion_buffer *buffer)

  {
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
struct page *pages = buffer->priv_virt;
+   unsigned long nr_pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
  
  	/* release memory */

-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
/* release sg table */
sg_free_table(buffer->sg_table);
kfree(buffer->sg_table);

Re: [RFC][PATCH] staging: ion: Fix ion_cma_heap allocations

[Laura Abbott]

On 12/07/2017 03:42 PM, John Stultz wrote:

In trying to add support for drm_hwcomposer to HiKey,
I've needed to utilize the ION CMA heap, and I've noticed
problems with allocations on newer kernels failing.

It seems back with 204f672255c22 ("ion: Use CMA APIs directly"),
the ion_cma_heap code was modified to use the CMA API, but
kept the arguments as buffer lengths rather then number of pages.

This results in errors as we don't have enough pages in CMA to
satisfy the exaggerated requests.

This patch converts the ion_cma_heap CMA API usage to properly
request pages.

It also fixes a minor issue in the allocation where in the error
path, the cma_release is called with the buffer->size value which
hasn't yet been set.

I'm no memory expert so close review would be appreciated!



Yup.

Acked-by: Laura Abbott 


Cc: Laura Abbott 
Cc: Sumit Semwal 
Cc: Benjamin Gaignard 
Cc: Archit Taneja 
Cc: Greg KH 
Cc: Daniel Vetter 
Cc: Dmitry Shmidt 
Cc: Todd Kjos 
Cc: Amit Pundir 
Signed-off-by: John Stultz 
---
  drivers/staging/android/ion/ion_cma_heap.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index dd5545d..b0fb895 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -39,9 +39,15 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
struct sg_table *table;
struct page *pages;
+   unsigned long size = PAGE_ALIGN(len);
+   unsigned long nr_pages = size >> PAGE_SHIFT;
+   unsigned long align = get_order(size);
int ret;
  
-	pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);

+   if (align > CONFIG_CMA_ALIGNMENT)
+align = CONFIG_CMA_ALIGNMENT;
+
+   pages = cma_alloc(cma_heap->cma, nr_pages, align, GFP_KERNEL);
if (!pages)
return -ENOMEM;
  
@@ -53,7 +59,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,

if (ret)
goto free_mem;
  
-	sg_set_page(table->sgl, pages, len, 0);

+   sg_set_page(table->sgl, pages, size, 0);
  
  	buffer->priv_virt = pages;

buffer->sg_table = table;
@@ -62,7 +68,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
  free_mem:
kfree(table);
  err:
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
return -ENOMEM;
  }
  
@@ -70,9 +76,10 @@ static void ion_cma_free(struct ion_buffer *buffer)

  {
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
struct page *pages = buffer->priv_virt;
+   unsigned long nr_pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
  
  	/* release memory */

-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
/* release sg table */
sg_free_table(buffer->sg_table);
kfree(buffer->sg_table);

[RFC][PATCH] staging: ion: Fix ion_cma_heap allocations

[John Stultz]

In trying to add support for drm_hwcomposer to HiKey,
I've needed to utilize the ION CMA heap, and I've noticed
problems with allocations on newer kernels failing.

It seems back with 204f672255c22 ("ion: Use CMA APIs directly"),
the ion_cma_heap code was modified to use the CMA API, but
kept the arguments as buffer lengths rather then number of pages.

This results in errors as we don't have enough pages in CMA to
satisfy the exaggerated requests.

This patch converts the ion_cma_heap CMA API usage to properly
request pages.

It also fixes a minor issue in the allocation where in the error
path, the cma_release is called with the buffer->size value which
hasn't yet been set.

I'm no memory expert so close review would be appreciated!

Cc: Laura Abbott 
Cc: Sumit Semwal 
Cc: Benjamin Gaignard 
Cc: Archit Taneja 
Cc: Greg KH 
Cc: Daniel Vetter 
Cc: Dmitry Shmidt 
Cc: Todd Kjos 
Cc: Amit Pundir 
Signed-off-by: John Stultz 
---
 drivers/staging/android/ion/ion_cma_heap.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index dd5545d..b0fb895 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -39,9 +39,15 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
struct sg_table *table;
struct page *pages;
+   unsigned long size = PAGE_ALIGN(len);
+   unsigned long nr_pages = size >> PAGE_SHIFT;
+   unsigned long align = get_order(size);
int ret;
 
-   pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);
+   if (align > CONFIG_CMA_ALIGNMENT)
+align = CONFIG_CMA_ALIGNMENT;
+
+   pages = cma_alloc(cma_heap->cma, nr_pages, align, GFP_KERNEL);
if (!pages)
return -ENOMEM;
 
@@ -53,7 +59,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
if (ret)
goto free_mem;
 
-   sg_set_page(table->sgl, pages, len, 0);
+   sg_set_page(table->sgl, pages, size, 0);
 
buffer->priv_virt = pages;
buffer->sg_table = table;
@@ -62,7 +68,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
 free_mem:
kfree(table);
 err:
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
return -ENOMEM;
 }
 
@@ -70,9 +76,10 @@ static void ion_cma_free(struct ion_buffer *buffer)
 {
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
struct page *pages = buffer->priv_virt;
+   unsigned long nr_pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
 
/* release memory */
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
/* release sg table */
sg_free_table(buffer->sg_table);
kfree(buffer->sg_table);
-- 
2.7.4

[RFC][PATCH] staging: ion: Fix ion_cma_heap allocations

[John Stultz]

In trying to add support for drm_hwcomposer to HiKey,
I've needed to utilize the ION CMA heap, and I've noticed
problems with allocations on newer kernels failing.

It seems back with 204f672255c22 ("ion: Use CMA APIs directly"),
the ion_cma_heap code was modified to use the CMA API, but
kept the arguments as buffer lengths rather then number of pages.

This results in errors as we don't have enough pages in CMA to
satisfy the exaggerated requests.

This patch converts the ion_cma_heap CMA API usage to properly
request pages.

It also fixes a minor issue in the allocation where in the error
path, the cma_release is called with the buffer->size value which
hasn't yet been set.

I'm no memory expert so close review would be appreciated!

Cc: Laura Abbott 
Cc: Sumit Semwal 
Cc: Benjamin Gaignard 
Cc: Archit Taneja 
Cc: Greg KH 
Cc: Daniel Vetter 
Cc: Dmitry Shmidt 
Cc: Todd Kjos 
Cc: Amit Pundir 
Signed-off-by: John Stultz 
---
 drivers/staging/android/ion/ion_cma_heap.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index dd5545d..b0fb895 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -39,9 +39,15 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
struct sg_table *table;
struct page *pages;
+   unsigned long size = PAGE_ALIGN(len);
+   unsigned long nr_pages = size >> PAGE_SHIFT;
+   unsigned long align = get_order(size);
int ret;
 
-   pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);
+   if (align > CONFIG_CMA_ALIGNMENT)
+align = CONFIG_CMA_ALIGNMENT;
+
+   pages = cma_alloc(cma_heap->cma, nr_pages, align, GFP_KERNEL);
if (!pages)
return -ENOMEM;
 
@@ -53,7 +59,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
if (ret)
goto free_mem;
 
-   sg_set_page(table->sgl, pages, len, 0);
+   sg_set_page(table->sgl, pages, size, 0);
 
buffer->priv_virt = pages;
buffer->sg_table = table;
@@ -62,7 +68,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
 free_mem:
kfree(table);
 err:
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
return -ENOMEM;
 }
 
@@ -70,9 +76,10 @@ static void ion_cma_free(struct ion_buffer *buffer)
 {
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
struct page *pages = buffer->priv_virt;
+   unsigned long nr_pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
 
/* release memory */
-   cma_release(cma_heap->cma, pages, buffer->size);
+   cma_release(cma_heap->cma, pages, nr_pages);
/* release sg table */
sg_free_table(buffer->sg_table);
kfree(buffer->sg_table);
-- 
2.7.4