Re: [PATCH] staging: android: ion: fix sys heap pool's gfp_flags

[Chen Feng]

Thanks.

On 2019/2/1 14:59, Qing Xia wrote:
> In the first loop, gfp_flags will be modified to high_order_gfp_flags,
> and there will be no chance to change back to low_order_gfp_flags.
> 
> Fixes: e7f63771 ("ION: Sys_heap: Add cached pool to spead up cached buffer 
> alloc")
> Signed-off-by: Qing Xia 
> ---
>  drivers/staging/android/ion/ion_system_heap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> b/drivers/staging/android/ion/ion_system_heap.c
> index 0383f75..20f2103 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -223,10 +223,10 @@ static void ion_system_heap_destroy_pools(struct 
> ion_page_pool **pools)
>  static int ion_system_heap_create_pools(struct ion_page_pool **pools)
>  {
>   int i;
> - gfp_t gfp_flags = low_order_gfp_flags;
>  
>   for (i = 0; i < NUM_ORDERS; i++) {
>   struct ion_page_pool *pool;
> + gfp_t gfp_flags = low_order_gfp_flags;
>  
>   if (orders[i] > 4)
>   gfp_flags = high_order_gfp_flags;
> 

-- 
Cheers,
- Feng

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Linaro-mm-sig] [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE

[Chen Feng]

Hi Laura,

When we enable kernel v4.4 or newer version on our platform, we meet the issue
of flushing cache without reference device. It seems that this patch set is
a solution. I'm curious the progress of the discussion. Do you have any plan
to fix it in v4.4 and newer kernel verison?

On 2016/9/14 2:41, Laura Abbott wrote:
> On 09/13/2016 08:14 AM, Will Deacon wrote:
>> On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote:
>>> On 09/13/2016 02:19 AM, Will Deacon wrote:
 On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote:
>
> arm64 may need to guarantee the caches are synced. Implement versions of
> the kernel_force_cache API to allow this.
>
> Signed-off-by: Laura Abbott 
> ---
> v3: Switch to calling cache operations directly instead of relying on
> DMA mapping.
> ---
> arch/arm64/include/asm/cacheflush.h |  8 
> arch/arm64/mm/cache.S   | 24 
> arch/arm64/mm/flush.c   | 11 +++
> 3 files changed, 39 insertions(+), 4 deletions(-)

 I'm really hesitant to expose these cache routines as an API solely to
 support a driver sitting in staging/. I appreciate that there's a chicken
 and egg problem here, but we *really* don't want people using these 
 routines
 in preference to the DMA API, and I fear that we'll simply grow a bunch
 more users of these things if we promote it as an API like you're 
 proposing.

 Can the code not be contained under staging/, as part of ion?

>>>
>>> I proposed that in V1 and it was suggested I make it a proper API
>>>
>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html
>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html
>>
>> :/ then I guess we're in disagreement. If ion really needs this stuff
>> (which I don't fully grok), perhaps we should be exposing something at
>> a higher level from the architecture, so it really can't be used for
>> anything other than ion.
> 
> I talked/complained about this at a past plumbers. The gist is that Ion
> ends up acting as a fake DMA layer for clients. It doesn't match nicely
> because clients can allocate both coherent and non-coherent memory.
> Trying to use dma_map doesn't work because a) a device for coherency isn't
> known at allocation time b) it kills performance. Part of the motivation
> for taking this approach is to avoid the need to rework the existing
> Android userspace and keep the existing behavior, as terrible as it
> is. Having Ion out of staging and not actually usable isn't helpful.
> 
> I'll give this all some more thought and hopefully have one or two more
> proposals before Connect/Plumbers.
> 
>>
>> Will
>>
> 
> Thanks,
> Laura
> ___
> Linaro-mm-sig mailing list
> linaro-mm-...@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Linaro-mm-sig] [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE

[Chen Feng]

On 2017/2/22 3:29, Laura Abbott wrote:
> On 02/20/2017 10:05 PM, Chen Feng wrote:
>> Hi Laura,
>>
>> When we enable kernel v4.4 or newer version on our platform, we meet the 
>> issue
>> of flushing cache without reference device. It seems that this patch set is
>> a solution. I'm curious the progress of the discussion. Do you have any plan
>> to fix it in v4.4 and newer kernel verison?
>>
> 
> No, I've abandoned this approach based on feedback. The APIs had too much
> potential for incorrect usage. I'm ripping out the implicit caching in Ion
> and switching it to a model where there should always be a device available.
> 
> What's your use case where you don't have a device structure?
> 
Userspace use ioctl to flush cache for device.

ion_sync_for_device
dma_sync_sg_for_device(NULL, buffer->sg_table->sgl,
 buffer->sg_table->nents, DMA_BIDIRECTIONAL);

And sys-heap when allocate a zero buffer flush zero data to ddr.
alloc_buffer_page
ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
DMA_BIDIRECTIONAL);



> Thanks,
> Laura
> 
>> On 2016/9/14 2:41, Laura Abbott wrote:
>>> On 09/13/2016 08:14 AM, Will Deacon wrote:
>>>> On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote:
>>>>> On 09/13/2016 02:19 AM, Will Deacon wrote:
>>>>>> On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote:
>>>>>>>
>>>>>>> arm64 may need to guarantee the caches are synced. Implement versions of
>>>>>>> the kernel_force_cache API to allow this.
>>>>>>>
>>>>>>> Signed-off-by: Laura Abbott 
>>>>>>> ---
>>>>>>> v3: Switch to calling cache operations directly instead of relying on
>>>>>>> DMA mapping.
>>>>>>> ---
>>>>>>> arch/arm64/include/asm/cacheflush.h |  8 
>>>>>>> arch/arm64/mm/cache.S   | 24 
>>>>>>> arch/arm64/mm/flush.c   | 11 +++
>>>>>>> 3 files changed, 39 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> I'm really hesitant to expose these cache routines as an API solely to
>>>>>> support a driver sitting in staging/. I appreciate that there's a chicken
>>>>>> and egg problem here, but we *really* don't want people using these 
>>>>>> routines
>>>>>> in preference to the DMA API, and I fear that we'll simply grow a bunch
>>>>>> more users of these things if we promote it as an API like you're 
>>>>>> proposing.
>>>>>>
>>>>>> Can the code not be contained under staging/, as part of ion?
>>>>>>
>>>>>
>>>>> I proposed that in V1 and it was suggested I make it a proper API
>>>>>
>>>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html
>>>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html
>>>>
>>>> :/ then I guess we're in disagreement. If ion really needs this stuff
>>>> (which I don't fully grok), perhaps we should be exposing something at
>>>> a higher level from the architecture, so it really can't be used for
>>>> anything other than ion.
>>>
>>> I talked/complained about this at a past plumbers. The gist is that Ion
>>> ends up acting as a fake DMA layer for clients. It doesn't match nicely
>>> because clients can allocate both coherent and non-coherent memory.
>>> Trying to use dma_map doesn't work because a) a device for coherency isn't
>>> known at allocation time b) it kills performance. Part of the motivation
>>> for taking this approach is to avoid the need to rework the existing
>>> Android userspace and keep the existing behavior, as terrible as it
>>> is. Having Ion out of staging and not actually usable isn't helpful.
>>>
>>> I'll give this all some more thought and hopefully have one or two more
>>> proposals before Connect/Plumbers.
>>>
>>>>
>>>> Will
>>>>
>>>
>>> Thanks,
>>> Laura
>>> ___
>>> Linaro-mm-sig mailing list
>>> linaro-mm-...@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
>>
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ion : Donnot wakeup kswapd in ion system alloc

[Chen Feng]

Since ion alloc can be called by userspace,eg gralloc.
When it is called frequently, the efficiency of kswapd is
to low. And the reclaimed memory is too lower. In this way,
the kswapd can use to much cpu resources.

With 3.5GB DMA Zone and 0.5 Normal Zone.

pgsteal_kswapd_dma 9364140
pgsteal_kswapd_normal 7071043
pgscan_kswapd_dma 10428250
pgscan_kswapd_normal 37840094

With this change the reclaim ratio has greatly improved
18.9% -> 72.5%

Signed-off-by: Chen Feng 
Signed-off-by: Lu bing 
---
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index d4c3e55..b69dfc7 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -27,7 +27,7 @@
 #include "ion_priv.h"
 
 static gfp_t high_order_gfp_flags = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN |
-__GFP_NORETRY) & ~__GFP_DIRECT_RECLAIM;
+__GFP_NORETRY) & ~__GFP_RECLAIM;
 static gfp_t low_order_gfp_flags  = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN);
 static const unsigned int orders[] = {8, 4, 0};
 static const int num_orders = ARRAY_SIZE(orders);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] android: lmk: add swap pte pmd in tasksize

[Chen Feng]

On 2016/3/11 15:30, yalin wang wrote:
> 
>> On Mar 11, 2016, at 15:23, Lu Bing  wrote:
>>
>> From: l00215322 
>>
>> Many android devices have zram,so we should add "MM_SWAPENTS" in tasksize.
>> Refer oom_kill.c,we add pte&pmd also.
>>
>> Reviewed-by: Chen Feng 
>> Reviewed-by: Fu Jun 
>> Reviewed-by: Xu YiPing 
>> Reviewed-by: Yu DongBin 
>> Signed-off-by: Lu Bing 
>> ---
>> drivers/staging/android/lowmemorykiller.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/android/lowmemorykiller.c 
>> b/drivers/staging/android/lowmemorykiller.c
>> index 8b5a4a8..0817d3b 100644
>> --- a/drivers/staging/android/lowmemorykiller.c
>> +++ b/drivers/staging/android/lowmemorykiller.c
>> @@ -139,7 +139,9 @@ static unsigned long lowmem_scan(struct shrinker *s, 
>> struct shrink_control *sc)
>>  task_unlock(p);
>>  continue;
>>  }
>> -tasksize = get_mm_rss(p->mm);
>> +tasksize = get_mm_rss(p->mm) +
>> +get_mm_counter(p->mm, MM_SWAPENTS) +
>> +atomic_long_read(&p->mm->nr_ptes) + mm_nr_pmds(p->mm);
> why not introduce a mm_nr_ptes()  help function here ?
> more clear to see .
> 
Since the code is in staging, we can add another patch to do this.


