Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752291AbdLHAzh (ORCPT ); Thu, 7 Dec 2017 19:55:37 -0500 Received: from mail-oi0-f65.google.com ([209.85.218.65]:35420 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751010AbdLHAzg (ORCPT ); Thu, 7 Dec 2017 19:55:36 -0500 X-Google-Smtp-Source: AGs4zManO5isWP+rzqXLG3kMG2Cu2iA78euBqt6oFt5zPsAMONHR/ByP6gk8SbqQY4+UCrRyG7tBDw== Subject: Re: [RFC][PATCH] staging: ion: Fix ion_cma_heap allocations To: John Stultz , linux-kernel@vger.kernel.org Cc: Sumit Semwal , Benjamin Gaignard , Archit Taneja , Greg KH , Daniel Vetter , Dmitry Shmidt , Todd Kjos , Amit Pundir References: <1512690169-27217-1-git-send-email-john.stultz@linaro.org> From: Laura Abbott Message-ID: Date: Thu, 7 Dec 2017 16:55:32 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1512690169-27217-1-git-send-email-john.stultz@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3369 Lines: 92 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); >