2015-07-01 11:01:27

by Gioh Kim

[permalink] [raw]
Subject: [PATCHv3 0/2] staging: ion: enable shrinking of pool

Hello,

The ion has internal page pool to keep freed pages.
There is no way to shrink the pool so that the pool sometimes
grows too big.

For example my platform allocates some graphic memory via ion.
Sometimes the pool can be several hundreds MB. I want to
flush the pool before critical page shortage.


This patch set combines 2 patches like followings.

1. Patch 1/2: make individual pool work by page unit
This patch makes the pool be shrinked in page unit.

2. Patch 2/2: enable pool shrink
This patch enables pool shrink and make debugfs to specify shrink amount.
The pool shrink is almost implemented but it is not complete.
This patch completes the implementation.

For instance, this shrinks all pages in every pool.
echo 0 > /sys/kernel/debug/ion/heaps/system_shrink
If you want specify the number of pages:
echo 30 > /sys/kernel/debug/ion/heaps/system_shrink

My thanks to Laura Abbott for reviews of the v2 patchset. Most
of the changes below were based on her feedback.

Changes since v2:
- Rebased to v4.1
- Call shrinker interfaces instead of calling heap->ops->shrink
- Add more description about the reason
- Remove a patch to limit pool size

This patchset is based on v4.1


Gioh Kim (2):
staging: ion: shrink page-pool by page unit
staging: ion: debugfs to shrink pool

drivers/staging/android/ion/ion.c | 22 +++++++++-------------
drivers/staging/android/ion/ion_page_pool.c | 5 +++--
drivers/staging/android/ion/ion_system_heap.c | 16 ++++++++++++++--
3 files changed, 26 insertions(+), 17 deletions(-)

--
1.7.9.5


2015-07-01 11:01:39

by Gioh Kim

[permalink] [raw]
Subject: [PATCHv3 1/2] staging: ion: shrink page-pool by page unit

This patch shrink page-pool by page unit.

The system shrinker calls ion_heap_shrink_count() to get nr_to_scan,
and pass it to ion_heap_shrink_scan().
The problem is the return value of ion_heap_shrink_count() is the number
of pages but ion_system_heap_shrink(), which is called by
ion_heap_shrink_scan(), gets the number of chunk.

The main root of this is that ion_page_pool_shrink() returns page count
via ion_page_pool_total() if it have to check pool size. But it frees
chunks of pages if it have to free pools.

This patch first fix ion_page_pool_shrink() to count only pages,
not chunks. And then ion_system_heap_shrink() to work on pages.

Signed-off-by: Gioh Kim <[email protected]>
---
drivers/staging/android/ion/ion_page_pool.c | 5 +++--
drivers/staging/android/ion/ion_system_heap.c | 16 ++++++++++++++--
2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 4b88f11..19ad3ab 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..7a7a9a0 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -212,14 +212,26 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask,
{
struct ion_system_heap *sys_heap;
int nr_total = 0;
- int i;
+ int i, nr_freed;
+ int only_scan = 0;

sys_heap = container_of(heap, struct ion_system_heap, heap);

+ if (!nr_to_scan)
+ only_scan = 1;
+
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;
+
+ if (!only_scan) {
+ nr_to_scan -= nr_freed;
+ /* shrink completed */
+ if (nr_to_scan <= 0)
+ break;
+ }
}

return nr_total;
--
1.7.9.5

2015-07-01 11:01:56

by Gioh Kim

[permalink] [raw]
Subject: [PATCHv3 2/2] staging: ion: debugfs to shrink pool

This patch enables debugfs file /sys/kernel/debug/ion/heaps/system_shrink
to shrink pool and get pool size. It is already implemented
but not complete. This patch completes and enables it.

Reading the file returns pool size
in page unit and writing the number of pages shrinks pool.
It flushes all pages to write zero at the file.

Signed-off-by: Gioh Kim <[email protected]>
---
drivers/staging/android/ion/ion.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 6f48112..9327e8a 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1466,7 +1466,6 @@ 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;
@@ -1474,15 +1473,14 @@ static int debug_shrink_set(void *data, u64 val)
int objs;

sc.gfp_mask = -1;
- sc.nr_to_scan = 0;
-
- if (!val)
- return 0;
+ sc.nr_to_scan = val;

- objs = heap->shrinker.shrink(&heap->shrinker, &sc);
- sc.nr_to_scan = objs;
+ if (!val) {
+ objs = heap->shrinker.count_objects(&heap->shrinker, &sc);
+ sc.nr_to_scan = objs;
+ }

- heap->shrinker.shrink(&heap->shrinker, &sc);
+ heap->shrinker.scan_objects(&heap->shrinker, &sc);
return 0;
}