>>  task_unlock(p);
>>  if (tasksize <= 0)
>>  continue;
>> -- 
>> 1.8.3.2
>>
> 
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] android: lmk: add swap pte pmd in tasksize

[Chen Feng]

Hi greg,

On 2016/3/12 1:12, Greg KH wrote:
> On Fri, Mar 11, 2016 at 05:05:45PM +0800, Chen Feng wrote:
>>
>>
>> On 2016/3/11 15:30, yalin wang wrote:
>>>
>>>> On Mar 11, 2016, at 15:23, Lu Bing  wrote:
>>>>
>>>> From: l00215322 
>>>>
>>>> Many android devices have zram,so we should add "MM_SWAPENTS" in tasksize.
>>>> Refer oom_kill.c,we add pte&pmd also.
>>>>
>>>> Reviewed-by: Chen Feng 
>>>> Reviewed-by: Fu Jun 
>>>> Reviewed-by: Xu YiPing 
>>>> Reviewed-by: Yu DongBin 
>>>> Signed-off-by: Lu Bing 
>>>> ---
>>>> drivers/staging/android/lowmemorykiller.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/android/lowmemorykiller.c 
>>>> b/drivers/staging/android/lowmemorykiller.c
>>>> index 8b5a4a8..0817d3b 100644
>>>> --- a/drivers/staging/android/lowmemorykiller.c
>>>> +++ b/drivers/staging/android/lowmemorykiller.c
>>>> @@ -139,7 +139,9 @@ static unsigned long lowmem_scan(struct shrinker *s, 
>>>> struct shrink_control *sc)
>>>>task_unlock(p);
>>>>continue;
>>>>}
>>>> -  tasksize = get_mm_rss(p->mm);
>>>> +  tasksize = get_mm_rss(p->mm) +
>>>> +  get_mm_counter(p->mm, MM_SWAPENTS) +
>>>> +  atomic_long_read(&p->mm->nr_ptes) + mm_nr_pmds(p->mm);
>>> why not introduce a mm_nr_ptes()  help function here ?
>>> more clear to see .
>>>
>> Since the code is in staging, we can add another patch to do this.
> 
> I do not understand this, please explain.
> 
I mean that we can submit another patch to add mm_nr_ptes() in mm-module and 
replace the code used nr_ptes
with this interface(mm_nr_ptes()).

This patch is just change the score of task-size in low memory killer.

> thanks,
> 
> greg k-h
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] ION: Sys_heap: Makes ion buffer always alloc from page pool

[Chen Feng]

Makes the ion buffer always alloced from page pool, no matter
it's cached or not. In this way, it can improve the efficiency
of it.

Currently, there is no difference from cached or non-cached buffer
for the page pool.

Signed-off-by: Chen Feng 
---
 drivers/staging/android/ion/ion_system_heap.c | 19 ++-
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..caf11fc 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -56,24 +56,10 @@ static struct page *alloc_buffer_page(struct 
ion_system_heap *heap,
  struct ion_buffer *buffer,
  unsigned long order)
 {
-   bool cached = ion_buffer_cached(buffer);
struct ion_page_pool *pool = heap->pools[order_to_index(order)];
struct page *page;
 
-   if (!cached) {
-   page = ion_page_pool_alloc(pool);
-   } else {
-   gfp_t gfp_flags = low_order_gfp_flags;
-
-   if (order > 4)
-   gfp_flags = high_order_gfp_flags;
-   page = alloc_pages(gfp_flags | __GFP_COMP, order);
-   if (!page)
-   return NULL;
-   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
-   DMA_BIDIRECTIONAL);
-   }
-
+   page = ion_page_pool_alloc(pool);
return page;
 }
 
@@ -81,9 +67,8 @@ static void free_buffer_page(struct ion_system_heap *heap,
 struct ion_buffer *buffer, struct page *page)
 {
unsigned int order = compound_order(page);
-   bool cached = ion_buffer_cached(buffer);
 
-   if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
+   if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
struct ion_page_pool *pool = heap->pools[order_to_index(order)];
 
ion_page_pool_free(pool, page);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] ION: Sys_heap: Makes ion buffer always alloc from page pool

[Chen Feng]

On 2016/5/6 1:09, Laura Abbott wrote:
> On 05/04/2016 08:27 PM, Chen Feng wrote:
>> Makes the ion buffer always alloced from page pool, no matter
>> it's cached or not. In this way, it can improve the efficiency
>> of it.
>>
>> Currently, there is no difference from cached or non-cached buffer
>> for the page pool.
> 
> 
> The advantage of the uncached pool was that the pages in the pool
> were always clean in the cache. This is lost here with the addition
> of cached pages to the same pool as uncached pages I agree the
> cache path could benefit from pooling but we need to keep the caching
> model consistent.
> 
Yes, the buffer in the pool is non-cached.

I found that the ion don't have a invalid cache ops.
Currently, we use ioctl to keep the cache coherency.
In this way, there is no difference between these.

So, how do you think add a new cached pool in the system heap?
If yes, I can file a new patch to do this.

>>
>> Signed-off-by: Chen Feng 
>> ---
>>  drivers/staging/android/ion/ion_system_heap.c | 19 ++-
>>  1 file changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
>> b/drivers/staging/android/ion/ion_system_heap.c
>> index b69dfc7..caf11fc 100644
>> --- a/drivers/staging/android/ion/ion_system_heap.c
>> +++ b/drivers/staging/android/ion/ion_system_heap.c
>> @@ -56,24 +56,10 @@ static struct page *alloc_buffer_page(struct 
>> ion_system_heap *heap,
>>struct ion_buffer *buffer,
>>unsigned long order)
>>  {
>> -bool cached = ion_buffer_cached(buffer);
>>  struct ion_page_pool *pool = heap->pools[order_to_index(order)];
>>  struct page *page;
>>
>> -if (!cached) {
>> -page = ion_page_pool_alloc(pool);
>> -} else {
>> -gfp_t gfp_flags = low_order_gfp_flags;
>> -
>> -if (order > 4)
>> -gfp_flags = high_order_gfp_flags;
>> -page = alloc_pages(gfp_flags | __GFP_COMP, order);
>> -if (!page)
>> -return NULL;
>> -ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
>> -DMA_BIDIRECTIONAL);
>> -}
>> -
>> +page = ion_page_pool_alloc(pool);
>>  return page;
>>  }
>>
>> @@ -81,9 +67,8 @@ static void free_buffer_page(struct ion_system_heap *heap,
>>   struct ion_buffer *buffer, struct page *page)
>>  {
>>  unsigned int order = compound_order(page);
>> -bool cached = ion_buffer_cached(buffer);
>>
>> -if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
>> +if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
>>  struct ion_page_pool *pool = heap->pools[order_to_index(order)];
>>
>>  ion_page_pool_free(pool, page);
>>
> 
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Add ion cached pool in system heap.

Signed-off-by: Chen Feng 
Signed-off-by: Xia  Qing 
---
 drivers/staging/android/ion/ion_system_heap.c | 120 +-
 1 file changed, 79 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..5b7e5b0 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -49,7 +49,8 @@ static inline unsigned int order_to_size(int order)
 
 struct ion_system_heap {
struct ion_heap heap;
-   struct ion_page_pool *pools[0];
+   struct ion_page_pool *uncached_pools[0];
+   struct ion_page_pool *cached_pools[0];
 };
 
 static struct page *alloc_buffer_page(struct ion_system_heap *heap,
@@ -57,39 +58,36 @@ static struct page *alloc_buffer_page(struct 
ion_system_heap *heap,
  unsigned long order)
 {
bool cached = ion_buffer_cached(buffer);
-   struct ion_page_pool *pool = heap->pools[order_to_index(order)];
+   struct ion_page_pool *pool;
struct page *page;
 
-   if (!cached) {
-   page = ion_page_pool_alloc(pool);
-   } else {
-   gfp_t gfp_flags = low_order_gfp_flags;
-
-   if (order > 4)
-   gfp_flags = high_order_gfp_flags;
-   page = alloc_pages(gfp_flags | __GFP_COMP, order);
-   if (!page)
-   return NULL;
-   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
-   DMA_BIDIRECTIONAL);
-   }
+   if (!cached)
+   pool = heap->uncached_pools[order_to_index(order)];
+   else
+   pool = heap->cached_pools[order_to_index(order)];
 
+   page = ion_page_pool_alloc(pool);
return page;
 }
 
 static void free_buffer_page(struct ion_system_heap *heap,
 struct ion_buffer *buffer, struct page *page)
 {
+   struct ion_page_pool *pool;
unsigned int order = compound_order(page);
bool cached = ion_buffer_cached(buffer);
 
-   if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
-   struct ion_page_pool *pool = heap->pools[order_to_index(order)];
-
-   ion_page_pool_free(pool, page);
-   } else {
+   if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) {
__free_pages(page, order);
+   return;
}
+
+   if (!cached)
+   pool = heap->uncached_pools[order_to_index(order)];
+   else
+   pool = heap->cached_pools[order_to_index(order)];
+
+   ion_page_pool_free(pool, page);
 }
 
 
