All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] staging: android: ion: Some cleanup about ion
@ 2018-02-12 10:43 Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 1/9] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings Yisheng Xie
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

Hi all,

When I tried to review the code of ion, I find that there are many place
can be cleanup, so I post this patchset.

I mark it as v2, because I have post them in different patchset before[1][2]
[3][4]. And now, I combine them in one patchset to make it easy to review,
Meanwhile, I add some acks and rebase them to v4.15-rc1.

Thanks.

[1] https://lkml.org/lkml/2018/1/31/711
[2] https://lkml.org/lkml/2018/2/1/240
[3] https://lkml.org/lkml/2018/2/4/320
[4] https://lkml.org/lkml/2018/2/6/949

Yisheng Xie (9):
  staging: android: ion: Remove unused declaration
    ion_buffer_fault_user_mappings
  staging: android: ion: Remove unused include files for
    ion_page_pool.c
  staging: android: ion: Nuke ion_page_pool_init
  staging: android: ion: Avoid NULL point in error path
  staging: android: ion: Remove lable debugfs_done
  staging: android: ion: Remove dead code in ion_page_pool_free
  staging: android: ion: Return void instead of int
  staging: android: ion: Cleanup ion_page_pool_alloc_pages
  staging: android: ion: Combine cache and uncache pools

 drivers/staging/android/ion/ion.c             | 20 ++-----
 drivers/staging/android/ion/ion.h             | 22 +-------
 drivers/staging/android/ion/ion_page_pool.c   | 33 ++----------
 drivers/staging/android/ion/ion_system_heap.c | 76 +++++----------------------
 4 files changed, 24 insertions(+), 127 deletions(-)

-- 
1.7.12.4

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/9] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 2/9] staging: android: ion: Remove unused include files for ion_page_pool.c Yisheng Xie
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

ion_buffer_fault_user_mappings's definition has been removed and not be
used anymore, just remove its useless declaration.

Acked-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index a238f23..1bc443f 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -193,15 +193,6 @@ struct ion_heap {
 bool ion_buffer_cached(struct ion_buffer *buffer);
 
 /**
- * ion_buffer_fault_user_mappings - fault in user mappings of this buffer
- * @buffer:		buffer
- *
- * indicates whether userspace mappings of this buffer will be faulted
- * in, this can affect how buffers are allocated from the heap.
- */
-bool ion_buffer_fault_user_mappings(struct ion_buffer *buffer);
-
-/**
  * ion_device_add_heap - adds a heap to the ion device
  * @heap:		the heap to add
  */
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/9] staging: android: ion: Remove unused include files for ion_page_pool.c
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 1/9] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 3/9] staging: android: ion: Nuke ion_page_pool_init Yisheng Xie
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

After rewrite of ion_page_pool, some of its include file is no need
anymore, just remove it.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion_page_pool.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index b3017f1..f0847b3 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -5,10 +5,6 @@
  * Copyright (C) 2011 Google, Inc.
  */
 
-#include <linux/debugfs.h>
-#include <linux/dma-mapping.h>
-#include <linux/err.h>
-#include <linux/fs.h>
 #include <linux/list.h>
 #include <linux/init.h>
 #include <linux/slab.h>
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/9] staging: android: ion: Nuke ion_page_pool_init
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 1/9] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 2/9] staging: android: ion: Remove unused include files for ion_page_pool.c Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path Yisheng Xie
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

ion_page_pool.c now is used to apply pool APIs for system heap, which do
not need do any initial at device_initcall. Therefore ion_page_pool_init
can be nuked.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion_page_pool.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index f0847b3..4452e28 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -6,7 +6,6 @@
  */
 
 #include <linux/list.h>
-#include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
 
@@ -158,9 +157,3 @@ void ion_page_pool_destroy(struct ion_page_pool *pool)
 {
 	kfree(pool);
 }
-
-static int __init ion_page_pool_init(void)
-{
-	return 0;
-}
-device_initcall(ion_page_pool_init);
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (2 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 3/9] staging: android: ion: Nuke ion_page_pool_init Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-16 16:27   ` Greg KH
  2018-02-12 10:43 ` [PATCH v2 5/9] staging: android: ion: Remove lable debugfs_done Yisheng Xie
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

