[RFCv2 0/3] enable pool shrinking in page unit

[RFCv2 0/3] enable pool shrinking in page unit

[Gioh Kim]

Hello,

Current shrinking is not page unit, block unit.
But shrinker returns the pool size in page unit,
so it is confused.

And there is no way to control pool size and shrink pool directly.

I have 3 patches:

1. Patch 1/3: make pool be shrinked by page unit
This patch trys to shrink pool in page unit.

2. Patch 2/3: enable debugfs to shrink page directly
This patch enables debugfs to specify shrink amount.

3. Patch 3/3: limit pool size
This patch specifies pool size limit.

This patchset is based on linux-next-20141023.

Gioh Kim (3):
  staging: ion: shrink page-pool by page unit
  staging: ion: debugfs to shrink pool
  staging: ion: limit pool size

 drivers/staging/android/ion/Kconfig           |    4 ++++
 drivers/staging/android/ion/ion.c             |   31 +++++++------------------
 drivers/staging/android/ion/ion_page_pool.c   |   31 +++++++++++++++----------
 drivers/staging/android/ion/ion_system_heap.c |    7 ++++--
 4 files changed, 36 insertions(+), 37 deletions(-)

-- 
1.7.9.5

[RFCv2 1/3] staging: ion: shrink page-pool by page unit

[Gioh Kim]

This patch shrink page-pool by page unit.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
---
 drivers/staging/android/ion/ion_page_pool.c   |    5 +++--
 drivers/staging/android/ion/ion_system_heap.c |    7 +++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 5864f3d..165152f 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -116,7 +116,7 @@ static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
 int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
 				int nr_to_scan)
 {
-	int freed;
+	int freed = 0;
 	bool high;
 
 	if (current_is_kswapd())
@@ -127,7 +127,7 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
 	if (nr_to_scan == 0)
 		return ion_page_pool_total(pool, high);
 
-	for (freed = 0; freed < nr_to_scan; freed++) {
+	while (freed <= nr_to_scan) {
 		struct page *page;
 
 		mutex_lock(&pool->mutex);
@@ -141,6 +141,7 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
 		}
 		mutex_unlock(&pool->mutex);
 		ion_page_pool_free_pages(pool, page);
+		freed += (1 << pool->order);
 	}
 
 	return freed;
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index da2a63c..91d862f 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -211,7 +211,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask,
 					int nr_to_scan)
 {
 	struct ion_system_heap *sys_heap;
-	int nr_total = 0;
+	int nr_total = 0, nr_freed;
 	int i;
 
 	sys_heap = container_of(heap, struct ion_system_heap, heap);
@@ -219,7 +219,10 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask,
 	for (i = 0; i < num_orders; i++) {
 		struct ion_page_pool *pool = sys_heap->pools[i];
 
-		nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
+		nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan);
+		nr_total += nr_freed;
+		/* nr_to_scan can be negative */
+		nr_to_scan -= nr_freed;
 	}
 
 	return nr_total;
-- 
1.7.9.5

Re: [RFCv2 1/3] staging: ion: shrink page-pool by page unit

[Laura Abbott]

Hi,

On 10/23/2014 11:47 PM, Gioh Kim wrote:
> This patch shrink page-pool by page unit.
>

Can you explain a bit more about what this patch is
fixing? The description in the cover letter would
be helpful here.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFCv2 1/3] staging: ion: shrink page-pool by page unit

[Gioh Kim]

2014-10-25 오전 5:09, Laura Abbott 쓴 글:
> Hi,
>
> On 10/23/2014 11:47 PM, Gioh Kim wrote:
>> This patch shrink page-pool by page unit.
>>
>
> Can you explain a bit more about what this patch is
> fixing? The description in the cover letter would
> be helpful here.

I'm sorry for the lack of explanation.

First, I wanted to shrink ion page-pool, but there was no way for it.
So I checked the existing codes and found a problem.

ion_page_pool_total() returns the number of pages in the pool,
so ion_heap_shrink_count() returns also the number of pages.
ion_page_pool_shrink() takes the argument nr_to_scan as the number of chunk, a bunch of pages.

The system shrinker calls ion_heap_shrink_count() to get nr_to_scan,
and pass it to ion_page_pool_shrink().
The problem is the argument of ion_page_pool_shrink() is the amount of chunk to be shrinked.