@@ -224,9 +222,11 @@ static int ion_system_heap_shrink(struct ion_heap *heap, 
gfp_t gfp_mask,
only_scan = 1;
 
for (i = 0; i < num_orders; i++) {
-   struct ion_page_pool *pool = sys_heap->pools[i];
+   struct ion_page_pool *pool = sys_heap->uncached_pools[i];
 
-   nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
+   nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
+   pool = sys_heap->cached_pools[i];
+   nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
nr_total += nr_freed;
 
if (!only_scan) {
@@ -259,27 +259,69 @@ static int ion_system_heap_debug_show(struct ion_heap 
*heap, struct seq_file *s,
struct ion_system_heap,
heap);
int i;
+   struct ion_page_pool *pool;
 
for (i = 0; i < num_orders; i++) {
-   struct ion_page_pool *pool = sys_heap->pools[i];
+   pool = sys_heap->uncached_pools[i];
 
-   seq_printf(s, "%d order %u highmem pages in pool = %lu total\n",
+   seq_printf(s, "%d order %u highmem pages uncached %lu total\n",
   pool->high_count, pool->order,
   (PAGE_SIZE << pool->order) * pool->high_count);
-   seq_printf(s, "%d order %u lowmem pages in pool = %lu total\n",
+   seq_printf(s, "%d order %u lowmem pages uncached %lu total\n",
   pool->low_count, pool->order,
   (PAGE_SIZE << pool->order) * pool->low_count);
}
+
+   for (i = 0; i < num_orders; i++) {
+   pool = sys_heap->cached_pools[i];
+
+   seq_printf(s, "%d order %u highmem pages cached %lu total\n",
+  pool->high_count, pool->order,
+  (PAGE_SIZE << pool->order) * pool->high_count);
+ 

Re: [PATCH] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Sorry,the order here is not right, please ignore this patch.

I will RESEND it.

On 2016/5/9 14:24, Chen Feng wrote:
> Add ion cached pool in system heap.
> 
> Signed-off-by: Chen Feng 
> Signed-off-by: Xia  Qing 
> ---
>  drivers/staging/android/ion/ion_system_heap.c | 120 
> +-
>  1 file changed, 79 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> b/drivers/staging/android/ion/ion_system_heap.c
> index b69dfc7..5b7e5b0 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -49,7 +49,8 @@ static inline unsigned int order_to_size(int order)
>  
>  struct ion_system_heap {
>   struct ion_heap heap;
> - struct ion_page_pool *pools[0];
> + struct ion_page_pool *uncached_pools[0];
> + struct ion_page_pool *cached_pools[0];
>  };
>  
>  static struct page *alloc_buffer_page(struct ion_system_heap *heap,
> @@ -57,39 +58,36 @@ static struct page *alloc_buffer_page(struct 
> ion_system_heap *heap,
> unsigned long order)
>  {
>   bool cached = ion_buffer_cached(buffer);
> - struct ion_page_pool *pool = heap->pools[order_to_index(order)];
> + struct ion_page_pool *pool;
>   struct page *page;
>  
> - if (!cached) {
> - page = ion_page_pool_alloc(pool);
> - } else {
> - gfp_t gfp_flags = low_order_gfp_flags;
> -
> - if (order > 4)
> - gfp_flags = high_order_gfp_flags;
> - page = alloc_pages(gfp_flags | __GFP_COMP, order);
> - if (!page)
> - return NULL;
> - ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
> - DMA_BIDIRECTIONAL);
> - }
> + if (!cached)
> + pool = heap->uncached_pools[order_to_index(order)];
> + else
> + pool = heap->cached_pools[order_to_index(order)];
>  
> + page = ion_page_pool_alloc(pool);
>   return page;
>  }
>  
>  static void free_buffer_page(struct ion_system_heap *heap,
>struct ion_buffer *buffer, struct page *page)
>  {
> + struct ion_page_pool *pool;
>   unsigned int order = compound_order(page);
>   bool cached = ion_buffer_cached(buffer);
>  
> - if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
> - struct ion_page_pool *pool = heap->pools[order_to_index(order)];
> -
> - ion_page_pool_free(pool, page);
> - } else {
> + if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) {
>   __free_pages(page, order);
> + return;
>   }
> +
> + if (!cached)
> + pool = heap->uncached_pools[order_to_index(order)];
> + else
> + pool = heap->cached_pools[order_to_index(order)];
> +
> + ion_page_pool_free(pool, page);
>  }
>  
>  
> @@ -224,9 +222,11 @@ static int ion_system_heap_shrink(struct ion_heap *heap, 
> gfp_t gfp_mask,
>   only_scan = 1;
>  
>   for (i = 0; i < num_orders; i++) {
> - struct ion_page_pool *pool = sys_heap->pools[i];
> + struct ion_page_pool *pool = sys_heap->uncached_pools[i];
>  
> - nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
> + nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
> + pool = sys_heap->cached_pools[i];
> + nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
>   nr_total += nr_freed;
>  
>   if (!only_scan) {
> @@ -259,27 +259,69 @@ static int ion_system_heap_debug_show(struct ion_heap 
> *heap, struct seq_file *s,
>   struct ion_system_heap,
>   heap);
>   int i;
> + struct ion_page_pool *pool;
>  
>   for (i = 0; i < num_orders; i++) {
> - struct ion_page_pool *pool = sys_heap->pools[i];
> + pool = sys_heap->uncached_pools[i];
>  
> - seq_printf(s, "%d order %u highmem pages in pool = %lu total\n",
> + seq_printf(s, "%d order %u highmem pages uncached %lu total\n",
>  pool->high_count, pool->order,
>  (PAGE_SIZE << pool->order) * pool->high_count);
> - seq_printf(s, "%d order %u lowmem pages in pool = %lu total\n",
> + seq_printf(s, "%d order %u lowm

[RESEND PATCH] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Add ion cached pool in system heap.

Signed-off-by: Chen Feng 
Signed-off-by: Xia  Qing 
Reviewed-by: Fu Jun 
---
 drivers/staging/android/ion/ion_system_heap.c | 145 +-
 1 file changed, 95 insertions(+), 50 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..c633252 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -49,7 +49,8 @@ static inline unsigned int order_to_size(int order)
 
 struct ion_system_heap {
struct ion_heap heap;
-   struct ion_page_pool *pools[0];
+   struct ion_page_pool *uncached_pools[0];
+   struct ion_page_pool *cached_pools[0];
 };
 
 static struct page *alloc_buffer_page(struct ion_system_heap *heap,
@@ -57,39 +58,36 @@ static struct page *alloc_buffer_page(struct 
ion_system_heap *heap,
  unsigned long order)
 {
bool cached = ion_buffer_cached(buffer);
-   struct ion_page_pool *pool = heap->pools[order_to_index(order)];
+   struct ion_page_pool *pool;
struct page *page;
 
-   if (!cached) {
-   page = ion_page_pool_alloc(pool);
-   } else {
-   gfp_t gfp_flags = low_order_gfp_flags;
-
-   if (order > 4)
-   gfp_flags = high_order_gfp_flags;
-   page = alloc_pages(gfp_flags | __GFP_COMP, order);
-   if (!page)
-   return NULL;
-   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
-   DMA_BIDIRECTIONAL);
-   }
+   if (!cached)
+   pool = heap->uncached_pools[order_to_index(order)];
+   else
+   pool = heap->cached_pools[order_to_index(order)];
 
+   page = ion_page_pool_alloc(pool);
return page;
 }
 
 static void free_buffer_page(struct ion_system_heap *heap,
 struct ion_buffer *buffer, struct page *page)
 {
+   struct ion_page_pool *pool;
unsigned int order = compound_order(page);
bool cached = ion_buffer_cached(buffer);
 
-   if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
-   struct ion_page_pool *pool = heap->pools[order_to_index(order)];
-
-   ion_page_pool_free(pool, page);
-   } else {
+   if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) {
__free_pages(page, order);
+   return;
}
+
+   if (!cached)
+   pool = heap->uncached_pools[order_to_index(order)];
+   else
+   pool = heap->cached_pools[order_to_index(order)];
+
+   ion_page_pool_free(pool, page);
 }
 
 
@@ -186,11 +184,10 @@ static void ion_system_heap_free(struct ion_buffer 
*buffer)
int i;
 
/*
-*  uncached pages come from the page pools, zero them before returning
-*  for security purposes (other allocations are zerod at
-*  alloc time
+*  zero the buffer before returning for security purposes
+*  (other allocations are zerod at alloc time)
 */
-   if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
+   if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
ion_heap_buffer_zero(buffer);
 
for_each_sg(table->sgl, sg, table->nents, i)
@@ -224,19 +221,29 @@ static int ion_system_heap_shrink(struct ion_heap *heap, 
gfp_t gfp_mask,
only_scan = 1;
 
for (i = 0; i < num_orders; i++) {
-   struct ion_page_pool *pool = sys_heap->pools[i];
-
-   nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
-   nr_total += nr_freed;
+   struct ion_page_pool *pool = sys_heap->uncached_pools[i];
 
if (!only_scan) {
+   nr_freed = ion_page_pool_shrink(pool, gfp_mask,
+   nr_to_scan);
+   nr_to_scan -= nr_freed;
+   nr_total += nr_freed;
+   pool = sys_heap->cached_pools[i];
+   nr_freed = ion_page_pool_shrink(pool, gfp_mask,
+   nr_to_scan);
nr_to_scan -= nr_freed;
-   /* shrink completed */
+   nr_total += nr_freed;
if (nr_to_scan <= 0)
break;
+   } else {
+   nr_freed = ion_page_pool_shrink(pool, gfp_mask,
+   nr_to_scan);
+   nr_total += nr_freed;
+   nr_freed = ion_page_pool_shrink(pool, gfp_mask,
+

[PATCH v1] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Add ion cached pool in system heap. This patch add a cached pool
in system heap. It has a great improvement of alloc for cached
buffer.

v1: Makes the cached buffer zeroed before going to pool

Signed-off-by: Chen Feng 
Signed-off-by: Xia  Qing 
Reviewed-by: Fu Jun 
---
 drivers/staging/android/ion/ion_system_heap.c | 155 +-
 1 file changed, 103 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..4b14a0b 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -49,47 +49,55 @@ static inline unsigned int order_to_size(int order)
 
 struct ion_system_heap {
struct ion_heap heap;
-   struct ion_page_pool *pools[0];
+   struct ion_page_pool *uncached_pools[0];
+   struct ion_page_pool *cached_pools[0];
 };
 
+/**
+ * The page from page-pool are all zeroed before. We need do cache
+ * clean for cached buffer. The uncached buffer are always non-cached
+ * since it's allocated. So no need for non-cached pages.
+ */
 static struct page *alloc_buffer_page(struct ion_system_heap *heap,
  struct ion_buffer *buffer,
  unsigned long order)
 {
bool cached = ion_buffer_cached(buffer);
-   struct ion_page_pool *pool = heap->pools[order_to_index(order)];
+   struct ion_page_pool *pool;
struct page *page;
 
-   if (!cached) {
-   page = ion_page_pool_alloc(pool);
-   } else {
-   gfp_t gfp_flags = low_order_gfp_flags;
+   if (!cached)
+   pool = heap->uncached_pools[order_to_index(order)];
+   else
+   pool = heap->cached_pools[order_to_index(order)];
 
-   if (order > 4)
-   gfp_flags = high_order_gfp_flags;
-   page = alloc_pages(gfp_flags | __GFP_COMP, order);
-   if (!page)
-   return NULL;
-   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
-   DMA_BIDIRECTIONAL);
-   }
+   page = ion_page_pool_alloc(pool);
 
+   if (cached)
+   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
+ DMA_BIDIRECTIONAL);
return page;
 }
 
 static void free_buffer_page(struct ion_system_heap *heap,
 struct ion_buffer *buffer, struct page *page)
 {
+   struct ion_page_pool *pool;
unsigned int order = compound_order(page);
bool cached = ion_buffer_cached(buffer);
 
-   if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
-   struct ion_page_pool *pool = heap->pools[order_to_index(order)];
-
-   ion_page_pool_free(pool, page);
-   } else {
+   /* go to system */
+   if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) {
__free_pages(page, order);
+   return;
}
+
+   if (!cached)
+   pool = heap->uncached_pools[order_to_index(order)];
+   else
+   pool = heap->cached_pools[order_to_index(order)];
+
+   ion_page_pool_free(pool, page);
 }
 
 
@@ -181,16 +189,11 @@ static void ion_system_heap_free(struct ion_buffer 
*buffer)
struct ion_system_heap,
heap);
struct sg_table *table = buffer->sg_table;
-   bool cached = ion_buffer_cached(buffer);
struct scatterlist *sg;
int i;
 
-   /*
-*  uncached pages come from the page pools, zero them before returning
-*  for security purposes (other allocations are zerod at
-*  alloc time
-*/
-   if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
+   /* zero the buffer before goto page pool */
+   if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
ion_heap_buffer_zero(buffer);
 
for_each_sg(table->sgl, sg, table->nents, i)
@@ -224,19 +227,29 @@ static int ion_system_heap_shrink(struct ion_heap *heap, 
gfp_t gfp_mask,
only_scan = 1;
 
for (i = 0; i < num_orders; i++) {
-   struct ion_page_pool *pool = sys_heap->pools[i];
-
-   nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
-   nr_total += nr_freed;
+   struct ion_page_pool *pool = sys_heap->uncached_pools[i];
 
if (!only_scan) {
+   nr_freed = ion_page_pool_shrink(pool, gfp_mask,
+   nr_to_scan);
nr_to_scan -= nr_freed;
-   /* shrink completed */
+

Re: [RESEND PATCH] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Hi Greg,

Please see my v1 version for detail.

Thank you.

On 2016年05月09日 17:02, Greg KH wrote:
> On Mon, May 09, 2016 at 04:37:34PM +0800, Chen Feng wrote:
>> Add ion cached pool in system heap.
>>
>> Signed-off-by: Chen Feng 
>> Signed-off-by: Xia  Qing 
>> Reviewed-by: Fu Jun 
>> ---
>>  drivers/staging/android/ion/ion_system_heap.c | 145 
>> +-
>>  1 file changed, 95 insertions(+), 50 deletions(-)
> 
> You didn't list what changed here.  Please do so, otherwise we think
> it's identical to the previous patch you sent :(
> 
> thanks,
> 
> greg k-h
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RESEND PATCH] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Hi Laura,

On 2016/5/10 7:50, Laura Abbott wrote:
> On 05/09/2016 01:37 AM, Chen Feng wrote:
>> Add ion cached pool in system heap.
>>
>> Signed-off-by: Chen Feng 
>> Signed-off-by: Xia  Qing 
>> Reviewed-by: Fu Jun 
>> ---
>>  drivers/staging/android/ion/ion_system_heap.c | 145 
>> +-
>>  1 file changed, 95 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
>> b/drivers/staging/android/ion/ion_system_heap.c
>> index b69dfc7..c633252 100644
>> --- a/drivers/staging/android/ion/ion_system_heap.c
>> +++ b/drivers/staging/android/ion/ion_system_heap.c
>> @@ -49,7 +49,8 @@ static inline unsigned int order_to_size(int order)
>>
>>  struct ion_system_heap {
>>  struct ion_heap heap;
>> -struct ion_page_pool *pools[0];
>> +struct ion_page_pool *uncached_pools[0];
>> +struct ion_page_pool *cached_pools[0];
>>  };
>>
>>  static struct page *alloc_buffer_page(struct ion_system_heap *heap,
>> @@ -57,39 +58,36 @@ static struct page *alloc_buffer_page(struct 
>> ion_system_heap *heap,
>>unsigned long order)
>>  {
>>  bool cached = ion_buffer_cached(buffer);
>> -struct ion_page_pool *pool = heap->pools[order_to_index(order)];
>> +struct ion_page_pool *pool;
>>  struct page *page;
>>
>> -if (!cached) {
>> -page = ion_page_pool_alloc(pool);
>> -} else {
>> -gfp_t gfp_flags = low_order_gfp_flags;
>> -
>> -if (order > 4)
>> -gfp_flags = high_order_gfp_flags;
>> -page = alloc_pages(gfp_flags | __GFP_COMP, order);
>> -if (!page)
>> -return NULL;
>> -ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
>> -DMA_BIDIRECTIONAL);
>> -}
>> +if (!cached)
>> +pool = heap->uncached_pools[order_to_index(order)];
>> +else
>> +pool = heap->cached_pools[order_to_index(order)];
>>
>> +page = ion_page_pool_alloc(pool);
>>  return page;
>>  }
>>
> 
> This is a change in behavior. The page is no longer guaranteed to be synced
> in the cache. If the page came from the pool (not just freshly allocated)
> the cache state is unknown. Do you have a good explanation why we no longer
> need to do the cache flush here on every allocation?
> 
Yes, no more explanation here.
Please see my v1[1] version.

Thanks for review!

[1] http://thread.gmane.org/gmane.linux.kernel/2217505

> Thanks,
> Laura
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

On 2016/5/11 6:48, Laura Abbott wrote:
> On 05/09/2016 04:35 AM, Chen Feng wrote:
>> Add ion cached pool in system heap. This patch add a cached pool
>> in system heap. It has a great improvement of alloc for cached
>> buffer.
>>
> 
> Can you give some benchmark numbers here?
> 

ok, My test is using iontest userspace.
I will share the result next version.
By the way, which benchmark you prefer to use for ION test?

>> v1: Makes the cached buffer zeroed before going to pool
>>
>> Signed-off-by: Chen Feng 
>> Signed-off-by: Xia  Qing 
>> Reviewed-by: Fu Jun 
>> ---
>>  drivers/staging/android/ion/ion_system_heap.c | 155 
>> +-
>>  1 file changed, 103 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
>> b/drivers/staging/android/ion/ion_system_heap.c
>> index b69dfc7..4b14a0b 100644
>> --- a/drivers/staging/android/ion/ion_system_heap.c
>> +++ b/drivers/staging/android/ion/ion_system_heap.c
>> @@ -49,47 +49,55 @@ static inline unsigned int order_to_size(int order)
>>
>>  struct ion_system_heap {
>>  struct ion_heap heap;
>> -struct ion_page_pool *pools[0];
>> +struct ion_page_pool *uncached_pools[0];
>> +struct ion_page_pool *cached_pools[0];
>>  };
>>
> 
> 
> I don't think this is correct based on 
> https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> and some discussions with others. Their tests showed this may just result in 
> the
> two struct members aliasing, which is not what we want.
> 
> The flexible array isn't really necessary here anyway. The number of orders 
> we want
> is known at compile time and
> 
Yes, change this latter.

> -static const int num_orders = ARRAY_SIZE(orders);
> +
> +#define NUM_ORDERS ARRAY_SIZE(orders)
> +
> 
> should let the NUM_ORDERS be used as a constant initializer in the struct.
> 
ok.
>> +/**
>> + * The page from page-pool are all zeroed before. We need do cache
>> + * clean for cached buffer. The uncached buffer are always non-cached
>> + * since it's allocated. So no need for non-cached pages.
>> + */
>>  static struct page *alloc_buffer_page(struct ion_system_heap *heap,
>>struct ion_buffer *buffer,
>>unsigned long order)
>>  {
>>  bool cached = ion_buffer_cached(buffer);
>> -struct ion_page_pool *pool = heap->pools[order_to_index(order)];
>> +struct ion_page_pool *pool;
>>  struct page *page;
>>
>> -if (!cached) {
>> -page = ion_page_pool_alloc(pool);
>> -} else {
>> -gfp_t gfp_flags = low_order_gfp_flags;
>> +if (!cached)
>> +pool = heap->uncached_pools[order_to_index(order)];
>> +else
>> +pool = heap->cached_pools[order_to_index(order)];
>>
>> -if (order > 4)
>> -gfp_flags = high_order_gfp_flags;
>> -page = alloc_pages(gfp_flags | __GFP_COMP, order);
>> -if (!page)
>> -return NULL;
>> -ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
>> -DMA_BIDIRECTIONAL);
>> -}
>> +page = ion_page_pool_alloc(pool);
>>
>> +if (cached)
>> +ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
>> +  DMA_BIDIRECTIONAL);
>>  return page;
>>  }
>>
> 
> This is doing an extra sync for newly allocated pages. If the buffer was
> just allocated the sync should be skipped.
> 


Yes, if we want not to do this extra sync for newly allocated pages.

I will add a new parameter in the ion page-pool to distinguish from
it's cached or not.

>>  static void free_buffer_page(struct ion_system_heap *heap,
>>   struct ion_buffer *buffer, struct page *page)
>>  {
>> +struct ion_page_pool *pool;
>>  unsigned int order = compound_order(page);
>>  bool cached = ion_buffer_cached(buffer);
>>
>> -if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
>> -struct ion_page_pool *pool = heap->pools[order_to_index(order)];
>> -
>> -ion_page_pool_free(pool, page);
>> -} else {
>> +/* go to system */
>> +if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) {
>>  __free_pages(page, order);
>> +return;
>>  }
>> +
>> +if (!cached)
>> +pool = heap->uncached_pools[order_to_index(order)];
>> +

[PATCH v2] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Add ion cached pool in system heap. This patch add a cached pool
in system heap. It has a great improvement of alloc for cached
buffer.

With memory pressue alloc test 800MB in userspace used iontest.
The result avg is 577ms. Without patch it's avg is about 883ms.

v1: Makes the cached buffer zeroed before going to pool
v2: Add cached param in pool to distinguish wheather need to flush
cache at a fresh alloc.
Rework the shrink function.

Signed-off-by: Chen Feng 
Signed-off-by: Xia  Qing 
Reviewed-by: Fu Jun 
---
 drivers/staging/android/ion/ion_page_pool.c   |  10 +-
 drivers/staging/android/ion/ion_priv.h|   5 +-
 drivers/staging/android/ion/ion_system_heap.c | 183 +-
 3 files changed, 133 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c 
b/drivers/staging/android/ion/ion_page_pool.c
index 1fe8016..2c5e5c5 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -30,8 +30,9 @@ static void *ion_page_pool_alloc_pages(struct ion_page_pool 
*pool)
 
if (!page)
return NULL;
-   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
-   DMA_BIDIRECTIONAL);
+   if (!pool->cached)
+   ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
+ DMA_BIDIRECTIONAL);
return page;
 }
 
@@ -147,7 +148,8 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t 
gfp_mask,
return freed;
 }
 
-struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order)
+struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order,
+  bool cached)
 {
struct ion_page_pool *pool = kmalloc(sizeof(*pool), GFP_KERNEL);
 
@@ -161,6 +163,8 @@ struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, 
unsigned int order)
pool->order = order;
mutex_init(&pool->mutex);
plist_node_init(&pool->list, order);
+   if (cached)
+   pool->cached = true;
 
return pool;
 }
diff --git a/drivers/staging/android/ion/ion_priv.h 
b/drivers/staging/android/ion/ion_priv.h
index 0239883..f3cb8b4 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -360,6 +360,7 @@ void ion_carveout_free(struct ion_heap *heap, 
ion_phys_addr_t addr,
  * @gfp_mask:  gfp_mask to use from alloc
  * @order: order of pages in the pool
  * @list:  plist node for list of pools
+ * @cached:it's cached pool or not
  *
  * Allows you to keep a pool of pre allocated pages to use from your heap.
  * Keeping a pool of pages that is ready for dma, ie any cached mapping have
@@ -369,6 +370,7 @@ void ion_carveout_free(struct ion_heap *heap, 
ion_phys_addr_t addr,
 struct ion_page_pool {
int high_count;
int low_count;
+   bool cached;
struct list_head high_items;
struct list_head low_items;
struct mutex mutex;
@@ -377,7 +379,8 @@ struct ion_page_pool {
struct plist_node list;
 };
 
-struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order);
+struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order,
+  bool cached);
 void ion_page_pool_destroy(struct ion_page_pool *);
 struct page *ion_page_pool_alloc(struct ion_page_pool *);
 void ion_page_pool_free(struct ion_page_pool *, struct page *);
diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..32a0ebf 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -26,16 +26,18 @@
 #include "ion.h"
 #include "ion_priv.h"
 
+#define NUM_ORDERS ARRAY_SIZE(orders)
+
 static gfp_t high_order_gfp_flags = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN |
 __GFP_NORETRY) & ~__GFP_RECLAIM;
 static gfp_t low_order_gfp_flags  = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN);
 static const unsigned int orders[] = {8, 4, 0};
-static const int num_orders = ARRAY_SIZE(orders);
+
 static int order_to_index(unsigned int order)
 {
int i;
 
-   for (i = 0; i < num_orders; i++)
+   for (i = 0; i < NUM_ORDERS; i++)
if (order == orders[i])
return i;
BUG();
@@ -49,47 +51,55 @@ static inline unsigned int order_to_size(int order)
 
 struct ion_system_heap {
struct ion_heap heap;
-   struct ion_page_pool *pools[0];
+   struct ion_page_pool *uncached_pools[NUM_ORDERS];
+   struct ion_page_pool *cached_pools[NUM_ORDERS];
 };
 
+/**
+ * The page from page-pool are all zeroed before. We need do cache
+ * clean for cached buffer. The u

[PATCH] staging: ion: make the pte default none PTE_RDONLY

[Chen Feng]

The page is already alloc at ion_alloc function,
ion_mmap map the alloced pages to user-space.

The default prot can be PTE_RDONLY. Take a look at
here:
set_pte_at()
arch/arm64/include/asm:
if (pte_dirty(pte) && pte_write(pte))
pte_val(pte) &= ~PTE_RDONLY;
else
pte_val(pte) |= PTE_RDONLY;

So with the dirty bit,it can improve the efficiency
and donnot need to handle memory fault when use access.

Signed-off-by: Chen Feng 
Signed-off-by: Wei Dong 
Reviewed-by: Zhuangluan Su 
---
 drivers/staging/android/ion/ion.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index e237e9f..dba5942 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1026,6 +1026,9 @@ static int ion_mmap(struct dma_buf *dmabuf, struct 
vm_area_struct *vma)
if (!(buffer->flags & ION_FLAG_CACHED))
vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
 
+   /*Default writeable*/
+   vma->vm_page_prot = pte_mkdirty(vma->vm_page_prot);
+
mutex_lock(&buffer->lock);
/* now map it to userspace */
ret = buffer->heap->ops->map_user(buffer->heap, buffer, vma);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ion : Donnot wakeup kswapd in ion system alloc

[Chen Feng]

Since ion alloc can be called by userspace,eg gralloc.
When it is called frequently, the efficiency of kswapd is
to low. And the reclaimed memory is too lower. In this way,
the kswapd can use to much cpu resources.

With 3.5GB DMA Zone and 0.5 Normal Zone.

pgsteal_kswapd_dma 9364140
pgsteal_kswapd_normal 7071043
pgscan_kswapd_dma 10428250
pgscan_kswapd_normal 37840094

With this change the reclaim ratio has greatly improved
18.9% -> 72.5%

Signed-off-by: Chen Feng 
Signed-off-by: Lu bing 
---
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index d4c3e55..b69dfc7 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -27,7 +27,7 @@
 #include "ion_priv.h"
 
 static gfp_t high_order_gfp_flags = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN |
-__GFP_NORETRY) & ~__GFP_DIRECT_RECLAIM;
+__GFP_NORETRY) & ~__GFP_RECLAIM;
 static gfp_t low_order_gfp_flags  = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN);
 static const unsigned int orders[] = {8, 4, 0};
 static const int num_orders = ARRAY_SIZE(orders);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] android: binder: Sanity check at binder ioctl

