> Date: Thu, 17 Dec 2020 19:25:44 -0300
> From: Martin Pieuchot <m...@openbsd.org>
>
> On 17/12/20(Thu) 23:16, Mark Kettenis wrote:
> > > Date: Thu, 17 Dec 2020 18:56:52 -0300
> > > From: Martin Pieuchot <m...@openbsd.org>
> > >
> > > On 16/12/20(Wed) 22:49, Greg Steuck wrote:
> > > > I just hit this while booting an i386-current in vmd. The source tree is
> > > > synced to "Remove the assertion in uvm_km_pgremove()."
> > > >
> > > > I enabled WITNESS on top of GENERIC. Naturally, GENERIC-Dec15 snap
> > > > works.
> > > >
> > > > Anybody else see this so I know it's worth a bisect?
> > > > [...]
> > >
> > > I can reproduce it. Diff below fixes it. This is the beginning of a
> > > rabbit hole... thanks!
> > >
> > > > witness: lock_object uninitialized: 0xd0f3c828
> > > > Starting stack trace...
> > > > witness_checkorder(0,d6bb011c,d1155e6c,d02e10e4,90) at
> > > > witness_checkorder+0x8a
> > > > witness_checkorder(d0f3c828,9,0) at witness_checkorder+0x8a
> > > > mtx_enter(d0f3c81c) at mtx_enter+0x27
> > > > pmap_extract_pae(d8bb0d80,f5605000,d8bb0da0) at pmap_extract_pae+0x53
> > > > pmap_pinit_pd_pae(d8bb0d80) at pmap_pinit_pd_pae+0x268
> > > > pmap_create(1,1000,f6fe5e86,d8bbfd54,d0f5ba18) at pmap_create+0xa8
> > > > uvmspace_fork(d0f5b5fc,d8bb3e34,d0f5b5fc,1,d1155f70) at
> > > > uvmspace_fork+0x56
> > > > process_new(d8bb3e34,d0f5b5fc,1) at process_new+0xeb
> > > > fork1(d0eb7b14,1,d04eb560,0,0,d1155f90) at fork1+0x1ba
> > > > panic: acquiring blockable sleep lock with spinlock or critical section
> > > > held (rwlock) kmmaplk
> > >
> > > pmap_kernel()'s mutexes aren't initialized. Diff below does that.
> >
> > Well, that is somewhat intentional. Those mutexes should never be
> > used for the kernel pmap. The kernel pmap is always there and is
> > updated atomically.
> >
> > So how did we end up trying to grab one of these mutexs?
>
> pmap_map_ptes() (both version of them) grab the current's pmap
> `pm_apte_mtx' which ends up being the kernel one in this case.
Actually I think only pmap_pinit_pd_pae() runs into this problem,
because it does this:
if (!pmap_extract(pmap, va + i * NBPG,
(paddr_t *)&pmap->pm_pdidx_intel[i]))
panic("%s: can't locate PD page\n", __func__);
I'm womewhat surprized by this. Why are we extracting addresses out
of the freshly created pmap?
Anyway, your analysis is right. When a kernel thread wants to use
pmap_extract(9) on a userland pmap, it needs to lock pm_apte_mtx to
prevent another thread from simultaniously activating a userland pmap
too. So indeed, pm_apte_mtx needs to be properly initialized for the
kernel pmap.
However, pm_mtx should never be used for the kernel pmap. If we don't
initialize the lock, witness will help us catching this condition, so
maybe we shouldn't...
> > > Index: arch/i386/i386/pmap.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/arch/i386/i386/pmap.c,v
> > > retrieving revision 1.209
> > > diff -u -p -r1.209 pmap.c
> > > --- arch/i386/i386/pmap.c 24 Sep 2020 11:36:50 -0000 1.209
> > > +++ arch/i386/i386/pmap.c 17 Dec 2020 21:47:11 -0000
> > > @@ -961,6 +961,8 @@ pmap_bootstrap(vaddr_t kva_start)
> > > */
> > >
> > > kpm = pmap_kernel();
> > > + mtx_init(&kpm->pm_mtx, IPL_VM);
> > > + mtx_init(&kpm->pm_apte_mtx, IPL_VM);
> > > uvm_objinit(&kpm->pm_obj, NULL, 1);
> > > bzero(&kpm->pm_list, sizeof(kpm->pm_list)); /* pm_list not used */
> > > kpm->pm_pdir = (vaddr_t)(proc0.p_addr->u_pcb.pcb_cr3 + KERNBASE);
> > >
> > >
>