If we failed to create debugfs for ion at ion_device_create, the
debug_root of ion_device will be NULL, and then when try to create debug
file for shrinker of heap it will be create on the top of debugfs. If we
also failed to create this the debug file, it call dentry_path to found
the path of debug_root, then a NULL point will occur.

Fix this by avoiding call dentry_path, but show the debug name only when
failed to create debug file for shrinker.

Acked-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 57e0d80..4b69372 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -564,13 +564,9 @@ void ion_device_add_heap(struct ion_heap *heap)
 		debug_file = debugfs_create_file(debug_name,
 						 0644, dev->debug_root, heap,
 						 &debug_shrink_fops);
-		if (!debug_file) {
-			char buf[256], *path;
-
-			path = dentry_path(dev->debug_root, buf, 256);
-			pr_err("Failed to create heap shrinker debugfs at %s/%s\n",
-			       path, debug_name);
-		}
+		if (!debug_file)
+			pr_err("Failed to create ion heap shrinker debugfs at %s\n",
+			       debug_name);
 	}
 
 	dev->heap_cnt++;
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/9] staging: android: ion: Remove lable debugfs_done
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (3 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-16 16:27   ` Greg KH
  2018-02-12 10:43 ` [PATCH v2 6/9] staging: android: ion: Remove dead code in ion_page_pool_free Yisheng Xie
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

When failed to create debug_root, we will go on initail other part of
ion, so we can just info this message to user and do not need a lable
to jump.

Acked-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 4b69372..461b193 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -595,12 +595,9 @@ static int ion_device_create(void)
 	}
 
 	idev->debug_root = debugfs_create_dir("ion", NULL);
-	if (!idev->debug_root) {
+	if (!idev->debug_root)
 		pr_err("ion: failed to create debugfs root directory.\n");
-		goto debugfs_done;
-	}
 
-debugfs_done:
 	idev->buffers = RB_ROOT;
 	mutex_init(&idev->buffer_lock);
 	init_rwsem(&idev->lock);
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/9] staging: android: ion: Remove dead code in ion_page_pool_free
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (4 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 5/9] staging: android: ion: Remove lable debugfs_done Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 7/9] staging: android: ion: Return void instead of int Yisheng Xie
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

ion_page_pool_add will always return 0, however ion_page_pool_free will
call ion_page_pool_free_pages when ion_page_pool_add's return value is
not 0, so it is a dead code which can be removed.

Acked-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion_page_pool.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 4452e28..150626f 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -79,13 +79,9 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
 
 void ion_page_pool_free(struct ion_page_pool *pool, struct page *page)
 {
-	int ret;
-
 	BUG_ON(pool->order != compound_order(page));
 
-	ret = ion_page_pool_add(pool, page);
-	if (ret)
-		ion_page_pool_free_pages(pool, page);
+	ion_page_pool_add(pool, page);
 }
 
 static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/9] staging: android: ion: Return void instead of int
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (5 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 6/9] staging: android: ion: Remove dead code in ion_page_pool_free Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 8/9] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +Cc: devel, Yisheng Xie, linux-kernel

Now, nobody care about the return value of ion_page_pool_add, therefore
we can just make it return void.

Acked-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/staging/android/ion/ion_page_pool.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 150626f..e3a6e32 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -26,7 +26,7 @@ static void ion_page_pool_free_pages(struct ion_page_pool *pool,
 	__free_pages(page, pool->order);
 }
 
-static int ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
+static void ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
 {
 	mutex_lock(&pool->mutex);
 	if (PageHighMem(page)) {
@@ -37,7 +37,6 @@ static int ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
 		pool->low_count++;
 	}
 	mutex_unlock(&pool->mutex);
-	return 0;
 }
 
 static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high)
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 8/9] staging: android: ion: Cleanup ion_page_pool_alloc_pages
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (6 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 7/9] staging: android: ion: Return void instead of int Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 10:43 ` [PATCH v2 9/9] staging: android: ion: Combine cache and uncache pools Yisheng Xie
  2018-02-12 11:17 ` [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Dan Carpenter
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +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.

Acked-by: Sumit Semwal <sumit.semwal@linaro.org>
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] 16+ messages in thread

