On Fri, Mar 30, 2018 at 09:39:03AM +0800, Ganesh Mahendran wrote: > 2018-03-30 9:29 GMT+08:00 Minchan Kim <minc...@kernel.org>: > > Hi Ganesh, > > > > On Fri, Mar 30, 2018 at 09:21:55AM +0800, Ganesh Mahendran wrote: > >> 2018-03-29 14:54 GMT+08:00 Minchan Kim <minc...@kernel.org>: > >> > binder_update_page_range needs down_write of mmap_sem because > >> > vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless > >> > it is set. However, when I profile binder working, it seems > >> > every binder buffers should be mapped in advance by binder_mmap. > >> > It means we could set VM_MIXEDMAP in binder_mmap time which is > >> > already hold a mmap_sem as down_write so binder_update_page_range > >> > doesn't need to hold a mmap_sem as down_write. > >> > > >> > Android suffers from mmap_sem contention so let's reduce mmap_sem > >> > down_write. > >> > >> Hi, Minchan: > >> > >> It seems there is performance regression of this patch. > > > > You mean "This patch aims for solving performance regression" not "This > > patch > > makes performance regression"? > > After applying this patch in our devices, app launch time increases > about 15% in average. > "This patch makes performance regression", yes, from the results, it > is like this. > > I will do more test of this patch.
Thanks for the testing. Every apps increases time? I will appreciate if you confirm the problem again. I guess the reason is currently rw_semaphore's preferrence for writer at up_write so If binder uses down_read instead of down_write, binder should compete other down_read callers at wake up time from up_write and could lose wakeup chance if it is low priority. Anyway, I think it's abuse of using down_write to get lock holding asap where it's down_read enough. I guess there is tradeoff based on binder's priority. If we go to the speculative page faults or range-lock instad of mmap_sem, the situation would be much improved. Anyway, currently, I have no idea to overcome such prioriy inversion problem from rw_semaphore and I believe generally changing from down_read to down_write is good thing to prevent contention. Ccing maintainers.
