On Fri, 2018-10-26 at 10:43 -0700, Matthew Wilcox wrote: +AD4 On Fri, Oct 26, 2018 at 09:49:05AM -0700, Bart Van Assche wrote: +AD4 +AD4 +-+-+- b/include/linux/rwsem.h +AD4 +AD4 +AEAAQA -41,6 +-41,10 +AEAAQA struct rw+AF8-semaphore +AHs +AD4 +AD4 +ACM-endif +AD4 +AD4 +ACM-ifdef CONFIG+AF8-DEBUG+AF8-LOCK+AF8-ALLOC +AD4 +AD4 struct lockdep+AF8-map dep+AF8-map+ADs +AD4 +AD4 +- /+ACo +AD4 +AD4 +- +ACo Number of up+AF8-write() calls that must skip rwsem+AF8-release(). +AD4 +AD4 +- +ACo-/ +AD4 +AD4 +- unsigned nolockdep+ADs +AD4 +AD4 This reads a bit weird. By definition, only one writer is allowed +AD4 at a time. And you can't call up+AF8-write() before down+AF8-write(). +AD4 So functionally, this is a bool, and the comment should at least +AD4 ackowledge that, even if it remains implemented as an unsigned int. +AD4 +AD4 I'd suggest the implementation uses '+AD0 1' and '+AD0 0' rather than +-+- and --. Hi Matthew, That sounds like a good idea to me. +AD4 +AD4 diff --git a/mm/rmap.c b/mm/rmap.c +AD4 +AD4 index 1e79fac3186b..2a953d3b7431 100644 +AD4 +AD4 --- a/mm/rmap.c +AD4 +AD4 +-+-+- b/mm/rmap.c +AD4 +AD4 +AEAAQA -81,6 +-81,7 +AEAAQA static inline struct anon+AF8-vma +ACo-anon+AF8-vma+AF8-alloc(void) +AD4 +AD4 +AD4 +AD4 anon+AF8-vma +AD0 kmem+AF8-cache+AF8-alloc(anon+AF8-vma+AF8-cachep, GFP+AF8-KERNEL)+ADs +AD4 +AD4 if (anon+AF8-vma) +AHs +AD4 +AD4 +- init+AF8-rwsem(+ACY-anon+AF8-vma-+AD4-rwsem)+ADs +AD4 +AD4 atomic+AF8-set(+ACY-anon+AF8-vma-+AD4-refcount, 1)+ADs +AD4 +AD4 anon+AF8-vma-+AD4-degree +AD0 1+ADs /+ACo Reference for first vma +ACo-/ +AD4 +AD4 anon+AF8-vma-+AD4-parent +AD0 anon+AF8-vma+ADs +AD4 +AD4 Why is this needed? The anon+AF8-vma+AF8-ctor() already calls init+AF8-rwsem(). +AD4 +AD4 (I suspect this is one of those ctors that isn't actually useful and +AD4 should be inlined into anon+AF8-vma+AF8-alloc()) Without that call I noticed that the +ACI-nolockdep+ACI variable was sometimes set when down+AF8-write() got called. Does that mean that it can happen that an anon+AF8-vma structure is freed without releasing anon+AF8-vma-+AD4-rwsem? Thanks, Bart.
