On Fri, Aug 31, 2012 at 8:43 PM, Nishanth Peethambaran nishanth.peethu@gmail.com wrote:
On Fri, Aug 31, 2012 at 2:27 PM, zhangfei gao zhangfei.gao@gmail.com wrote:
On Thu, Aug 30, 2012 at 5:34 PM, Nishanth Peethambaran
Thanks Nishanth for careful review.
nishanth.peethu@gmail.com wrote:
ion_buffer_create is called with dev->lock acquired and so dev->lock would have to be freed and schedule out before retry so that the process getting killed can free up all the memory. Else, it will get blocked while freeing the first buffer in ion_buffer_destroy() when it tries to acquire the dev->lock.
Your concern is correct. My mistake for considering no problem if debugfs test no problem. Will resend the updated the patch,
- move ion_shrink out of mutex.
- check error flag of ERR_PTR(-ENOMEM)
- add msleep to allow schedule out.
debugfs of carveout_heap might show the client not using the heap though memory is not freed. If the buffer was mapped to userspace, ion_share_dma_buf() would have taken one reference of ion_buffer() So, all the ion_buffer_put() will succeed if dma_buf fd is not released first and thus all the ion_handle_put() will succeed and will remove the handles from the client before ion_client_destroy() will try to acquire dev->lock()
Sorry, not fully understand. The ion_shrink will send_sig according to used size, is this solve your concern.
It was an explanation of how the first issue can get hidden by debugfs. Client would be removed though the buffers may not get actually freed and so won't be shown in debugfs as a client of the heap.
A lowmem killer with multiple threshold, omm_adj values similar to android lowmem killer would be also an option but it is a matter of policy whether each carveout/cma heap needs to do a lowmem killer or not. For some cases, fallback to other heaps would be ok and for some cases fallback to other heaps like kmalloc and vmalloc or even other carveout/cma areas might not be an option.
This simple killer is refer lownmemorykiller. It is useful for us, since we still not enable CMA.
The lowmemkiller should kick in once the frees space goes below a threshold or heap gets fragmented, but in the attached patch, it kicks it only when allocation fails. Refer to the comparison of free space to lowmem_minfree[n] done in lowmem_shrink(). Even for CMA, lowmemkiller would be needed unless a huge area is reserved for CMA which will restrict the area available for non-movable allocations.
Could you help correct my understand.
Though lowmem provide lowmem_minfree and lowmem_adj, lowmem_shrink registered as shrink function and only be called when allocation fail.
3.4: mm/page_alloc.c __alloc_pages_nodemask -> if (unlikely(!page)) __alloc_pages_slowpath -> wake_all_kswapd -> vmscan.c kswapd -> do_shrinker_shrink -> lowmemorykiller.c lowmem_shrink
Simple experiment in my environment. by default static int lowmem_minfree[6] = { 3 * 512, /* 6MB */ 2 * 1024, /* 8MB */ 4 * 1024, /* 16MB */ 16 * 1024, /* 64MB */ }; # cd /tmp # dd if=/dev/zero of=del bs=1M count=350 #free total used free shared buffers Mem: 384576 377588 6988 0 0 Swap: 0 0 0 Total: 384576 377588 6988
lowmem_shrink is not triggered even free=6M and only happen when free=3M, suppose allocation fail.
Conclusion is lowmem_shrink only be triggered when allocation fail. While lowmem_minfree and lowmem_adj are useful when allocation fail. And there is no dameon monitoring system memory touch to the threshold.
I am not sure whether the understand is correct, however in our ion oom killer we also triggered the shrink function when allocation fail, which is simple. Or we have to start a kernel task keep monitoring ion size? looks complicated.
Is there any particular reason to use send_sig() and set_tsk_thread_flag() instead of force_sig(). Just curious to know.
This staff is referring lowmemorykiller.c set_tsk_thread_flag is required no matter use send_sig() or force_sig(). However, during stress testing, force_sig() may cause system NULL pointer error. While send_sig() is much stable.
Besides, the stress test on non android system, is manually ion_shrink(heap, -1). While usually ion_shrink(heap, 0), without kill adj <= 0.
Ok. I am in the process of migration to 3.4 and so was referring to 3.0 lowmemkiller which uses force_sig().
drivers/staging/android/lowmemorykiller.c 3.0 use force_sig, 3.4: send_sig(SIGKILL, selected, 0); set_tsk_thread_flag(selected, TIF_MEMDIE);
- Nishanth Peethambaran