@@ -1495,14 +1493,13 @@ static int debug_shrink_get(void *data, u64 *val)
sc.gfp_mask = -1;
sc.nr_to_scan = 0;

- objs = heap->shrinker.shrink(&heap->shrinker, &sc);
+ objs = heap->shrinker.count_objects(&heap->shrinker, &sc);
*val = objs;
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)
{
@@ -1540,8 +1537,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->shrinker.count_objects && heap->shrinker.scan_objects) {
char debug_name[64];

snprintf(debug_name, 64, "%s_shrink", heap->name);
@@ -1556,7 +1552,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

2015-07-02 18:21:43

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCHv3 1/2] staging: ion: shrink page-pool by page unit

On 07/01/2015 04:02 AM, Gioh Kim wrote:
> This patch shrink page-pool by page unit.
>
> The system shrinker calls ion_heap_shrink_count() to get nr_to_scan,
> and pass it to ion_heap_shrink_scan().
> The problem is the return value of ion_heap_shrink_count() is the number
> of pages but ion_system_heap_shrink(), which is called by
> ion_heap_shrink_scan(), gets the number of chunk.
>
> The main root of this is that ion_page_pool_shrink() returns page count
> via ion_page_pool_total() if it have to check pool size. But it frees
> chunks of pages if it have to free pools.
>
> This patch first fix ion_page_pool_shrink() to count only pages,
> not chunks. And then ion_system_heap_shrink() to work on pages.
>
> Signed-off-by: Gioh Kim <[email protected]>

Reviewed-by: Laura Abbott <[email protected]>

> ---
> drivers/staging/android/ion/ion_page_pool.c | 5 +++--
> drivers/staging/android/ion/ion_system_heap.c | 16 ++++++++++++++--
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
> index 4b88f11..19ad3ab 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..7a7a9a0 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -212,14 +212,26 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask,
> {
> struct ion_system_heap *sys_heap;
> int nr_total = 0;
> - int i;
> + int i, nr_freed;
> + int only_scan = 0;
>
> sys_heap = container_of(heap, struct ion_system_heap, heap);
>
> + if (!nr_to_scan)
> + only_scan = 1;
> +
> 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;
> +
> + if (!only_scan) {
> + nr_to_scan -= nr_freed;
> + /* shrink completed */
> + if (nr_to_scan <= 0)
> + break;
> + }
> }
>
> return nr_total;
>

2015-07-02 18:24:26

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCHv3 2/2] staging: ion: debugfs to shrink pool

On 07/01/2015 04:02 AM, Gioh Kim wrote:
> This patch enables debugfs file /sys/kernel/debug/ion/heaps/system_shrink

Nit: This technically enables debugfs shrinking for all heaps, not just
the system heap although the system heap is the only one with a shrinker
right now.

> to shrink pool and get pool size. It is already implemented
> but not complete. This patch completes and enables it.
>
> Reading the file returns pool size
> in page unit and writing the number of pages shrinks pool.
> It flushes all pages to write zero at the file.
>
> Signed-off-by: Gioh Kim <[email protected]>

Reviewed-by: Laura Abbott <[email protected]>

> ---
> drivers/staging/android/ion/ion.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 6f48112..9327e8a 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -1466,7 +1466,6 @@ 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;
> @@ -1474,15 +1473,14 @@ static int debug_shrink_set(void *data, u64 val)
> int objs;
>
> sc.gfp_mask = -1;
> - sc.nr_to_scan = 0;
> -
> - if (!val)
> - return 0;
> + sc.nr_to_scan = val;
>
> - objs = heap->shrinker.shrink(&heap->shrinker, &sc);
> - sc.nr_to_scan = objs;
> + if (!val) {
> + objs = heap->shrinker.count_objects(&heap->shrinker, &sc);
> + sc.nr_to_scan = objs;
> + }
>
> - heap->shrinker.shrink(&heap->shrinker, &sc);
> + heap->shrinker.scan_objects(&heap->shrinker, &sc);
> return 0;
> }
>
> @@ -1495,14 +1493,13 @@ static int debug_shrink_get(void *data, u64 *val)
> sc.gfp_mask = -1;
> sc.nr_to_scan = 0;
>
> - objs = heap->shrinker.shrink(&heap->shrinker, &sc);
> + objs = heap->shrinker.count_objects(&heap->shrinker, &sc);
> *val = objs;
> 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)
> {
> @@ -1540,8 +1537,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->shrinker.count_objects && heap->shrinker.scan_objects) {
> char debug_name[64];
>
> snprintf(debug_name, 64, "%s_shrink", heap->name);
> @@ -1556,7 +1552,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
> path, debug_name);
> }
> }
> -#endif
> +
> up_write(&dev->lock);
> }
>
>