* [PATCH v2 9/9] staging: android: ion: Combine cache and uncache pools
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (7 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 8/9] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie
@ 2018-02-12 10:43 ` Yisheng Xie
  2018-02-12 11:17 ` [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Dan Carpenter
  9 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 10:43 UTC (permalink / raw)
  To: gregkh, labbott, sumit.semwal; +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.

Acked-by: Sumit Semwal <sumit.semwal@linaro.org>
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] 16+ messages in thread

* Re: [PATCH v2 0/9] staging: android: ion: Some cleanup about ion
  2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
                   ` (8 preceding siblings ...)
  2018-02-12 10:43 ` [PATCH v2 9/9] staging: android: ion: Combine cache and uncache pools Yisheng Xie
@ 2018-02-12 11:17 ` Dan Carpenter
  2018-02-12 11:33   ` Yisheng Xie
  9 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2018-02-12 11:17 UTC (permalink / raw)
  To: Yisheng Xie; +Cc: devel, gregkh, sumit.semwal, linux-kernel

On Mon, Feb 12, 2018 at 06:43:05PM +0800, Yisheng Xie wrote:
> Hi all,
> 
> When I tried to review the code of ion, I find that there are many place
> can be cleanup, so I post this patchset.
> 
> I mark it as v2, because I have post them in different patchset before[1][2]
> [3][4]. And now, I combine them in one patchset to make it easy to review,
> Meanwhile, I add some acks and rebase them to v4.15-rc1.
> 
> Thanks.
> 
> [1] https://lkml.org/lkml/2018/1/31/711
> [2] https://lkml.org/lkml/2018/2/1/240
> [3] https://lkml.org/lkml/2018/2/4/320
> [4] https://lkml.org/lkml/2018/2/6/949
> 

Could you go back and reply to these threads that you have redone the
patches?  Otherwise, what's likely to happen is that Greg will apply
as many as he can get to this series and it won't apply.

regards,
dan carpenter

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/9] staging: android: ion: Some cleanup about ion
  2018-02-12 11:17 ` [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Dan Carpenter
@ 2018-02-12 11:33   ` Yisheng Xie
  0 siblings, 0 replies; 16+ messages in thread
From: Yisheng Xie @ 2018-02-12 11:33 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: gregkh, labbott, sumit.semwal, devel, linux-kernel

Hi Dan,

On 2018/2/12 19:17, Dan Carpenter wrote:
> On Mon, Feb 12, 2018 at 06:43:05PM +0800, Yisheng Xie wrote:
>> Hi all,
>>
>> When I tried to review the code of ion, I find that there are many place
>> can be cleanup, so I post this patchset.
>>
>> I mark it as v2, because I have post them in different patchset before[1][2]
>> [3][4]. And now, I combine them in one patchset to make it easy to review,
>> Meanwhile, I add some acks and rebase them to v4.15-rc1.
>>
>> Thanks.
>>
>> [1] https://lkml.org/lkml/2018/1/31/711
>> [2] https://lkml.org/lkml/2018/2/1/240
>> [3] https://lkml.org/lkml/2018/2/4/320
>> [4] https://lkml.org/lkml/2018/2/6/949
>>
> 
> Could you go back and reply to these threads that you have redone the
> patches?  Otherwise, what's likely to happen is that Greg will apply
> as many as he can get to this series and it won't apply.

Sure! thanks