[Chen Feng]

When a process fork a child process, we should not allow the
child process use the binder which opened by parent process.

But if the binder-object creater is a thread of one process who exit,
the other thread can also use this binder-object normally.
We can distinguish this by the member proc->tsk->mm.
If the thread exit the tsk->mm will be NULL.

proc->tsk->mm != current->tsk->mm && proc->tsk->mm

So only allow the shared mm_struct to use the same binder-object and
check the existence of mm_struct.

Signed-off-by: Chen Feng 
Signed-off-by: Wei  Dong 
Signed-off-by: Junmin Zhao 
Reviewed-by: Zhuangluan Su 
---
 drivers/android/binder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index a39e85f..3a74c5d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2736,6 +2736,8 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
 
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
+   if (unlikely(proc->tsk->mm != current->tsk->mm && proc->tsk->mm))
+   return -EINVAL;
 
trace_binder_ioctl(cmd, arg);
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH RESEND] android: binder: Sanity check at binder ioctl

[Chen Feng]

When a process fork a child process, we should not allow the
child process use the binder which opened by parent process.

But if the binder-object creater is a thread of one process who exit,
the other thread can also use this binder-object normally.
We can distinguish this by the member proc->tsk->mm.
If the thread exit the tsk->mm will be NULL.

proc->tsk->mm != current->mm && proc->tsk->mm

