On Mon, 2014-05-26 at 12:35 -0700, Hugh Dickins wrote: > On Thu, 22 May 2014, Davidlohr Bueso wrote: > > > Similarly to rmap_walk_anon() and collect_procs_anon(), > > there is opportunity to share the lock in rmap_walk_file() > > and collect_procs_file() for file backed pages. > > And lots of other places, no? I welcome i_mmap_rwsem, but I think > you're approaching it wrongly to separate this off from 2/5, then > follow anon_vma for the places that can be converted to lock_read(). Sure, but as you can imagine, the reasoning behind it is simplicity and bisectability. 2/5 is easy to commit typo-like errors, and end up locking instead of unlocking and vice versa. I ran into a few while testing and wanted to make life easier for reviewers. > If you go back through 2/5 and study the context of each, I think > you'll find most make no modification to the tree, and can well > use the lock_read() rather than the lock_write(). I was planning on revisiting some of that. I have no concrete examples yet, but I agree, there could very well be further opportunity to share the lock in read-only paths. This 4/5 is just the first, and most obvious, step towards improving the usage of the i_mmap lock. > I could be wrong, but I don't think there are any hidden gotchas. > There certainly are in the anon_vma case (where THP makes special > use of the anon_vma lock), and used to be in the i_mmap_lock case > (when invalidation had to be single-threaded across cond_rescheds), > but I think i_mmap_rwsem should be straightforward. > > Sure, it's safe to use the lock_write() variant, but please don't > prefer it to lock_read() without good reason. I will dig deeper (probably for 3.17 now), but I really believe this is the correct way of splitting the patches for this particular series. Thanks, Davidlohr -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>