We have to make both of them take number of page or chunk respectively.

The first patch is doing it.

The second one is, based on the first patch, making shrink interface via debugfs.
The third is for extra interface.

>
> Thanks,
> Laura
>

[RFCv2 2/3] staging: ion: debugfs to shrink pool

[Gioh Kim]

This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink,
to shrink pool or get pool size.
Reading the file returns pool size and writing occurs to shrink pool.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
---
 drivers/staging/android/ion/ion.c |   31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 290d4d2..ecc1121 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1463,43 +1463,29 @@ static const struct file_operations debug_heap_fops = {
 	.release = single_release,
 };
 
-#ifdef DEBUG_HEAP_SHRINKER
 static int debug_shrink_set(void *data, u64 val)
 {
 	struct ion_heap *heap = data;
-	struct shrink_control sc;
 	int objs;
 
-	sc.gfp_mask = -1;
-	sc.nr_to_scan = 0;
-
-	if (!val)
-		return 0;
-
-	objs = heap->shrinker.shrink(&heap->shrinker, &sc);
-	sc.nr_to_scan = objs;
+	if (val)
+		objs = val;
+	else
+		objs = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
 
-	heap->shrinker.shrink(&heap->shrinker, &sc);
+	(void)heap->ops->shrink(heap, __GFP_HIGHMEM, objs);
 	return 0;
 }
 
 static int debug_shrink_get(void *data, u64 *val)
 {
 	struct ion_heap *heap = data;
-	struct shrink_control sc;
-	int objs;
-
-	sc.gfp_mask = -1;
-	sc.nr_to_scan = 0;
-
-	objs = heap->shrinker.shrink(&heap->shrinker, &sc);
-	*val = objs;
+	*val = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
 	return 0;
 }
 
 DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
 			debug_shrink_set, "%llu\n");
-#endif
 
 void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 {
@@ -1534,8 +1520,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 			path, heap->name);
 	}
 
-#ifdef DEBUG_HEAP_SHRINKER
-	if (heap->shrinker.shrink) {
+	if (heap->ops->shrink) {
 		char debug_name[64];
 
 		snprintf(debug_name, 64, "%s_shrink", heap->name);
@@ -1550,7 +1535,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 				path, debug_name);
 		}
 	}
-#endif
+
 	up_write(&dev->lock);
 }
 
-- 
1.7.9.5

Re: [RFCv2 2/3] staging: ion: debugfs to shrink pool

[Laura Abbott]

Hi,

On 10/23/2014 11:47 PM, Gioh Kim wrote:
> This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink,
> to shrink pool or get pool size.
> Reading the file returns pool size and writing occurs to shrink pool.
>

Can you clarify here that you are updating the existing debugfs
infrastructure? Since the shrinker debugfs was commented out before
it missed the API update so it would be good to note that as well.

> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> ---
>   drivers/staging/android/ion/ion.c |   31 ++++++++-----------------------
>   1 file changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 290d4d2..ecc1121 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -1463,43 +1463,29 @@ static const struct file_operations debug_heap_fops = {
>   	.release = single_release,
>   };
>
> -#ifdef DEBUG_HEAP_SHRINKER

We're now unconditionally adding the shrinker debugfs. I'm not sure if
this is something we want in production code. Could you turn this
into a proper Kconfig?

