Re: [PATCH] staging: android: ion: Make ion_free asynchronous more actively.

[Laura Abbott]

On 10/10/2016 07:31 PM, ming.ling wrote:
> From: Ming Ling <ming.ling@xxxxxxxxxxxxxx>
>
> So far some ion heaps such as carveout_heap, chunk_heap,
> system_heap have freed buffers asynchrounously. Freed buffers
> are placed on a free list and freed from a low priority background
> thread. If allocations from a particular heap fail, the free list
> is drained.
>
> But that mechanism does not work well to free those buffers when
> cpu is very busy:
> 	1.That low priority background thread has no chance to run.
> 	2.And with helps of kinds of memory reclaim mechanisms,
> 	allocations from a system_heap never fail, so the free list
> 	can't be drained on that condition.
> 	3.Ion_heap_shrink_scan can shrink the free list, but it is
> 	too slow.
>
> Too many freed buffers in heap's free list waiting to be freed will
> lead to low memory killer over-killing innocent processes.
>
> For example:
> On my Android Mobile phone(arm32, 2G ram)
> lowmemorykiller: Killing 'viders.calendar' (2271), adj 15,
> to free 45060kB on behalf of 'kswapd0' (60) because
> cache 17208kB is below limit 46080kB for oom_score_adj 0.
> Free memory is 3424kB above reserved.
>
> Memory footprint is as below:
> --------------------------------------------
> MemTotal:        1972164 kB
> --------------------------------------------
> Normal free:43624kB min:3228kB low:5308kB high:6116kB active_anon:
> 656kB inactive_anon:1308kB active_file:10600kB inactive_file:6628kB
> HighMem free:13208kB min:512kB low:4720kB high:6352kB active_anon:
> 126188kB inactive_anon:74364kB active_file:325720kB inactive_file:
> 585832k
> --------------------------------------------
> ion system heap:
> --------------------
> 	total orphaned		9961472
> 	total			58986496
> 	deferred free		630575104
> --------------------------------------------
>
> There are about 630575104=601MB memory on ion system heap's free
> list waiting to be freed. And it kills more processes, until ion low
> priority background thread gets a chance to run as below:
> lowmemorykiller: Killing 'd.process.media' (1402), adj 11,
> lowmemorykiller: Killing 'dsrt.srtmemtest' (2552), adj 9,
> lowmemorykiller: Killing 'rd.engineermode' (2167), adj 8,
> lowmemorykiller: Killing 'rd.fileexplorer' (2431), adj 7,
> lowmemorykiller: Killing 'd.process.acore' (1499), adj 5,
> ...
>
> Limit ion_heap_deferred_free thread as a low priority thread doesn't
> make any sense. Remove that restriction makes ion_free asynchronous
> more actively, and more background applications can survive.

I don't have the full history here of why the thread was set to
run at a lower priority. If I had to guess, it's because the
deferred thread was blocking out other higher priority threads
and causing graphics issues, so it's possibly trading one issue
for another.

I'm reluctant to Ack this without some testing from others first.
Any other eager Ion users want to test this on their system?
(Any eager Ion users at all?)

Thanks,
Laura

> Signed-off-by: Ming Ling <ming.ling@xxxxxxxxxxxxxx>
> ---
>  drivers/staging/android/ion/ion_heap.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
> index 4e5c0f1..d68a80c 100644
> --- a/drivers/staging/android/ion/ion_heap.c
> +++ b/drivers/staging/android/ion/ion_heap.c
> @@ -250,8 +250,6 @@ static int ion_heap_deferred_free(void *data)
>
>  int ion_heap_init_deferred_free(struct ion_heap *heap)
>  {
> -	struct sched_param param = { .sched_priority = 0 };
> -
>  	INIT_LIST_HEAD(&heap->free_list);
>  	init_waitqueue_head(&heap->waitqueue);
>  	heap->task = kthread_run(ion_heap_deferred_free, heap,
> @@ -261,7 +259,6 @@ int ion_heap_init_deferred_free(struct ion_heap *heap)
>  		       __func__);
>  		return PTR_ERR_OR_ZERO(heap->task);
>  	}
> -	sched_setscheduler(heap->task, SCHED_IDLE, &param);
>  	return 0;
>  }