So only allow the shared mm_struct to use the same binder-object and
check the existence of mm_struct.

Signed-off-by: Chen Feng 
Signed-off-by: Wei  Dong 
Signed-off-by: Junmin Zhao 
Reviewed-by: Zhuangluan Su 
---
 drivers/android/binder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index a39e85f..279063c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2736,6 +2736,8 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
 
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
+   if (unlikely(proc->tsk->mm != current->mm && proc->tsk->mm))
+   return -EINVAL;
 
trace_binder_ioctl(cmd, arg);
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] android: binder: Sanity check at binder ioctl

[Chen Feng]

When a process fork a child process, we should not allow the
child process use the binder which opened by parent process.

But if the binder-object creater is a thread of one process who exit,
the other thread can also use this binder-object normally.
We can distinguish this by the member proc->tsk->mm.
If the thread exit the tsk->mm will be NULL.

proc->tsk->mm != current->mm && proc->tsk->mm

So only allow the shared mm_struct to use the same binder-object and
check the existence of mm_struct.

V2: Fix compile error for error commit

Signed-off-by: Chen Feng 
Signed-off-by: Wei  Dong 
Signed-off-by: Junmin Zhao 
Reviewed-by: Zhuangluan Su 
---
 drivers/android/binder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index a39e85f..279063c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2736,6 +2736,8 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
 
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
+   if (unlikely(proc->tsk->mm != current->mm && proc->tsk->mm))
+   return -EINVAL;
 
trace_binder_ioctl(cmd, arg);
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] android: binder: Sanity check at binder ioctl

[Chen Feng]

Sanity check at binder ioctl function,
Only allow the shared mm_struct to use the same binder-object
to do binder operate.

And add proc->vma_vm_mm = current->mm at the open function.
The libbinder do ioctl before mmap called.

V2: Fix compile error for error commit
V3: Change the condition to proc->vma_vm_mm

Signed-off-by: Chen Feng 
Signed-off-by: Wei  Dong 
Signed-off-by: Junmin Zhao 
Reviewed-by: Zhuangluan Su 
---
 drivers/android/binder.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index a39e85f..f080a8b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2737,6 +2737,10 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
 