Yisheng
> 
> regards,
> dan carpenter
> 
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path
  2018-02-12 10:43 ` [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path Yisheng Xie
@ 2018-02-16 16:27   ` Greg KH
  2018-02-22  8:59     ` Yisheng Xie
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2018-02-16 16:27 UTC (permalink / raw)
  To: Yisheng Xie; +Cc: devel, sumit.semwal, linux-kernel

On Mon, Feb 12, 2018 at 06:43:09PM +0800, Yisheng Xie wrote:
> If we failed to create debugfs for ion at ion_device_create, the
> debug_root of ion_device will be NULL, and then when try to create debug
> file for shrinker of heap it will be create on the top of debugfs. If we
> also failed to create this the debug file, it call dentry_path to found
> the path of debug_root, then a NULL point will occur.
> 
> Fix this by avoiding call dentry_path, but show the debug name only when
> failed to create debug file for shrinker.
> 
> Acked-by: Laura Abbott <labbott@redhat.com>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
>  drivers/staging/android/ion/ion.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 57e0d80..4b69372 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -564,13 +564,9 @@ void ion_device_add_heap(struct ion_heap *heap)
>  		debug_file = debugfs_create_file(debug_name,
>  						 0644, dev->debug_root, heap,
>  						 &debug_shrink_fops);
> -		if (!debug_file) {
> -			char buf[256], *path;
> -
> -			path = dentry_path(dev->debug_root, buf, 256);
> -			pr_err("Failed to create heap shrinker debugfs at %s/%s\n",
> -			       path, debug_name);
> -		}
> +		if (!debug_file)
> +			pr_err("Failed to create ion heap shrinker debugfs at %s\n",
> +			       debug_name);

Really we can just remove this, there's no need to check the return
value of this debugfs call at all, it doesn't matter.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/9] staging: android: ion: Remove lable debugfs_done
  2018-02-12 10:43 ` [PATCH v2 5/9] staging: android: ion: Remove lable debugfs_done Yisheng Xie
@ 2018-02-16 16:27   ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2018-02-16 16:27 UTC (permalink / raw)
  To: Yisheng Xie; +Cc: devel, sumit.semwal, linux-kernel

On Mon, Feb 12, 2018 at 06:43:10PM +0800, Yisheng Xie wrote:
> When failed to create debug_root, we will go on initail other part of
> ion, so we can just info this message to user and do not need a lable
> to jump.
> 
> Acked-by: Laura Abbott <labbott@redhat.com>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
>  drivers/staging/android/ion/ion.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 4b69372..461b193 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -595,12 +595,9 @@ static int ion_device_create(void)
>  	}
>  
>  	idev->debug_root = debugfs_create_dir("ion", NULL);
> -	if (!idev->debug_root) {
> +	if (!idev->debug_root)
>  		pr_err("ion: failed to create debugfs root directory.\n");
> -		goto debugfs_done;
> -	}

Again, does not matter to check the return value of any debugfs call.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path
  2018-02-16 16:27   ` Greg KH
@ 2018-02-22  8:59     ` Yisheng Xie
  2018-02-22  9:13       ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Yisheng Xie @ 2018-02-22  8:59 UTC (permalink / raw)
  To: Greg KH; +Cc: labbott, sumit.semwal, devel, linux-kernel

Hi Greg,

Sorry for late responds for I was on vacation.

On 2018/2/17 0:27, Greg KH wrote:
> On Mon, Feb 12, 2018 at 06:43:09PM +0800, Yisheng Xie wrote:
>> If we failed to create debugfs for ion at ion_device_create, the
>> debug_root of ion_device will be NULL, and then when try to create debug
>> file for shrinker of heap it will be create on the top of debugfs. If we
>> also failed to create this the debug file, it call dentry_path to found
>> the path of debug_root, then a NULL point will occur.
>>
>> Fix this by avoiding call dentry_path, but show the debug name only when
>> failed to create debug file for shrinker.
>>
>> Acked-by: Laura Abbott <labbott@redhat.com>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> ---
>>  drivers/staging/android/ion/ion.c | 10 +++-------
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
>> index 57e0d80..4b69372 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -564,13 +564,9 @@ void ion_device_add_heap(struct ion_heap *heap)
>>  		debug_file = debugfs_create_file(debug_name,
>>  						 0644, dev->debug_root, heap,
>>  						 &debug_shrink_fops);
>> -		if (!debug_file) {
>> -			char buf[256], *path;
>> -
>> -			path = dentry_path(dev->debug_root, buf, 256);
>> -			pr_err("Failed to create heap shrinker debugfs at %s/%s\n",
>> -			       path, debug_name);
>> -		}
>> +		if (!debug_file)
>> +			pr_err("Failed to create ion heap shrinker debugfs at %s\n",
>> +			       debug_name);
> 
> Really we can just remove this, there's no need to check the return
> value of this debugfs call at all, it doesn't matter.

