Re: [patch] staging: ion: use two separate locks for heaps and clients in ion_device

[Greg KH]

On Fri, Oct 07, 2016 at 04:27:49PM +0800, YiPing Xu wrote:
> 
> 
> On 2016/10/5 2:02, Laura Abbott wrote:
> > On 09/30/2016 01:18 AM, Xu YiPing wrote:
> > > ion_alloc may get into slow path to get free page,
> > > the call stack:
> > > 
> > > __alloc_pages_slowpath
> > > ion_page_pool_alloc_pages
> > > alloc_buffer_page
> > > ion_system_heap_allocate
> > > ion_buffer_create  <-- hold ion_device->lock
> > > ion_alloc
> > > 
> > > after that, kernel invokes low-memory killer to kill some apps in
> > > order to free memory in android system. However, sometimes, the
> > > killing is blocked,
> > > the app's call stack:
> > > 
> > > rwsem_down_write_failed
> > > down_write
> > > ion_client_destroy
> > > ion_release
> > > fput
> > > do_signal
> > > 
> > > the killing is blocked because ion_device->lock is held by ion_alloc.
> > > 
> > > ion_alloc hold the lock for accessing the heaps list,
> > > ion_destroy_client hold the lock for accessing the clients list.
> > > 
> > > so, we can use two separate locks for heaps and clients, to avoid the
> > > unnecessary race.
> > > 
> > 
> > I've reviewed this and it looks okay at first pass but I don't want it
> > applied just yet. Ion locking is a bit of a mess and has been added
> 
> yes, and now "debugfs_mutex" and "ion_root_client" is redundant, after
> commit 49d200deaa680501f19a247b1fffb29301e51d2b and
> 9fd4dcece43a53e5a9e65a973df5693702ee6401.

Ok, now dropping this patch from my queue.

thanks,

greg k-h