Re: [PATCH RFC] kernel/locking, fs/direct-io: Introduce and use down_write_nolockdep()

[Bart Van Assche <bvanassche@acm.org>]

On Fri, 2018-10-26 at 11:38 -0700, Matthew Wilcox wrote:
> On Fri, Oct 26, 2018 at 11:11:18AM -0700, Bart Van Assche wrote:
> > On Fri, 2018-10-26 at 10:43 -0700, Matthew Wilcox wrote:
> > > On Fri, Oct 26, 2018 at 09:49:05AM -0700, Bart Van Assche wrote:
> > > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > > index 1e79fac3186b..2a953d3b7431 100644
> > > > --- a/mm/rmap.c
> > > > +++ b/mm/rmap.c
> > > > @@ -81,6 +81,7 @@ static inline struct anon_vma *anon_vma_alloc(void)
> > > >  
> > > >  	anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL);
> > > >  	if (anon_vma) {
> > > > +		init_rwsem(&anon_vma->rwsem);
> > > >  		atomic_set(&anon_vma->refcount, 1);
> > > >  		anon_vma->degree = 1;	/* Reference for first vma */
> > > >  		anon_vma->parent = anon_vma;
> > > 
> > > Why is this needed?  The anon_vma_ctor() already calls init_rwsem().
> > > 
> > > (I suspect this is one of those ctors that isn't actually useful and
> > > should be inlined into anon_vma_alloc())
> > 
> > Without that call I noticed that the "nolockdep" variable was sometimes set
> > when down_write() got called. Does that mean that it can happen that an
> > anon_vma structure is freed without releasing anon_vma->rwsem?
> 
> How strange.  The only call to down_write_nolockdep() you added (in this
> patch) was for the inode->i_mutex.  So how could that possibly affect
> the anon_vma->rwsem?  Are you seeing some kind of corruption here?
> 
> Maybe try initialising ->nolockdep with some 32-bit magic value,
> and reporting if it's not 0 or the magic value will lead to some kind
> of insight?

Hi Matthew,

If I remove the init_rwsem() call shown above from mm/rmap.c the following
appears in the kernel log:

WARNING: CPU: 1 PID: 143 at kernel/locking/rwsem.c:102 down_write+0x4d/0x60
Modules linked in:
CPU: 1 PID: 143 Comm: kworker/u12:3 Not tainted 4.19.0-dbg+ #20
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
RIP: 0010:down_write+0x4d/0x60
Code: e8 88 6d 30 ff 48 89 df e8 90 ef 2f ff 48 8d bb 80 00 00 00 e8 c4 58 56 ff 8b 93 80 00 00 00 58 85 d2 75 06 48 8b 5d f8 c9 c3 <0f> 0b 48 8b 5d f8 c9 c3 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44
RSP: 0000:ffff88010e2d7970 EFLAGS: 00010202
RAX: ffffffff8137f2b9 RBX: ffff8801170f9a58 RCX: ffffffff81e550dc
RDX: 0000000000000001 RSI: dffffc0000000000 RDI: ffff8801170f9ad8
RBP: ffff88010e2d7978 R08: 0000000000000001 R09: 0000000000000000
R10: ffff88010e2d78f0 R11: ffffed0022e1f35c R12: ffff880113f3cc60
R13: ffff88010e0e93f0 R14: ffff8801170f84e0 R15: ffff8801170f9a50
FS:  0000000000000000(0000) GS:ffff88011b640000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000002c13001 CR4: 00000000003606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 __anon_vma_prepare+0x89/0x230
 __handle_mm_fault+0x1463/0x1590
 handle_mm_fault+0x20c/0x4d0
 __get_user_pages+0x302/0x960
 get_user_pages_remote+0x137/0x1f0
 copy_strings.isra.23+0x31a/0x600
 copy_strings_kernel+0x6b/0xa0
 __do_execve_file.isra.35+0xb60/0x1120
 do_execve+0x25/0x30
 call_usermodehelper_exec_async+0x26e/0x280
 ret_from_fork+0x24/0x30
irq event stamp: 64
hardirqs last  enabled at (63): [<ffffffff813b5e65>] __slab_alloc.isra.56+0x65/0x90
hardirqs last disabled at (64): [<ffffffff81002768>] trace_hardirqs_off_thunk+0x1a/0x1c
softirqs last  enabled at (0): [<ffffffff810b3952>] copy_process.part.34+0xb52/0x3af0
softirqs last disabled at (0): [<0000000000000000>]           (null)

Please let me know if you need more information.

Bart.