Right, and I found that you have already patched this patch into next, should I send
another version for the two patches your commented, or just send fix patches to fold?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path
  2018-02-22  8:59     ` Yisheng Xie
@ 2018-02-22  9:13       ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2018-02-22  9:13 UTC (permalink / raw)
  To: Yisheng Xie; +Cc: labbott, sumit.semwal, devel, linux-kernel

On Thu, Feb 22, 2018 at 04:59:27PM +0800, Yisheng Xie wrote:
> Hi Greg,
> 
> Sorry for late responds for I was on vacation.
> 
> On 2018/2/17 0:27, Greg KH wrote:
> > On Mon, Feb 12, 2018 at 06:43:09PM +0800, Yisheng Xie wrote:
> >> If we failed to create debugfs for ion at ion_device_create, the
> >> debug_root of ion_device will be NULL, and then when try to create debug
> >> file for shrinker of heap it will be create on the top of debugfs. If we
> >> also failed to create this the debug file, it call dentry_path to found
> >> the path of debug_root, then a NULL point will occur.
> >>
> >> Fix this by avoiding call dentry_path, but show the debug name only when
> >> failed to create debug file for shrinker.
> >>
> >> Acked-by: Laura Abbott <labbott@redhat.com>
> >> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> >> ---
> >>  drivers/staging/android/ion/ion.c | 10 +++-------
> >>  1 file changed, 3 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> >> index 57e0d80..4b69372 100644
> >> --- a/drivers/staging/android/ion/ion.c
> >> +++ b/drivers/staging/android/ion/ion.c
> >> @@ -564,13 +564,9 @@ void ion_device_add_heap(struct ion_heap *heap)
> >>  		debug_file = debugfs_create_file(debug_name,
> >>  						 0644, dev->debug_root, heap,
> >>  						 &debug_shrink_fops);
> >> -		if (!debug_file) {
> >> -			char buf[256], *path;
> >> -
> >> -			path = dentry_path(dev->debug_root, buf, 256);
> >> -			pr_err("Failed to create heap shrinker debugfs at %s/%s\n",
> >> -			       path, debug_name);
> >> -		}
> >> +		if (!debug_file)
> >> +			pr_err("Failed to create ion heap shrinker debugfs at %s\n",
> >> +			       debug_name);
> > 
> > Really we can just remove this, there's no need to check the return
> > value of this debugfs call at all, it doesn't matter.
> 
> Right, and I found that you have already patched this patch into next, should I send
> another version for the two patches your commented, or just send fix patches to fold?

Just send add-on patches, I can't "fold" anything as the commit is
already in the tree.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-02-22  9:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-12 10:43 [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 1/9] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 2/9] staging: android: ion: Remove unused include files for ion_page_pool.c Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 3/9] staging: android: ion: Nuke ion_page_pool_init Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 4/9] staging: android: ion: Avoid NULL point in error path Yisheng Xie
2018-02-16 16:27   ` Greg KH
2018-02-22  8:59     ` Yisheng Xie
2018-02-22  9:13       ` Greg KH
2018-02-12 10:43 ` [PATCH v2 5/9] staging: android: ion: Remove lable debugfs_done Yisheng Xie
2018-02-16 16:27   ` Greg KH
2018-02-12 10:43 ` [PATCH v2 6/9] staging: android: ion: Remove dead code in ion_page_pool_free Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 7/9] staging: android: ion: Return void instead of int Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 8/9] staging: android: ion: Cleanup ion_page_pool_alloc_pages Yisheng Xie
2018-02-12 10:43 ` [PATCH v2 9/9] staging: android: ion: Combine cache and uncache pools Yisheng Xie
2018-02-12 11:17 ` [PATCH v2 0/9] staging: android: ion: Some cleanup about ion Dan Carpenter
2018-02-12 11:33   ` 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.