+   if (unlikely(current->mm != proc->vma_vm_mm)) {
+   pr_err("current mm mismatch proc mm\n");
+   return -EINVAL;
+   }
trace_binder_ioctl(cmd, arg);
 
ret = wait_event_interruptible(binder_user_error_wait, 
binder_stop_on_user_error < 2);
@@ -2951,6 +2955,7 @@ static int binder_open(struct inode *nodp, struct file 
*filp)
return -ENOMEM;
get_task_struct(current);
proc->tsk = current;
+   proc->vma_vm_mm = current->mm;
INIT_LIST_HEAD(&proc->todo);
init_waitqueue_head(&proc->wait);
proc->default_priority = task_nice(current);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] android: binder: Sanity check at binder ioctl

[Chen Feng]

Add android kernel team.

On 2016/2/1 14:04, Chen Feng wrote:
> Sanity check at binder ioctl function,
> Only allow the shared mm_struct to use the same binder-object
> to do binder operate.
> 
> And add proc->vma_vm_mm = current->mm at the open function.
> The libbinder do ioctl before mmap called.
> 
> V2: Fix compile error for error commit
> V3: Change the condition to proc->vma_vm_mm
> 
> Signed-off-by: Chen Feng 
> Signed-off-by: Wei  Dong 
> Signed-off-by: Junmin Zhao 
> Reviewed-by: Zhuangluan Su 
> ---
>  drivers/android/binder.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index a39e85f..f080a8b 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2737,6 +2737,10 @@ static long binder_ioctl(struct file *filp, unsigned 
> int cmd, unsigned long arg)
>   /*pr_info("binder_ioctl: %d:%d %x %lx\n",
>   proc->pid, current->pid, cmd, arg);*/
>  
> + if (unlikely(current->mm != proc->vma_vm_mm)) {
> + pr_err("current mm mismatch proc mm\n");
> + return -EINVAL;
> + }
>   trace_binder_ioctl(cmd, arg);
>  
>   ret = wait_event_interruptible(binder_user_error_wait, 
> binder_stop_on_user_error < 2);
> @@ -2951,6 +2955,7 @@ static int binder_open(struct inode *nodp, struct file 
> *filp)
>   return -ENOMEM;
>   get_task_struct(current);
>   proc->tsk = current;
> + proc->vma_vm_mm = current->mm;
>   INIT_LIST_HEAD(&proc->todo);
>   init_waitqueue_head(&proc->wait);
>   proc->default_priority = task_nice(current);
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Hi Greg,

Can you take this patch?
Thanks

On 2016年05月24日 06:32, Laura Abbott wrote:
> On 05/18/2016 08:03 PM, Chen Feng wrote:
>> Add ion cached pool in system heap. This patch add a cached pool
>> in system heap. It has a great improvement of alloc for cached
>> buffer.
>>
>> With memory pressue alloc test 800MB in userspace used iontest.
>> The result avg is 577ms. Without patch it's avg is about 883ms.
>>
>> v1: Makes the cached buffer zeroed before going to pool
>> v2: Add cached param in pool to distinguish wheather need to flush
>> cache at a fresh alloc.
>> Rework the shrink function.
>>
>> Signed-off-by: Chen Feng 
>> Signed-off-by: Xia  Qing 
>> Reviewed-by: Fu Jun 
> 
> Acked-by: Laura Abbott 
> 
>> ---
>>  drivers/staging/android/ion/ion_page_pool.c   |  10 +-
>>  drivers/staging/android/ion/ion_priv.h|   5 +-
>>  drivers/staging/android/ion/ion_system_heap.c | 183 
>> +-
>>  3 files changed, 133 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_page_pool.c 
>> b/drivers/staging/android/ion/ion_page_pool.c
>> index 1fe8016..2c5e5c5 100644
>> --- a/drivers/staging/android/ion/ion_page_pool.c
>> +++ b/drivers/staging/android/ion/ion_page_pool.c
>> @@ -30,8 +30,9 @@ static void *ion_page_pool_alloc_pages(struct 
>> ion_page_pool *pool)
>>
>>  if (!page)
>>  return NULL;
>> -ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
>> -DMA_BIDIRECTIONAL);
>> +if (!pool->cached)
>> +ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
>> +  DMA_BIDIRECTIONAL);
>>  return page;
>>  }
>>
>> @@ -147,7 +148,8 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, 
>> gfp_t gfp_mask,
>>  return freed;
>>  }
>>
>> -struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int 
>> order)
>> +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int 
>> order,
>> +   bool cached)
>>  {
>>  struct ion_page_pool *pool = kmalloc(sizeof(*pool), GFP_KERNEL);
>>
>> @@ -161,6 +163,8 @@ struct ion_page_pool *ion_page_pool_create(gfp_t 
>> gfp_mask, unsigned int order)
>>  pool->order = order;
>>  mutex_init(&pool->mutex);
>>  plist_node_init(&pool->list, order);
>> +if (cached)
>> +pool->cached = true;
>>
>>  return pool;
>>  }
>> diff --git a/drivers/staging/android/ion/ion_priv.h 
>> b/drivers/staging/android/ion/ion_priv.h
>> index 0239883..f3cb8b4 100644
>> --- a/drivers/staging/android/ion/ion_priv.h
>> +++ b/drivers/staging/android/ion/ion_priv.h
>> @@ -360,6 +360,7 @@ void ion_carveout_free(struct ion_heap *heap, 
>> ion_phys_addr_t addr,
>>   * @gfp_mask:gfp_mask to use from alloc
>>   * @order:order of pages in the pool
>>   * @list:plist node for list of pools
>> + * @cached:it's cached pool or not
>>   *
>>   * Allows you to keep a pool of pre allocated pages to use from your heap.
>>   * Keeping a pool of pages that is ready for dma, ie any cached mapping have
>> @@ -369,6 +370,7 @@ void ion_carveout_free(struct ion_heap *heap, 
>> ion_phys_addr_t addr,
>>  struct ion_page_pool {
>>  int high_count;
>>  int low_count;
>> +bool cached;
>>  struct list_head high_items;
>>  struct list_head low_items;
>>  struct mutex mutex;
>> @@ -377,7 +379,8 @@ struct ion_page_pool {
>>  struct plist_node list;
>>  };
>>
>> -struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int 
>> order);
>> +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int 
>> order,
>> +   bool cached);
>>  void ion_page_pool_destroy(struct ion_page_pool *);
>>  struct page *ion_page_pool_alloc(struct ion_page_pool *);
>>  void ion_page_pool_free(struct ion_page_pool *, struct page *);
>> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
>> b/drivers/staging/android/ion/ion_system_heap.c
>> index b69dfc7..32a0ebf 100644
>> --- a/drivers/staging/android/ion/ion_system_heap.c
>> +++ b/drivers/staging/android/ion/ion_system_heap.c
>> @@ -26,16 +26,18 @@
>>  #include "ion.h"
>>  #include "ion_priv.h"
>>
>> +#define NUM_ORDERS ARRAY_SIZE(orders)
>> +
>&g

Re: [Linaro-mm-sig] [RFC][PATCH 0/6] ion: improved ABI

[Chen Feng]

The idea is good, define the heap ids in header file is inconvenient.

But if we query the heaps information from user-space.

It need to maintain this ids and name userspace one by one. The code may
be complicated in different module user-space.

In android, the gralloc and other lib will all use ion to alloc memory.

This will make it more difficult to maintain user-space code.


But beyond this, The new alloc2 with not-handle flag is good.
And the pull out of ioctl interface is also a good cleanup.

On 2016/6/7 2:23, Laura Abbott wrote:
> 
> The ABI for Ion's ioctl interface are a pain to work with. The heap IDs
> are a 32-bit non-discoverable namespace that form part of the ABI. There's
> no way to determine what ABI version is in use which leads to problems
> if the ABI changes or needs to be updated.
> 
> This series is a first approach to give a better ABI for Ion. This includes:
> 
> - Following the advice in botching-up-ioctls.txt
> - Ioctl for ABI version
> - Dynamic assignment of heap ids
> - queryable heap ids
> - Runtime mapping of heap ids, including fallbacks. This avoids the need to
>   encode the fallbacks as an ABI.
> 
> I'm most interested in feedback if this ABI is actually an improvement and
> usable. The heap id map/query interface seems error prone but I didn't have
> a cleaner solution. There aren't any kernel APIs for the new features as the
> focus was on a userspace API but I anticipate that following easily once
> the userspace API is established.
> 
> 
> Thanks,
> Laura
> 
> P.S. Not to turn this into a bike shedding session but if you have suggestions
> for a name for this framework other than Ion I would be interested to hear
> them. Too many other things are already named Ion.
> 
> Laura Abbott (6):
>   staging: android: ion: return error value for ion_device_add_heap
>   staging: android: ion: Switch to using an idr to manage heaps
>   staging: android: ion: Drop heap type masks
>   staging: android: ion: Pull out ion ioctls to a separate file
>   staging: android: ion: Add an ioctl for ABI checking
>   staging: android: ion: Introduce new ioctls for dynamic heaps
> 
>  drivers/staging/android/ion/Makefile|   3 +-
>  drivers/staging/android/ion/ion-ioctl.c | 243 ++
>  drivers/staging/android/ion/ion.c   | 438 
> 
>  drivers/staging/android/ion/ion_priv.h  | 109 +++-
>  drivers/staging/android/uapi/ion.h  | 164 +++-
>  5 files changed, 728 insertions(+), 229 deletions(-)
>  create mode 100644 drivers/staging/android/ion/ion-ioctl.c
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] android: binder: fix dangling pointer comparison

[Chen Feng]

looks good to me.

On 2016/6/16 6:45, Jann Horn wrote:
> If /dev/binder is opened and the opener process then e.g. calls execve,
> proc->vma_vm_mm will still point to the location of the now-freed
> mm_struct. If the process then calls ioctl(binder_fd, ...), the dangling
> proc->vma_vm_mm pointer will be compared to current->mm.
> 
> Let the binder take a reference to the mm_struct to avoid this.
> 
> v2: use the right refcounter
> 
> Fixes: a906d6931f3ccaf7de805643190765ddd7378e27
> Signed-off-by: Jann Horn 