>   static int debug_shrink_set(void *data, u64 val)
>   {
>   	struct ion_heap *heap = data;
> -	struct shrink_control sc;
>   	int objs;
>
> -	sc.gfp_mask = -1;
> -	sc.nr_to_scan = 0;
> -
> -	if (!val)
> -		return 0;
> -
> -	objs = heap->shrinker.shrink(&heap->shrinker, &sc);
> -	sc.nr_to_scan = objs;
> +	if (val)
> +		objs = val;
> +	else
> +		objs = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
>
> -	heap->shrinker.shrink(&heap->shrinker, &sc);
> +	(void)heap->ops->shrink(heap, __GFP_HIGHMEM, objs);

The shrinker API now has separate functions for counting and scanning
and ion wraps the calls to those as well to account for the deferred
freelist. I realize the existing behavior may have been broken but the
debugfs seems incomplete if it's not taking that into account as well.

>   	return 0;
>   }
>
>   static int debug_shrink_get(void *data, u64 *val)
>   {
>   	struct ion_heap *heap = data;
> -	struct shrink_control sc;
> -	int objs;
> -
> -	sc.gfp_mask = -1;
> -	sc.nr_to_scan = 0;
> -
> -	objs = heap->shrinker.shrink(&heap->shrinker, &sc);
> -	*val = objs;
> +	*val = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
>   	return 0;
>   }
>
>   DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
>   			debug_shrink_set, "%llu\n");
> -#endif
>
>   void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>   {
> @@ -1534,8 +1520,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>   			path, heap->name);
>   	}
>
> -#ifdef DEBUG_HEAP_SHRINKER
> -	if (heap->shrinker.shrink) {
> +	if (heap->ops->shrink) {

Technically a heap doesn't need to set ops->shrink to have a shrinker
set up (not that it really matters for the current setup). Checking
for scan_objects would be better.

>   		char debug_name[64];
>
>   		snprintf(debug_name, 64, "%s_shrink", heap->name);
> @@ -1550,7 +1535,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>   				path, debug_name);
>   		}
>   	}
> -#endif
> +
>   	up_write(&dev->lock);
>   }
>
>

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFCv2 2/3] staging: ion: debugfs to shrink pool

[Gioh Kim]

2014-10-25 오전 5:38, Laura Abbott 쓴 글:
> Hi,
>
> On 10/23/2014 11:47 PM, Gioh Kim wrote:
>> This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink,
>> to shrink pool or get pool size.
>> Reading the file returns pool size and writing occurs to shrink pool.
>>
>
> Can you clarify here that you are updating the existing debugfs
> infrastructure? Since the shrinker debugfs was commented out before
> it missed the API update so it would be good to note that as well.

OK, I will at the v2 patch.

>
>> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
>> ---
>>   drivers/staging/android/ion/ion.c |   31 ++++++++-----------------------
>>   1 file changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
>> index 290d4d2..ecc1121 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -1463,43 +1463,29 @@ static const struct file_operations debug_heap_fops = {
>>       .release = single_release,
>>   };
>>
>> -#ifdef DEBUG_HEAP_SHRINKER
>
> We're now unconditionally adding the shrinker debugfs. I'm not sure if
> this is something we want in production code. Could you turn this
> into a proper Kconfig?

The interface is via debugfs.
I think we don't turn on the debugfs in production code.

>
>>   static int debug_shrink_set(void *data, u64 val)
>>   {
>>       struct ion_heap *heap = data;
>> -    struct shrink_control sc;
>>       int objs;
>>
>> -    sc.gfp_mask = -1;
>> -    sc.nr_to_scan = 0;
>> -
>> -    if (!val)
>> -        return 0;
>> -
>> -    objs = heap->shrinker.shrink(&heap->shrinker, &sc);
>> -    sc.nr_to_scan = objs;
>> +    if (val)
>> +        objs = val;
>> +    else
>> +        objs = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
>>
>> -    heap->shrinker.shrink(&heap->shrinker, &sc);
>> +    (void)heap->ops->shrink(heap, __GFP_HIGHMEM, objs);
>
> The shrinker API now has separate functions for counting and scanning
> and ion wraps the calls to those as well to account for the deferred
> freelist. I realize the existing behavior may have been broken but the
> debugfs seems incomplete if it's not taking that into account as well.

ion_heap_shrink_count() and ion_heap_shrink_scan() are called by shrinker.
I think debug_shrink_set()/get() are for debugfs, so they can work without shrinker.

You can count deferred freelist via other debugfs file, for instance /sys/kernel/debug/ion/heaps/system.
I think debug_shrink_set()/get() should work by the number of pages in the pool.


>
>>       return 0;
>>   }
>>
>>   static int debug_shrink_get(void *data, u64 *val)
>>   {
>>       struct ion_heap *heap = data;
>> -    struct shrink_control sc;
>> -    int objs;
>> -
>> -    sc.gfp_mask = -1;
>> -    sc.nr_to_scan = 0;
>> -
>> -    objs = heap->shrinker.shrink(&heap->shrinker, &sc);
>> -    *val = objs;
>> +    *val = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
>>       return 0;
>>   }
>>
>>   DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
>>               debug_shrink_set, "%llu\n");
>> -#endif
>>
>>   void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>>   {
>> @@ -1534,8 +1520,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>>               path, heap->name);
>>       }
>>
>> -#ifdef DEBUG_HEAP_SHRINKER
>> -    if (heap->shrinker.shrink) {
>> +    if (heap->ops->shrink) {
>
> Technically a heap doesn't need to set ops->shrink to have a shrinker
> set up (not that it really matters for the current setup). Checking
> for scan_objects would be better.
>
>>           char debug_name[64];
>>
>>           snprintf(debug_name, 64, "%s_shrink", heap->name);
>> @@ -1550,7 +1535,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>>                   path, debug_name);
>>           }
>>       }
>> -#endif
>> +
>>       up_write(&dev->lock);
>>   }
>>
>>
>
> Thanks,
> Laura
>

