* [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages @ 2018-02-07 3:59 Yisheng Xie 2018-02-07 3:59 ` [PATCH 2/2] staging: android: ion: Combine cache and uncache pools Yisheng Xie ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Yisheng Xie @ 2018-02-07 3:59 UTC (permalink / raw) To: gregkh, labbott; +Cc: devel, Yisheng Xie, linux-kernel ion_page_pool_alloc_pages calls alloc_pages to allocate pages for page pools. If alloc_pages return NULL, it will return NULL, or it will return the pages allocate from alloc_pages. So we can just return alloc_pages without any judgement. Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> --- drivers/staging/android/ion/ion_page_pool.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index e3a6e32..6d2caf0 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -11,13 +11,9 @@ #include "ion.h" -static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool) +static inline struct page *ion_page_pool_alloc_pages(struct ion_page_pool *pool) { - struct page *page = alloc_pages(pool->gfp_mask, pool->order); - - if (!page) - return NULL; - return page; + return alloc_pages(pool->gfp_mask, pool->order); } static void ion_page_pool_free_pages(struct ion_page_pool *pool, -- 1.7.12.4 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] staging: android: ion: Combine cache and uncache pools 2018-02-07 3:59 [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie @ 2018-02-07 3:59 ` Yisheng Xie 2018-02-07 14:34 ` Sumit Semwal 2018-02-07 14:35 ` [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Sumit Semwal 2018-02-12 11:41 ` Yisheng Xie 2 siblings, 1 reply; 6+ messages in thread From: Yisheng Xie @ 2018-02-07 3:59 UTC (permalink / raw) To: gregkh, labbott; +Cc: devel, Yisheng Xie, linux-kernel Now we call dma_map in the dma_buf API callbacks and handle explicit caching by the dma_buf sync API, which make cache and uncache pools in the same handling flow, which can be combined. Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> --- drivers/staging/android/ion/ion.c | 5 -- drivers/staging/android/ion/ion.h | 13 +---- drivers/staging/android/ion/ion_page_pool.c | 5 +- drivers/staging/android/ion/ion_system_heap.c | 76 +++++---------------------- 4 files changed, 16 insertions(+), 83 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 461b193..c094be2 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -33,11 +33,6 @@ static struct ion_device *internal_dev; static int heap_id; -bool ion_buffer_cached(struct ion_buffer *buffer) -{ - return !!(buffer->flags & ION_FLAG_CACHED); -} - /* this function should only be called while dev->lock is held */ static void ion_buffer_add(struct ion_device *dev, struct ion_buffer *buffer) diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index 1bc443f..ea08978 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -185,14 +185,6 @@ struct ion_heap { }; /** - * ion_buffer_cached - this ion buffer is cached - * @buffer: buffer - * - * indicates whether this ion buffer is cached - */ -bool ion_buffer_cached(struct ion_buffer *buffer); - -/** * ion_device_add_heap - adds a heap to the ion device * @heap: the heap to add */ @@ -302,7 +294,6 @@ size_t ion_heap_freelist_shrink(struct ion_heap *heap, * @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 @@ -312,7 +303,6 @@ size_t ion_heap_freelist_shrink(struct ion_heap *heap, 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; @@ -321,8 +311,7 @@ struct ion_page_pool { struct plist_node list; }; -struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order, - bool cached); +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order); void ion_page_pool_destroy(struct ion_page_pool *pool); struct page *ion_page_pool_alloc(struct ion_page_pool *pool); void ion_page_pool_free(struct ion_page_pool *pool, struct page *page); diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index 6d2caf0..db8f614 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -123,8 +123,7 @@ 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, - bool cached) +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order) { struct ion_page_pool *pool = kmalloc(sizeof(*pool), GFP_KERNEL); @@ -138,8 +137,6 @@ 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_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index bc19cdd..701eb9f 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -41,31 +41,16 @@ static inline unsigned int order_to_size(int order) struct ion_system_heap { struct ion_heap heap; - struct ion_page_pool *uncached_pools[NUM_ORDERS]; - struct ion_page_pool *cached_pools[NUM_ORDERS]; + struct ion_page_pool *pools[NUM_ORDERS]; }; -/** - * 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; - struct page *page; + struct ion_page_pool *pool = heap->pools[order_to_index(order)]; - 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; + return ion_page_pool_alloc(pool); } static void free_buffer_page(struct ion_system_heap *heap, @@ -73,7 +58,6 @@ static void free_buffer_page(struct ion_system_heap *heap, { struct ion_page_pool *pool; unsigned int order = compound_order(page); - bool cached = ion_buffer_cached(buffer); /* go to system */ if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) { @@ -81,10 +65,7 @@ static void free_buffer_page(struct ion_system_heap *heap, return; } - if (!cached) - pool = heap->uncached_pools[order_to_index(order)]; - else - pool = heap->cached_pools[order_to_index(order)]; + pool = heap->pools[order_to_index(order)]; ion_page_pool_free(pool, page); } @@ -190,8 +171,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer) static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, int nr_to_scan) { - struct ion_page_pool *uncached_pool; - struct ion_page_pool *cached_pool; + struct ion_page_pool *pool; struct ion_system_heap *sys_heap; int nr_total = 0; int i, nr_freed; @@ -203,26 +183,15 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, only_scan = 1; for (i = 0; i < NUM_ORDERS; i++) { - uncached_pool = sys_heap->uncached_pools[i]; - cached_pool = sys_heap->cached_pools[i]; + pool = sys_heap->pools[i]; if (only_scan) { - nr_total += ion_page_pool_shrink(uncached_pool, + nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); - nr_total += ion_page_pool_shrink(cached_pool, - gfp_mask, - nr_to_scan); } else { - nr_freed = ion_page_pool_shrink(uncached_pool, - gfp_mask, - nr_to_scan); - nr_to_scan -= nr_freed; - nr_total += nr_freed; - if (nr_to_scan <= 0) - break; - nr_freed = ion_page_pool_shrink(cached_pool, + nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); nr_to_scan -= nr_freed; @@ -253,26 +222,16 @@ static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s, struct ion_page_pool *pool; for (i = 0; i < NUM_ORDERS; i++) { - pool = sys_heap->uncached_pools[i]; + pool = sys_heap->pools[i]; - seq_printf(s, "%d order %u highmem pages uncached %lu total\n", + seq_printf(s, "%d order %u highmem pages %lu total\n", pool->high_count, pool->order, (PAGE_SIZE << pool->order) * pool->high_count); - seq_printf(s, "%d order %u lowmem pages uncached %lu total\n", + seq_printf(s, "%d order %u lowmem pages %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); - seq_printf(s, "%d order %u lowmem pages cached %lu total\n", - pool->low_count, pool->order, - (PAGE_SIZE << pool->order) * pool->low_count); - } return 0; } @@ -285,8 +244,7 @@ static void ion_system_heap_destroy_pools(struct ion_page_pool **pools) ion_page_pool_destroy(pools[i]); } -static int ion_system_heap_create_pools(struct ion_page_pool **pools, - bool cached) +static int ion_system_heap_create_pools(struct ion_page_pool **pools) { int i; gfp_t gfp_flags = low_order_gfp_flags; @@ -297,7 +255,7 @@ static int ion_system_heap_create_pools(struct ion_page_pool **pools, if (orders[i] > 4) gfp_flags = high_order_gfp_flags; - pool = ion_page_pool_create(gfp_flags, orders[i], cached); + pool = ion_page_pool_create(gfp_flags, orders[i]); if (!pool) goto err_create_pool; pools[i] = pool; @@ -320,18 +278,12 @@ static struct ion_heap *__ion_system_heap_create(void) heap->heap.type = ION_HEAP_TYPE_SYSTEM; heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; - if (ion_system_heap_create_pools(heap->uncached_pools, false)) + if (ion_system_heap_create_pools(heap->pools)) goto free_heap; - if (ion_system_heap_create_pools(heap->cached_pools, true)) - goto destroy_uncached_pools; - heap->heap.debug_show = ion_system_heap_debug_show; return &heap->heap; -destroy_uncached_pools: - ion_system_heap_destroy_pools(heap->uncached_pools); - free_heap: kfree(heap); return ERR_PTR(-ENOMEM); -- 1.7.12.4 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] staging: android: ion: Combine cache and uncache pools 2018-02-07 3:59 ` [PATCH 2/2] staging: android: ion: Combine cache and uncache pools Yisheng Xie @ 2018-02-07 14:34 ` Sumit Semwal 2018-02-08 0:53 ` Yisheng Xie 0 siblings, 1 reply; 6+ messages in thread From: Sumit Semwal @ 2018-02-07 14:34 UTC (permalink / raw) To: Yisheng Xie; +Cc: Greg Kroah-Hartman, Laura Abbott, devel, LKML Hello Yisheng, On 7 February 2018 at 09:29, Yisheng Xie <xieyisheng1@huawei.com> wrote: > Now we call dma_map in the dma_buf API callbacks and handle explicit > caching by the dma_buf sync API, which make cache and uncache pools > in the same handling flow, which can be combined. > Thanks for the patch! Perhaps you should also put the version history here, to capture the changes from previous versions? > Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> With that done, please feel free to add Acked-by: Sumit Semwal <sumit.semwal@linaro.org> > --- > drivers/staging/android/ion/ion.c | 5 -- > drivers/staging/android/ion/ion.h | 13 +---- > drivers/staging/android/ion/ion_page_pool.c | 5 +- > drivers/staging/android/ion/ion_system_heap.c | 76 +++++---------------------- > 4 files changed, 16 insertions(+), 83 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index 461b193..c094be2 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -33,11 +33,6 @@ > static struct ion_device *internal_dev; > static int heap_id; > > -bool ion_buffer_cached(struct ion_buffer *buffer) > -{ > - return !!(buffer->flags & ION_FLAG_CACHED); > -} > - > /* this function should only be called while dev->lock is held */ > static void ion_buffer_add(struct ion_device *dev, > struct ion_buffer *buffer) > diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h > index 1bc443f..ea08978 100644 > --- a/drivers/staging/android/ion/ion.h > +++ b/drivers/staging/android/ion/ion.h > @@ -185,14 +185,6 @@ struct ion_heap { > }; > > /** > - * ion_buffer_cached - this ion buffer is cached > - * @buffer: buffer > - * > - * indicates whether this ion buffer is cached > - */ > -bool ion_buffer_cached(struct ion_buffer *buffer); > - > -/** > * ion_device_add_heap - adds a heap to the ion device > * @heap: the heap to add > */ > @@ -302,7 +294,6 @@ size_t ion_heap_freelist_shrink(struct ion_heap *heap, > * @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 > @@ -312,7 +303,6 @@ size_t ion_heap_freelist_shrink(struct ion_heap *heap, > 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; > @@ -321,8 +311,7 @@ struct ion_page_pool { > struct plist_node list; > }; > > -struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order, > - bool cached); > +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order); > void ion_page_pool_destroy(struct ion_page_pool *pool); > struct page *ion_page_pool_alloc(struct ion_page_pool *pool); > void ion_page_pool_free(struct ion_page_pool *pool, struct page *page); > diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c > index 6d2caf0..db8f614 100644 > --- a/drivers/staging/android/ion/ion_page_pool.c > +++ b/drivers/staging/android/ion/ion_page_pool.c > @@ -123,8 +123,7 @@ 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, > - bool cached) > +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order) > { > struct ion_page_pool *pool = kmalloc(sizeof(*pool), GFP_KERNEL); > > @@ -138,8 +137,6 @@ 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_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index bc19cdd..701eb9f 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -41,31 +41,16 @@ static inline unsigned int order_to_size(int order) > > struct ion_system_heap { > struct ion_heap heap; > - struct ion_page_pool *uncached_pools[NUM_ORDERS]; > - struct ion_page_pool *cached_pools[NUM_ORDERS]; > + struct ion_page_pool *pools[NUM_ORDERS]; > }; > > -/** > - * 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; > - struct page *page; > + struct ion_page_pool *pool = heap->pools[order_to_index(order)]; > > - 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; > + return ion_page_pool_alloc(pool); > } > > static void free_buffer_page(struct ion_system_heap *heap, > @@ -73,7 +58,6 @@ static void free_buffer_page(struct ion_system_heap *heap, > { > struct ion_page_pool *pool; > unsigned int order = compound_order(page); > - bool cached = ion_buffer_cached(buffer); > > /* go to system */ > if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) { > @@ -81,10 +65,7 @@ static void free_buffer_page(struct ion_system_heap *heap, > return; > } > > - if (!cached) > - pool = heap->uncached_pools[order_to_index(order)]; > - else > - pool = heap->cached_pools[order_to_index(order)]; > + pool = heap->pools[order_to_index(order)]; > > ion_page_pool_free(pool, page); > } > @@ -190,8 +171,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer) > static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, > int nr_to_scan) > { > - struct ion_page_pool *uncached_pool; > - struct ion_page_pool *cached_pool; > + struct ion_page_pool *pool; > struct ion_system_heap *sys_heap; > int nr_total = 0; > int i, nr_freed; > @@ -203,26 +183,15 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, > only_scan = 1; > > for (i = 0; i < NUM_ORDERS; i++) { > - uncached_pool = sys_heap->uncached_pools[i]; > - cached_pool = sys_heap->cached_pools[i]; > + pool = sys_heap->pools[i]; > > if (only_scan) { > - nr_total += ion_page_pool_shrink(uncached_pool, > + nr_total += ion_page_pool_shrink(pool, > gfp_mask, > nr_to_scan); > > - nr_total += ion_page_pool_shrink(cached_pool, > - gfp_mask, > - nr_to_scan); > } else { > - nr_freed = ion_page_pool_shrink(uncached_pool, > - gfp_mask, > - nr_to_scan); > - nr_to_scan -= nr_freed; > - nr_total += nr_freed; > - if (nr_to_scan <= 0) > - break; > - nr_freed = ion_page_pool_shrink(cached_pool, > + nr_freed = ion_page_pool_shrink(pool, > gfp_mask, > nr_to_scan); > nr_to_scan -= nr_freed; > @@ -253,26 +222,16 @@ static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s, > struct ion_page_pool *pool; > > for (i = 0; i < NUM_ORDERS; i++) { > - pool = sys_heap->uncached_pools[i]; > + pool = sys_heap->pools[i]; > > - seq_printf(s, "%d order %u highmem pages uncached %lu total\n", > + seq_printf(s, "%d order %u highmem pages %lu total\n", > pool->high_count, pool->order, > (PAGE_SIZE << pool->order) * pool->high_count); > - seq_printf(s, "%d order %u lowmem pages uncached %lu total\n", > + seq_printf(s, "%d order %u lowmem pages %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); > - seq_printf(s, "%d order %u lowmem pages cached %lu total\n", > - pool->low_count, pool->order, > - (PAGE_SIZE << pool->order) * pool->low_count); > - } > return 0; > } > > @@ -285,8 +244,7 @@ static void ion_system_heap_destroy_pools(struct ion_page_pool **pools) > ion_page_pool_destroy(pools[i]); > } > > -static int ion_system_heap_create_pools(struct ion_page_pool **pools, > - bool cached) > +static int ion_system_heap_create_pools(struct ion_page_pool **pools) > { > int i; > gfp_t gfp_flags = low_order_gfp_flags; > @@ -297,7 +255,7 @@ static int ion_system_heap_create_pools(struct ion_page_pool **pools, > if (orders[i] > 4) > gfp_flags = high_order_gfp_flags; > > - pool = ion_page_pool_create(gfp_flags, orders[i], cached); > + pool = ion_page_pool_create(gfp_flags, orders[i]); > if (!pool) > goto err_create_pool; > pools[i] = pool; > @@ -320,18 +278,12 @@ static struct ion_heap *__ion_system_heap_create(void) > heap->heap.type = ION_HEAP_TYPE_SYSTEM; > heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; > > - if (ion_system_heap_create_pools(heap->uncached_pools, false)) > + if (ion_system_heap_create_pools(heap->pools)) > goto free_heap; > > - if (ion_system_heap_create_pools(heap->cached_pools, true)) > - goto destroy_uncached_pools; > - > heap->heap.debug_show = ion_system_heap_debug_show; > return &heap->heap; > > -destroy_uncached_pools: > - ion_system_heap_destroy_pools(heap->uncached_pools); > - > free_heap: > kfree(heap); > return ERR_PTR(-ENOMEM); > -- > 1.7.12.4 > -- Thanks and regards, Sumit Semwal Linaro Mobile Group - Kernel Team Lead Linaro.org │ Open source software for ARM SoCs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] staging: android: ion: Combine cache and uncache pools 2018-02-07 14:34 ` Sumit Semwal @ 2018-02-08 0:53 ` Yisheng Xie 0 siblings, 0 replies; 6+ messages in thread From: Yisheng Xie @ 2018-02-08 0:53 UTC (permalink / raw) To: Sumit Semwal; +Cc: devel, Greg Kroah-Hartman, LKML Hi Sumit, On 2018/2/7 22:34, Sumit Semwal wrote: > Hello Yisheng, > > On 7 February 2018 at 09:29, Yisheng Xie <xieyisheng1@huawei.com> wrote: >> Now we call dma_map in the dma_buf API callbacks and handle explicit >> caching by the dma_buf sync API, which make cache and uncache pools >> in the same handling flow, which can be combined. >> > Thanks for the patch! Perhaps you should also put the version history > here, to capture the changes from previous versions? hmm, this is first version of this patch. Except for the resent ones, all of the patches I have sent about ion is v1 with different cleanup. Maybe I can send them into one patchset in v2 after merge window. Sorry for not having put them together. Thanks Yisheng > >> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> > With that done, please feel free to add > Acked-by: Sumit Semwal <sumit.semwal@linaro.org> > >> --- >> drivers/staging/android/ion/ion.c | 5 -- >> drivers/staging/android/ion/ion.h | 13 +---- >> drivers/staging/android/ion/ion_page_pool.c | 5 +- >> drivers/staging/android/ion/ion_system_heap.c | 76 +++++---------------------- >> 4 files changed, 16 insertions(+), 83 deletions(-) >> >> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c >> index 461b193..c094be2 100644 >> --- a/drivers/staging/android/ion/ion.c >> +++ b/drivers/staging/android/ion/ion.c >> @@ -33,11 +33,6 @@ >> static struct ion_device *internal_dev; >> static int heap_id; >> >> -bool ion_buffer_cached(struct ion_buffer *buffer) >> -{ >> - return !!(buffer->flags & ION_FLAG_CACHED); >> -} >> - >> /* this function should only be called while dev->lock is held */ >> static void ion_buffer_add(struct ion_device *dev, >> struct ion_buffer *buffer) >> diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h >> index 1bc443f..ea08978 100644 >> --- a/drivers/staging/android/ion/ion.h >> +++ b/drivers/staging/android/ion/ion.h >> @@ -185,14 +185,6 @@ struct ion_heap { >> }; >> >> /** >> - * ion_buffer_cached - this ion buffer is cached >> - * @buffer: buffer >> - * >> - * indicates whether this ion buffer is cached >> - */ >> -bool ion_buffer_cached(struct ion_buffer *buffer); >> - >> -/** >> * ion_device_add_heap - adds a heap to the ion device >> * @heap: the heap to add >> */ >> @@ -302,7 +294,6 @@ size_t ion_heap_freelist_shrink(struct ion_heap *heap, >> * @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 >> @@ -312,7 +303,6 @@ size_t ion_heap_freelist_shrink(struct ion_heap *heap, >> 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; >> @@ -321,8 +311,7 @@ struct ion_page_pool { >> struct plist_node list; >> }; >> >> -struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order, >> - bool cached); >> +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order); >> void ion_page_pool_destroy(struct ion_page_pool *pool); >> struct page *ion_page_pool_alloc(struct ion_page_pool *pool); >> void ion_page_pool_free(struct ion_page_pool *pool, struct page *page); >> diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c >> index 6d2caf0..db8f614 100644 >> --- a/drivers/staging/android/ion/ion_page_pool.c >> +++ b/drivers/staging/android/ion/ion_page_pool.c >> @@ -123,8 +123,7 @@ 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, >> - bool cached) >> +struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order) >> { >> struct ion_page_pool *pool = kmalloc(sizeof(*pool), GFP_KERNEL); >> >> @@ -138,8 +137,6 @@ 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_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c >> index bc19cdd..701eb9f 100644 >> --- a/drivers/staging/android/ion/ion_system_heap.c >> +++ b/drivers/staging/android/ion/ion_system_heap.c >> @@ -41,31 +41,16 @@ static inline unsigned int order_to_size(int order) >> >> struct ion_system_heap { >> struct ion_heap heap; >> - struct ion_page_pool *uncached_pools[NUM_ORDERS]; >> - struct ion_page_pool *cached_pools[NUM_ORDERS]; >> + struct ion_page_pool *pools[NUM_ORDERS]; >> }; >> >> -/** >> - * 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; >> - struct page *page; >> + struct ion_page_pool *pool = heap->pools[order_to_index(order)]; >> >> - 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; >> + return ion_page_pool_alloc(pool); >> } >> >> static void free_buffer_page(struct ion_system_heap *heap, >> @@ -73,7 +58,6 @@ static void free_buffer_page(struct ion_system_heap *heap, >> { >> struct ion_page_pool *pool; >> unsigned int order = compound_order(page); >> - bool cached = ion_buffer_cached(buffer); >> >> /* go to system */ >> if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) { >> @@ -81,10 +65,7 @@ static void free_buffer_page(struct ion_system_heap *heap, >> return; >> } >> >> - if (!cached) >> - pool = heap->uncached_pools[order_to_index(order)]; >> - else >> - pool = heap->cached_pools[order_to_index(order)]; >> + pool = heap->pools[order_to_index(order)]; >> >> ion_page_pool_free(pool, page); >> } >> @@ -190,8 +171,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer) >> static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, >> int nr_to_scan) >> { >> - struct ion_page_pool *uncached_pool; >> - struct ion_page_pool *cached_pool; >> + struct ion_page_pool *pool; >> struct ion_system_heap *sys_heap; >> int nr_total = 0; >> int i, nr_freed; >> @@ -203,26 +183,15 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, >> only_scan = 1; >> >> for (i = 0; i < NUM_ORDERS; i++) { >> - uncached_pool = sys_heap->uncached_pools[i]; >> - cached_pool = sys_heap->cached_pools[i]; >> + pool = sys_heap->pools[i]; >> >> if (only_scan) { >> - nr_total += ion_page_pool_shrink(uncached_pool, >> + nr_total += ion_page_pool_shrink(pool, >> gfp_mask, >> nr_to_scan); >> >> - nr_total += ion_page_pool_shrink(cached_pool, >> - gfp_mask, >> - nr_to_scan); >> } else { >> - nr_freed = ion_page_pool_shrink(uncached_pool, >> - gfp_mask, >> - nr_to_scan); >> - nr_to_scan -= nr_freed; >> - nr_total += nr_freed; >> - if (nr_to_scan <= 0) >> - break; >> - nr_freed = ion_page_pool_shrink(cached_pool, >> + nr_freed = ion_page_pool_shrink(pool, >> gfp_mask, >> nr_to_scan); >> nr_to_scan -= nr_freed; >> @@ -253,26 +222,16 @@ static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s, >> struct ion_page_pool *pool; >> >> for (i = 0; i < NUM_ORDERS; i++) { >> - pool = sys_heap->uncached_pools[i]; >> + pool = sys_heap->pools[i]; >> >> - seq_printf(s, "%d order %u highmem pages uncached %lu total\n", >> + seq_printf(s, "%d order %u highmem pages %lu total\n", >> pool->high_count, pool->order, >> (PAGE_SIZE << pool->order) * pool->high_count); >> - seq_printf(s, "%d order %u lowmem pages uncached %lu total\n", >> + seq_printf(s, "%d order %u lowmem pages %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); >> - seq_printf(s, "%d order %u lowmem pages cached %lu total\n", >> - pool->low_count, pool->order, >> - (PAGE_SIZE << pool->order) * pool->low_count); >> - } >> return 0; >> } >> >> @@ -285,8 +244,7 @@ static void ion_system_heap_destroy_pools(struct ion_page_pool **pools) >> ion_page_pool_destroy(pools[i]); >> } >> >> -static int ion_system_heap_create_pools(struct ion_page_pool **pools, >> - bool cached) >> +static int ion_system_heap_create_pools(struct ion_page_pool **pools) >> { >> int i; >> gfp_t gfp_flags = low_order_gfp_flags; >> @@ -297,7 +255,7 @@ static int ion_system_heap_create_pools(struct ion_page_pool **pools, >> if (orders[i] > 4) >> gfp_flags = high_order_gfp_flags; >> >> - pool = ion_page_pool_create(gfp_flags, orders[i], cached); >> + pool = ion_page_pool_create(gfp_flags, orders[i]); >> if (!pool) >> goto err_create_pool; >> pools[i] = pool; >> @@ -320,18 +278,12 @@ static struct ion_heap *__ion_system_heap_create(void) >> heap->heap.type = ION_HEAP_TYPE_SYSTEM; >> heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; >> >> - if (ion_system_heap_create_pools(heap->uncached_pools, false)) >> + if (ion_system_heap_create_pools(heap->pools)) >> goto free_heap; >> >> - if (ion_system_heap_create_pools(heap->cached_pools, true)) >> - goto destroy_uncached_pools; >> - >> heap->heap.debug_show = ion_system_heap_debug_show; >> return &heap->heap; >> >> -destroy_uncached_pools: >> - ion_system_heap_destroy_pools(heap->uncached_pools); >> - >> free_heap: >> kfree(heap); >> return ERR_PTR(-ENOMEM); >> -- >> 1.7.12.4 >> > > > _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages 2018-02-07 3:59 [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie 2018-02-07 3:59 ` [PATCH 2/2] staging: android: ion: Combine cache and uncache pools Yisheng Xie @ 2018-02-07 14:35 ` Sumit Semwal 2018-02-12 11:41 ` Yisheng Xie 2 siblings, 0 replies; 6+ messages in thread From: Sumit Semwal @ 2018-02-07 14:35 UTC (permalink / raw) To: Yisheng Xie; +Cc: Greg Kroah-Hartman, Laura Abbott, devel, LKML Hi Yisheng, On 7 February 2018 at 09:29, Yisheng Xie <xieyisheng1@huawei.com> wrote: > ion_page_pool_alloc_pages calls alloc_pages to allocate pages for page > pools. If alloc_pages return NULL, it will return NULL, or it will > return the pages allocate from alloc_pages. So we can just return > alloc_pages without any judgement. > With patch history comment addressed (same as the other patch you sent), > Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> Please feel free to add my Acked-by: Sumit Semwal <sumit.semwal@linaro.org> > --- > drivers/staging/android/ion/ion_page_pool.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c > index e3a6e32..6d2caf0 100644 > --- a/drivers/staging/android/ion/ion_page_pool.c > +++ b/drivers/staging/android/ion/ion_page_pool.c > @@ -11,13 +11,9 @@ > > #include "ion.h" > > -static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool) > +static inline struct page *ion_page_pool_alloc_pages(struct ion_page_pool *pool) > { > - struct page *page = alloc_pages(pool->gfp_mask, pool->order); > - > - if (!page) > - return NULL; > - return page; > + return alloc_pages(pool->gfp_mask, pool->order); > } > > static void ion_page_pool_free_pages(struct ion_page_pool *pool, > -- > 1.7.12.4 > Best, Sumit. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages 2018-02-07 3:59 [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie 2018-02-07 3:59 ` [PATCH 2/2] staging: android: ion: Combine cache and uncache pools Yisheng Xie 2018-02-07 14:35 ` [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Sumit Semwal @ 2018-02-12 11:41 ` Yisheng Xie 2 siblings, 0 replies; 6+ messages in thread From: Yisheng Xie @ 2018-02-12 11:41 UTC (permalink / raw) To: gregkh, labbott; +Cc: devel, linux-kernel Hi Greg, JFYI, I have rebase this patchset to v4.15-rc1.[1] [1] https://lkml.org/lkml/2018/2/12/204 Thanks Yisheng On 2018/2/7 11:59, Yisheng Xie wrote: > ion_page_pool_alloc_pages calls alloc_pages to allocate pages for page > pools. If alloc_pages return NULL, it will return NULL, or it will > return the pages allocate from alloc_pages. So we can just return > alloc_pages without any judgement. > > Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> > --- > drivers/staging/android/ion/ion_page_pool.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c > index e3a6e32..6d2caf0 100644 > --- a/drivers/staging/android/ion/ion_page_pool.c > +++ b/drivers/staging/android/ion/ion_page_pool.c > @@ -11,13 +11,9 @@ > > #include "ion.h" > > -static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool) > +static inline struct page *ion_page_pool_alloc_pages(struct ion_page_pool *pool) > { > - struct page *page = alloc_pages(pool->gfp_mask, pool->order); > - > - if (!page) > - return NULL; > - return page; > + return alloc_pages(pool->gfp_mask, pool->order); > } > > static void ion_page_pool_free_pages(struct ion_page_pool *pool, > _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-12 11:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-07 3:59 [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie 2018-02-07 3:59 ` [PATCH 2/2] staging: android: ion: Combine cache and uncache pools Yisheng Xie 2018-02-07 14:34 ` Sumit Semwal 2018-02-08 0:53 ` Yisheng Xie 2018-02-07 14:35 ` [PATCH 1/2] staging: android: ion: Cleanup ion_page_pool_alloc_pages Sumit Semwal 2018-02-12 11:41 ` Yisheng Xie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.