Reviewed-by: Chen Feng 
> ---
>  drivers/android/binder.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 16288e7..09fdb42 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2962,6 +2962,7 @@ static int binder_open(struct inode *nodp, struct file 
> *filp)
>   return -ENOMEM;
>   get_task_struct(current);
>   proc->tsk = current;
> + atomic_inc(¤t->mm->mm_count);
>   proc->vma_vm_mm = current->mm;
>   INIT_LIST_HEAD(&proc->todo);
>   init_waitqueue_head(&proc->wait);
> @@ -3167,6 +3168,7 @@ static void binder_deferred_release(struct binder_proc 
> *proc)
>   vfree(proc->buffer);
>   }
>  
> + mmdrop(proc->vma_vm_mm);
>   put_task_struct(proc->tsk);
>  
>   binder_debug(BINDER_DEBUG_OPEN_CLOSE,
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] ION: Sys_heap: Add cached pool to spead up cached buffer alloc

[Chen Feng]

Greg,

I checkout your staging tree[1].

Not find this patch. Can you take it thanks!

[1]git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
On 2016/6/5 23:02, Greg KH wrote:
> On Sun, Jun 05, 2016 at 04:51:23PM +0800, Chen Feng wrote:
>> Hi Greg,
>>
>> Can you take this patch?
> 
> It's in my queue, please wait, staging patches are at the bottom of it,
> rightfully so...
> 
> thanks,
> 
> greg k-h
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ion: remove __GFP_NOWARN when use low order gfp flags

[Chen Feng]

It's useful to show the current memory in detail when alloc failed.

And, there may be a lot of high order alloc failed, just show memory
when an order 0 alloc failed.

Signed-off-by: Chen Feng 
---
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b69dfc7..c3b7975 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -28,7 +28,7 @@
 
 static gfp_t high_order_gfp_flags = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN |
 __GFP_NORETRY) & ~__GFP_RECLAIM;
-static gfp_t low_order_gfp_flags  = (GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN);
+static gfp_t low_order_gfp_flags  = (GFP_HIGHUSER | __GFP_ZERO);
 static const unsigned int orders[] = {8, 4, 0};
 static const int num_orders = ARRAY_SIZE(orders);
 static int order_to_index(unsigned int order)
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: android: ion: Add ion driver for Hi6220 SoC platform

[Chen Feng]

Signed-off-by: Chen Feng 
Signed-off-by: Yu Dongbin 
---
 drivers/staging/android/ion/Kconfig|   7 +
 drivers/staging/android/ion/Makefile   |   1 +
 drivers/staging/android/ion/hisilicon/Kconfig  |   5 +
 drivers/staging/android/ion/hisilicon/Makefile |   1 +
 drivers/staging/android/ion/hisilicon/hi6220_ion.c | 201 +
 5 files changed, 215 insertions(+)
 create mode 100644 drivers/staging/android/ion/hisilicon/Kconfig
 create mode 100644 drivers/staging/android/ion/hisilicon/Makefile
 create mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c

diff --git a/drivers/staging/android/ion/Kconfig 
b/drivers/staging/android/ion/Kconfig
index 3452346..19c1572 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -33,3 +33,10 @@ config ION_TEGRA
help
  Choose this option if you wish to use ion on an nVidia Tegra.
 
+config ION_HISI
+   tristate "Ion for Hisilicon"
+   depends on ARCH_HISI && ION
+   help
+ Choose this option if you wish to use ion on Hisilicon Platform.
+
+source "drivers/staging/android/ion/hisilicon/Kconfig"
diff --git a/drivers/staging/android/ion/Makefile 
b/drivers/staging/android/ion/Makefile
index b56fd2b..18cc2aa 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -7,4 +7,5 @@ endif
 
 obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
 obj-$(CONFIG_ION_TEGRA) += tegra/
+obj-$(CONFIG_ION_HISI) += hisilicon/
 