[RFCv2 3/3] staging: ion: limit pool size

[Gioh Kim]

This patch limits pool size by page unit.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
---
 drivers/staging/android/ion/Kconfig         |    4 ++++
 drivers/staging/android/ion/ion_page_pool.c |   26 ++++++++++++++++----------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
index 3452346..e6b1a54 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -33,3 +33,7 @@ config ION_TEGRA
 	help
 	  Choose this option if you wish to use ion on an nVidia Tegra.
 
+config ION_POOL_LIMIT
+	int "Limit count of pages in pool"
+	depends on ION
+	default "0"
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 165152f..d63e93f 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -22,8 +22,11 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
+#include <linux/kconfig.h>
 #include "ion_priv.h"
 
+#define POOL_LIMIT CONFIG_ION_POOL_LIMIT
+
 static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
 {
 	struct page *page = alloc_pages(pool->gfp_mask, pool->order);
@@ -41,8 +44,21 @@ static void ion_page_pool_free_pages(struct ion_page_pool *pool,
 	__free_pages(page, pool->order);
 }
 
+static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
+{
+	int count = pool->low_count;
+
+	if (high)
+		count += pool->high_count;
+
+	return count << pool->order;
+}
+
 static int ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
 {
+	if (POOL_LIMIT && ion_page_pool_total(pool, 1) > POOL_LIMIT)
+		return 1;
+
 	mutex_lock(&pool->mutex);
 	if (PageHighMem(page)) {
 		list_add_tail(&page->lru, &pool->high_items);
@@ -103,16 +119,6 @@ void ion_page_pool_free(struct ion_page_pool *pool, struct page *page)
 		ion_page_pool_free_pages(pool, page);
 }
 
-static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
-{
-	int count = pool->low_count;
-
-	if (high)
-		count += pool->high_count;
-
-	return count << pool->order;
-}
-
 int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
 				int nr_to_scan)
 {
-- 
1.7.9.5

Re: [RFCv2 3/3] staging: ion: limit pool size

[Laura Abbott]

Hi,

On 10/23/2014 11:47 PM, Gioh Kim wrote:
> This patch limits pool size by page unit.
>

This looks useful. Might be nice to add a debugfs option
to change this at runtime as well.

> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> ---
>   drivers/staging/android/ion/Kconfig         |    4 ++++
>   drivers/staging/android/ion/ion_page_pool.c |   26 ++++++++++++++++----------
>   2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
> index 3452346..e6b1a54 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -33,3 +33,7 @@ config ION_TEGRA
>   	help
>   	  Choose this option if you wish to use ion on an nVidia Tegra.
>
> +config ION_POOL_LIMIT
> +	int "Limit count of pages in pool"
> +	depends on ION
> +	default "0"

Can you add help text here? It would be useful to clarify that the
units are in pages and that 0 will allow unlimited growth of the
pool. This should also clarify that this is a limit per
individual pool and not a limit for all page pools in the system.

> diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
> index 165152f..d63e93f 100644
> --- a/drivers/staging/android/ion/ion_page_pool.c
> +++ b/drivers/staging/android/ion/ion_page_pool.c
> @@ -22,8 +22,11 @@
>   #include <linux/module.h>
>   #include <linux/slab.h>
>   #include <linux/swap.h>
> +#include <linux/kconfig.h>
>   #include "ion_priv.h"
>
> +#define POOL_LIMIT CONFIG_ION_POOL_LIMIT
> +

I don't think the extra #define helps anything here, was
there something else intended here?

>   static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
>   {
>   	struct page *page = alloc_pages(pool->gfp_mask, pool->order);
> @@ -41,8 +44,21 @@ static void ion_page_pool_free_pages(struct ion_page_pool *pool,
>   	__free_pages(page, pool->order);
>   }
>
> +static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
> +{
> +	int count = pool->low_count;
> +
> +	if (high)
> +		count += pool->high_count;
> +
> +	return count << pool->order;
> +}
> +
>   static int ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
>   {
> +	if (POOL_LIMIT && ion_page_pool_total(pool, 1) > POOL_LIMIT)
> +		return 1;
> +
>   	mutex_lock(&pool->mutex);
>   	if (PageHighMem(page)) {
>   		list_add_tail(&page->lru, &pool->high_items);
> @@ -103,16 +119,6 @@ void ion_page_pool_free(struct ion_page_pool *pool, struct page *page)
>   		ion_page_pool_free_pages(pool, page);
>   }
>
> -static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
> -{
> -	int count = pool->low_count;
> -
> -	if (high)
> -		count += pool->high_count;
> -
> -	return count << pool->order;
> -}
> -
>   int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
>   				int nr_to_scan)
>   {
>

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFCv2 3/3] staging: ion: limit pool size

[Gioh Kim]

2014-10-25 오전 5:53, Laura Abbott 쓴 글:
> Hi,
>
> On 10/23/2014 11:47 PM, Gioh Kim wrote:
>> This patch limits pool size by page unit.
>>
>
> This looks useful. Might be nice to add a debugfs option
> to change this at runtime as well.
>
>> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
>> ---
>>   drivers/staging/android/ion/Kconfig         |    4 ++++
>>   drivers/staging/android/ion/ion_page_pool.c |   26 ++++++++++++++++----------
>>   2 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
>> index 3452346..e6b1a54 100644
>> --- a/drivers/staging/android/ion/Kconfig
>> +++ b/drivers/staging/android/ion/Kconfig
>> @@ -33,3 +33,7 @@ config ION_TEGRA
>>       help
>>         Choose this option if you wish to use ion on an nVidia Tegra.
>>
>> +config ION_POOL_LIMIT
>> +    int "Limit count of pages in pool"
>> +    depends on ION
>> +    default "0"
>
> Can you add help text here? It would be useful to clarify that the
> units are in pages and that 0 will allow unlimited growth of the
> pool. This should also clarify that this is a limit per
> individual pool and not a limit for all page pools in the system.
>
>> diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
>> index 165152f..d63e93f 100644
>> --- a/drivers/staging/android/ion/ion_page_pool.c
>> +++ b/drivers/staging/android/ion/ion_page_pool.c
>> @@ -22,8 +22,11 @@
>>   #include <linux/module.h>
>>   #include <linux/slab.h>
>>   #include <linux/swap.h>
>> +#include <linux/kconfig.h>
>>   #include "ion_priv.h"
>>
>> +#define POOL_LIMIT CONFIG_ION_POOL_LIMIT
>> +
>
> I don't think the extra #define helps anything here, was
> there something else intended here?

No, I just follow other codes.
If it isn't necessary, I will remove it at v2 patch.


>
>>   static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
>>   {
>>       struct page *page = alloc_pages(pool->gfp_mask, pool->order);
>> @@ -41,8 +44,21 @@ static void ion_page_pool_free_pages(struct ion_page_pool *pool,
>>       __free_pages(page, pool->order);
>>   }
>>
>> +static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
>> +{
>> +    int count = pool->low_count;
>> +
>> +    if (high)
>> +        count += pool->high_count;
>> +
>> +    return count << pool->order;
>> +}
>> +
>>   static int ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
>>   {
>> +    if (POOL_LIMIT && ion_page_pool_total(pool, 1) > POOL_LIMIT)
>> +        return 1;
>> +
>>       mutex_lock(&pool->mutex);
>>       if (PageHighMem(page)) {
>>           list_add_tail(&page->lru, &pool->high_items);
>> @@ -103,16 +119,6 @@ void ion_page_pool_free(struct ion_page_pool *pool, struct page *page)
>>           ion_page_pool_free_pages(pool, page);
>>   }
>>
>> -static int ion_page_pool_total(struct ion_page_pool *pool, bool high)
>> -{
>> -    int count = pool->low_count;
>> -
>> -    if (high)
>> -        count += pool->high_count;
>> -
>> -    return count << pool->order;
>> -}
>> -
>>   int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
>>                   int nr_to_scan)
>>   {
>>
>
> Thanks,
> Laura
>