Messages in this thread | | | Date | Thu, 16 May 2013 13:08:17 -0400 | Subject | Re: [PATCH -next] ashmem: Fix ashmem_shrink deadlock. | From | Robert Love <> |
| |
On Thu, May 16, 2013 at 12:45 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > A better approach would be to add a new __GFP_NOSHRINKERS, but it's all > variations on a theme.
I don't like this proposal, either. Many of the existing GFP flags already exist to prevent recurse into that flag's respective shrinker.
This problem seems a rare proper use of mutex_trylock.
> The mutex_trylock(ashmem_mutex) will actually have the best > performance, because it skips the least amount of memory reclaim > opportunities.
Right.
> But it still sucks! The real problem is that there exists a lock > called "ashmem_mutex", taken by both the high-level mmap() and by the > low-level shrinker. And taken by everything else too! The ashmem > locking is pretty crude...
The locking is "crude" because I optimized for space, not time, and there was (and is) no indication we were suffering lock contention due to the global lock. I haven't thought through the implications of pushing locking into the ashmem_area and ashmem_range objects, but it does look like we'd end up often grabbing all of the locks ...
> What is the mutex_lock() in ashmem_mmap() actually protecting? I don't > see much, apart from perhaps some incidental races around the contents > of the file's ashmem_area, and those could/should be protected by a > per-object lock, not a global one?
... but not, as you note, in ashmem_mmap. The main race there is around the allocation of asma->file. That could definitely be a lock local to ashmem_area. I'm OK if anyone wants to take that on but it seems a lot of work for a driver with an unclear future.
Robert
| |