diff --git a/drivers/staging/android/ion/hisilicon/Kconfig 
b/drivers/staging/android/ion/hisilicon/Kconfig
new file mode 100644
index 000..2b4bd07
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/Kconfig
@@ -0,0 +1,5 @@
+config HI6220_ION
+bool "Hi6220 ION Driver"
+depends on ARCH_HISI && ION
+help
+  Build the Hisilicon Hi6220 ion driver.
diff --git a/drivers/staging/android/ion/hisilicon/Makefile 
b/drivers/staging/android/ion/hisilicon/Makefile
new file mode 100644
index 000..2a89414
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_HI6220_ION) += hi6220_ion.o
diff --git a/drivers/staging/android/ion/hisilicon/hi6220_ion.c 
b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
new file mode 100644
index 000..b7d39b8
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
@@ -0,0 +1,201 @@
+#define pr_fmt(fmt) "Ion: " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../ion_priv.h"
+#include "../ion.h"
+
+struct hi6220_ion_type_table {
+   const char *name;
+   enum ion_heap_type type;
+};
+
+static struct hi6220_ion_type_table ion_type_table[] = {
+   {"ion_system", ION_HEAP_TYPE_SYSTEM},
+   {"ion_system_contig", ION_HEAP_TYPE_SYSTEM_CONTIG},
+   {"ion_carveout", ION_HEAP_TYPE_CARVEOUT},
+   {"ion_chunk", ION_HEAP_TYPE_CHUNK},
+   {"ion_dma", ION_HEAP_TYPE_DMA},
+   {"ion_custom", ION_HEAP_TYPE_CUSTOM},
+};
+
+static struct ion_device *idev;
+static int num_heaps;
+static struct ion_heap **heaps;
+static struct ion_platform_heap **heaps_data;
+
+static int get_type_by_name(const char *name, enum ion_heap_type *type)
+{
+   int i, n;
+
+   n = ARRAY_SIZE(ion_type_table);
+   for (i = 0; i < n; i++) {
+   if (strncmp(name, ion_type_table[i].name, strlen(name)))
+   continue;
+
+   *type = ion_type_table[i].type;
+   return 0;
+   }
+
+   return -1;
+}
+
+static int hi6220_set_platform_data(struct platform_device *pdev)
+{
+   unsigned int base;
+   unsigned int size;
+   unsigned int id;
+   const char *heap_name;
+   const char *type_name;
+   enum ion_heap_type type;
+   int ret;
+   struct device_node *np;
+   struct ion_platform_heap *p_data;
+   const struct device_node *dt_node = pdev->dev.of_node;
+   int index = 0;
+
+   for_each_child_of_node(dt_node, np)
+   num_heaps++;
+
+   heaps_data = devm_kzalloc(&pdev->dev,
+ sizeof(struct ion_platform_heap *) * 
num_heaps,
+ GFP_KERNEL);
+
+   for_each_child_of_node(dt_node, np) {
+   p_data = devm_kzalloc(&pdev->dev,
+ sizeof(struct ion_platform_heap),
+ GFP_KERNEL);
+
+   ret = of_property_read_string(np, "heap-name", &heap_name);
+   if (ret < 0) {
+   pr_err("check the name of node %s\n", np->name);
+   continue;
+   }
+
+   ret = of_property_read_u32(np, "heap-id", &id);
+   if (ret < 0) {
+

[PATCH 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC.

[Chen Feng]

Add ION node to enable ION on hi6220 SoC platform

Signed-off-by: Chen Feng 
Signed-off-by: Yu Dongbin 
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts |  1 +
 arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi  | 23 +++
 2 files changed, 24 insertions(+)
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts 
b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index e36a539..44b75d2 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -11,6 +11,7 @@
 /memreserve/ 0x05e0 0x0010;
 
 #include "hi6220.dtsi"
+#include "hi6220-ion.dtsi"
 
 / {
model = "HiKey Development Board";
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
new file mode 100644
index 000..24d3722
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
@@ -0,0 +1,23 @@
+/ {
+   hi6220-ion {
+   compatible = "hisilicon,hi6220-ion";
+
+   heap_sys_user@0 {
+   heap-name = "sys_user";
+   heap-id   = <0x0>;
+   heap-base = <0x0>;
+   heap-size = <0x0>;
+   heap-type = "ion_system";
+   };
+
+   heap_sys_contig@0 {
+   heap-name = "sys_contig";
+   heap-id   = <0x1>;
+   heap-base = <0x0>;
+   heap-size = <0x0>;
+   heap-type = "ion_system_contig";
+   };
+   };
+
+};
+
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] docs: dts: Add documentation for hi6220 SoC ION node

[Chen Feng]

Documentation for hi6220 SoC ION node

Signed-off-by: Chen Feng 
Signed-off-by: Yu Dongbin 
---
 .../devicetree/bindings/staging/ion/hi6220-ion.txt | 27 ++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt

diff --git a/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt 
b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
new file mode 100644
index 000..5f969d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
@@ -0,0 +1,27 @@
+Hi6220 SoC ION
+
+Required properties:
+- compatible : "hisilicon,hi6220-ion"
+- list of the ION heaps
+
+Example:
+hi6220-ion {
+compatible = "hisilicon,hi6220-ion";
+
+heap_sys_user@0 {
+heap-name = "sys_user";
+heap-id   = <0x0>;
+heap-base = <0x0>;
+heap-size = <0x0>;
+heap-type = "ion_system";
+};
+
+heap_sys_contig@0 {
+heap-name = "sys_contig";
+heap-id   = <0x1>;
+heap-base = <0x0>;
+heap-size = <0x0>;
+heap-type = "ion_system_contig";
+};
+};
+
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH V1 2/3] taging: android: ion: Add ion driver for Hi6220 SoC platform

[Chen Feng]

Add ion support for hi6220 SoC platform.

Signed-off-by: Chen Feng 
Signed-off-by: Yu Dongbin 
---
 drivers/staging/android/ion/Kconfig|   7 +
 drivers/staging/android/ion/Makefile   |   1 +
 drivers/staging/android/ion/hisilicon/Kconfig  |   5 +
 drivers/staging/android/ion/hisilicon/Makefile |   1 +
 drivers/staging/android/ion/hisilicon/hi6220_ion.c | 204 +
 5 files changed, 218 insertions(+)
 create mode 100644 drivers/staging/android/ion/hisilicon/Kconfig
 create mode 100644 drivers/staging/android/ion/hisilicon/Makefile
 create mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c

diff --git a/drivers/staging/android/ion/Kconfig 
b/drivers/staging/android/ion/Kconfig
index 3452346..19c1572 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -33,3 +33,10 @@ config ION_TEGRA
help
  Choose this option if you wish to use ion on an nVidia Tegra.
 
+config ION_HISI
+   tristate "Ion for Hisilicon"
+   depends on ARCH_HISI && ION
+   help
+ Choose this option if you wish to use ion on Hisilicon Platform.
+
+source "drivers/staging/android/ion/hisilicon/Kconfig"
diff --git a/drivers/staging/android/ion/Makefile 
b/drivers/staging/android/ion/Makefile
index b56fd2b..18cc2aa 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -7,4 +7,5 @@ endif
 
 obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
 obj-$(CONFIG_ION_TEGRA) += tegra/
+obj-$(CONFIG_ION_HISI) += hisilicon/
 
diff --git a/drivers/staging/android/ion/hisilicon/Kconfig 
b/drivers/staging/android/ion/hisilicon/Kconfig
new file mode 100644
index 000..2b4bd07
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/Kconfig
@@ -0,0 +1,5 @@
+config HI6220_ION
+bool "Hi6220 ION Driver"
+depends on ARCH_HISI && ION
+help
+  Build the Hisilicon Hi6220 ion driver.
diff --git a/drivers/staging/android/ion/hisilicon/Makefile 
b/drivers/staging/android/ion/hisilicon/Makefile
new file mode 100644
index 000..2a89414
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_HI6220_ION) += hi6220_ion.o
diff --git a/drivers/staging/android/ion/hisilicon/hi6220_ion.c 
b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
new file mode 100644
index 000..0e8632a
--- /dev/null
+++ b/drivers/staging/android/ion/hisilicon/hi6220_ion.c
@@ -0,0 +1,204 @@
+#define pr_fmt(fmt) "Ion: " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../ion_priv.h"
+#include "../ion.h"
+
+struct hi6220_ion_type_table {
+   const char *name;
+   enum ion_heap_type type;
+};
+
+static struct hi6220_ion_type_table ion_type_table[] = {
+   {"ion_system", ION_HEAP_TYPE_SYSTEM},
+   {"ion_system_contig", ION_HEAP_TYPE_SYSTEM_CONTIG},
+   {"ion_carveout", ION_HEAP_TYPE_CARVEOUT},
+   {"ion_chunk", ION_HEAP_TYPE_CHUNK},
+   {"ion_dma", ION_HEAP_TYPE_DMA},
+   {"ion_custom", ION_HEAP_TYPE_CUSTOM},
+};
+
+static struct ion_device *idev;
+static int num_heaps;
+static struct ion_heap **heaps;
+static struct ion_platform_heap **heaps_data;
+
+static int get_type_by_name(const char *name, enum ion_heap_type *type)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(ion_type_table); i++) {
+   if (strncmp(name, ion_type_table[i].name, strlen(name)))
+   continue;
+
+   *type = ion_type_table[i].type;
+   return 0;
+   }
+
+   return -EINVAL;
+}
+
+static int hi6220_set_platform_data(struct platform_device *pdev)
+{
+   unsigned int base;
+   unsigned int size;
+   unsigned int id;
+   const char *heap_name;
+   const char *type_name;
+   enum ion_heap_type type;
+   int ret;
+   struct device_node *np;
+   struct ion_platform_heap *p_data;
+   const struct device_node *dt_node = pdev->dev.of_node;
+   int index = 0;
+
+   for_each_child_of_node(dt_node, np)
+   num_heaps++;
+
+   heaps_data = devm_kzalloc(&pdev->dev,
+ sizeof(struct ion_platform_heap *) *
+ num_heaps,
+ GFP_KERNEL);
+   if (!heaps_data)
+   return -ENOMEM;
+
+   for_each_child_of_node(dt_node, np) {
+   ret = of_property_read_string(np, "heap-name", &heap_name);
+   if (ret < 0) {
+   pr_err("check the name of node %s\n", np->name);
+   continue;
+   }
+
+   ret = of_property_read_u32(np, "heap-id", &id);
+   if (ret < 0) {
+   pr_err("check the id %s\n", np->

[PATCH V1 3/3] arm64: dts: Add dts files to enable ION on Hi6220 SoC.

[Chen Feng]

Add ION node to enable ION on hi6220 SoC platform

Signed-off-by: Chen Feng 
Signed-off-by: Yu Dongbin 
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts |  1 +
 arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi  | 20 
 2 files changed, 21 insertions(+)
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts 
b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index e36a539..44b75d2 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -11,6 +11,7 @@
 /memreserve/ 0x05e0 0x0010;
 
 #include "hi6220.dtsi"
+#include "hi6220-ion.dtsi"
 
 / {
model = "HiKey Development Board";
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
new file mode 100644
index 000..c79f2ce
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-ion.dtsi
@@ -0,0 +1,20 @@
+/ {
+   hi6220-ion {
+   compatible = "hisilicon,hi6220-ion";
+   heap_sys_user@0 {
+   heap-name = "sys_user";
+   heap-id   = <0x0>;
+   heap-base = <0x0>;
+   heap-size = <0x0>;
+   heap-type = "ion_system";
+   };
+   heap_sys_contig@0 {
+   heap-name = "sys_contig";
+   heap-id   = <0x1>;
+   heap-base = <0x0>;
+   heap-size = <0x0>;
+   heap-type = "ion_system_contig";
+   };
+   };
+
+};
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH V1 1/3] docs: dts: Add documentation for hi6220 SoC ION node

[Chen Feng]

Documentation for hi6220 SoC ION node

Signed-off-by: Chen Feng 
Signed-off-by: Yu Dongbin 
---
 .../devicetree/bindings/staging/ion/hi6220-ion.txt | 31 ++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt

diff --git a/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt 
b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
new file mode 100644
index 000..97ba233
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
@@ -0,0 +1,31 @@
+Hi6220 SoC ION
+
+Required properties:
+- compatible : "hisilicon,hi6220-ion"
+- list of the ION heaps
+   - heap name : maybe heap_sys_user@0
+   - heap id   : id should be unique in the system.
+   - heap base : base ddr address of the heap,0 means that
+   it is dynamic.
+   - heap size : memory size and 0 means it is dynamic.
+   - heap type : the heap type of the heap, please also
+see the define in ion.h(drivers/staging/android/uapi/ion.h)
+
+Example:
+   hi6220-ion {
+   compatible = "hisilicon,hi6220-ion";
+   heap_sys_user@0 {
+   heap-name = "sys_user";
+   heap-id   = <0x0>;
+   heap-base = <0x0>;
+   heap-size = <0x0>;
+   heap-type = "ion_system";
+   };
+   heap_sys_contig@0 {
+   heap-name = "sys_contig";
+   heap-id   = <0x1>;
+   heap-base = <0x0>;
+   heap-size = <0x0>;
+   heap-type = "ion_system_contig";
+   };
+   };
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting

[Chen Feng]

On 2018/1/9 18:43, Zeng Tao wrote:
> This issue is introduced by the commit  ("ION: Sys_heap:
> Add cached pool to spead up cached buffer alloc"), the gfp_mask low
> order pool is overlapped by the high order inside the loop, so the
> gfp_mask of all pools are set to high_order_gfp_flags.
> 

Thanks
> Signed-off-by: Zeng Tao 
> ---
>  drivers/staging/android/ion/ion_system_heap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> b/drivers/staging/android/ion/ion_system_heap.c
> index 4dc5d7a..b6386be 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -298,10 +298,10 @@ static int ion_system_heap_create_pools(struct 
> ion_page_pool **pools,
>   bool cached)
>  {
>   int i;
> - gfp_t gfp_flags = low_order_gfp_flags;
>  
>   for (i = 0; i < NUM_ORDERS; i++) {
>   struct ion_page_pool *pool;
> + gfp_t gfp_flags = low_order_gfp_flags;

Not define here. Better "gfp_flags = low_order_gfp_flags"
>  
>   if (orders[i] > 4)
>   gfp_flags = high_order_gfp_flags;
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Linaro-mm-sig] [PATCH v3] staging: android: ion: Zero CMA allocated memory

[Chen Feng]

On 2018/1/27 1:48, Liam Mark wrote:
> Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
> the CMA API is now used directly and therefore the allocated memory is no
> longer automatically zeroed.
> 
> Explicitly zero CMA allocated memory to ensure that no data is exposed to
> userspace.
> 
> Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
> Signed-off-by: Liam Mark 
> ---
> Changes in v2:
>   - Clean up the commit message.
>   - Add 'Fixes:'
> 
> Changes in v3:
>  - Add support for highmem pages
> 
>  drivers/staging/android/ion/ion_cma_heap.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
> b/drivers/staging/android/ion/ion_cma_heap.c
> index 86196ffd2faf..fa3e4b7e0c9f 100644
> --- a/drivers/staging/android/ion/ion_cma_heap.c
> +++ b/drivers/staging/android/ion/ion_cma_heap.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "ion.h"
>  
> @@ -51,6 +52,22 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
> ion_buffer *buffer,
>   if (!pages)
>   return -ENOMEM;
>  
> + if (PageHighMem(pages)) {
> + unsigned long nr_clear_pages = nr_pages;
> + struct page *page = pages;
> +
> + while (nr_clear_pages > 0) {
> + void *vaddr = kmap_atomic(page);
> +
> + memset(vaddr, 0, PAGE_SIZE);
> + kunmap_atomic(vaddr);

Here. This way may cause performance latency at mapping-memset-umap page one 
bye one.

Take a look at ion_heap_pages_zero.

Not very critical, arm64 always have linear mapping.


> + page++;
> + nr_clear_pages--;
> + }
> + } else {
> + memset(page_address(pages), 0, size);
> + }
> +
>   table = kmalloc(sizeof(*table), GFP_KERNEL);
>   if (!table)
>   goto err;
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel