Date: Fri, 15 Mar 2002 06:21:49 -0500 From: "Brian F. Feldman" <green@FreeBSD.org> To: Alfred Perlstein <bright@mu.org> Cc: Brian Feldman <green@FreeBSD.org>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_mtxpool.c src/sys/sys kernel.h src/sys/vm vm_fault.c vm_glue.c vm_map.c vm_map.h vm_pageout.c vm_zone.c Message-ID: <200203151121.g2FBLnj36094@green.bikeshed.org> In-Reply-To: Message from Alfred Perlstein <bright@mu.org> of "Thu, 14 Mar 2002 21:41:13 PST." <20020315054113.GC4857@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Alfred Perlstein <bright@mu.org> wrote:
> * Brian Feldman <green@FreeBSD.org> [020313 15:53] wrote:
> > green 2002/03/13 15:48:08 PST
> >
> > Modified files:
> > sys/kern kern_mtxpool.c
> > sys/sys kernel.h
> > sys/vm vm_fault.c vm_glue.c vm_map.c vm_map.h
> > vm_pageout.c vm_zone.c
> > Log:
> > Rename SI_SUB_MUTEX to SI_SUB_MTX_POOL to make the name at all accurate.
> > While doing this, move it earlier in the sysinit boot process so that the
> > VM system can use it.
> >
> > After that, the system is now able to use sx locks instead of lockmgr
> > locks in the VM system. To accomplish this, some of the more
> > questionable uses of the locks (such as testing whether they are
> > owned or not, as well as allowing shared+exclusive recursion) are
> > removed, and simpler logic throughout is used so locks should also be
> > easier to understand.
> >
> > This has been tested on my laptop for months, and has not shown any
> > problems on SMP systems, either, so appears quite safe. One more
> > user of lockmgr down, many more to go :)
>
> You broke the kernel.
>
> Two deadlock tracebacks:
>
> siointr1(c5ed2400,c0464940,0,c03aa0e3,662) at siointr1+0xb1
> siointr(c5ed2400) at siointr+0x23
> Xfastintr4() at Xfastintr4+0x34
> --- interrupt, eip = 0xc03139f5, esp = 0xd26968e8, ebp = 0xd26968f4 ---
> _vm_map_lock_upgrade(c045b870,c03a72c1,af4) at _vm_map_lock_upgrade+0xd
> vm_map_lookup(d269699c,cc644000,2,d26969a0,d2696994) at vm_map_lookup+0x19f
> vm_fault1(c045bb54,cc644000,2,0,c04098c0) at vm_fault1+0x54
> vm_fault(c045bb54,cc644000,2,0,c) at vm_fault+0x34
> trap_pfault(d2696a8c,0,cc644000,d2696bd4,80f5349) at trap_pfault+0x161
> trap(c03b0018,d2690010,c0400010,cc644000,80f5349) at trap+0x36f
> calltrap() at calltrap+0x5
> --- trap 0xc, eip = 0xc03474e4, esp = 0xd2696acc, ebp = 0xd2696af4 ---
> copyinstr(d2696bd4,2,0,d2696d20,0) at copyinstr+0x38
> exec_elf_imgact(d2696bd4,d25ac580,0,3,d2696bd4) at exec_elf_imgact+0xf3
> execve(d25ac680,d2696d20,80f5358,ffffffff,80f53fc) at execve+0x2e0
> syscall(2f,2f,2f,80f53fc,ffffffff) at syscall+0x207
> syscall_with_err_pushed() at syscall_with_err_pushed+0x1b
>
> witness_unlock(c042d7f8,8,c038cfe3,112) at witness_unlock+0x1c5
> _mtx_unlock_flags(c042d7f8,0,c038cfe3,112,c045b870) at _mtx_unlock_flags+0x1d
> _sx_try_upgrade(c045b8a0,c03a72c1,af4,d257b570,d2546924) at _sx_try_upgrade+0x38
> _vm_map_lock_upgrade(c045b870,c03a72c1,af4) at _vm_map_lock_upgrade+0x16
> vm_map_lookup(d254699c,cc5ed000,2,d25469a0,d2546994) at vm_map_lookup+0x19f
> vm_fault1(c045bb54,cc5ed000,2,0,c04098c0) at vm_fault1+0x54
> vm_fault(c045bb54,cc5ed000,2,0,c) at vm_fault+0x34
> trap_pfault(d2546a8c,0,cc5ed000,d2546bd4,bfbffe69) at trap_pfault+0x161
> trap(18,d2540010,c0340010,cc5ed000,bfbffe69) at trap+0x36f
> calltrap() at calltrap+0x5
> --- trap 0xc, eip = 0xc03474e4, esp = 0xd2546acc, ebp = 0xd2546af4 ---
> copyinstr(d2546bd4,2,0,d2546d20,0) at copyinstr+0x38
> exec_elf_imgact(d2546bd4,cf8ce260,0,3,d2546bd4) at exec_elf_imgact+0xf3
> execve(cf8ce360,d2546d20,28105658,bfbff940,4) at execve+0x2e0
> syscall(2f,2f,2f,4,bfbff940) at syscall+0x207
> syscall_with_err_pushed() at syscall_with_err_pushed+0x1b
>
> What is the problem?
Damn good question. Are the tracebacks related? If not, what are you
supposed to be telling me it's deadlocking on? I don't see the system being
deadlocked. What is it actually supposed to be blocked on?
> Well basically you changed:
>
> ! static __inline__ int
> ! _vm_map_lock_upgrade(vm_map_t map, struct thread *td) {
> ! int error;
> !
> ! vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map);
> ! error = lockmgr(&map->lock, LK_EXCLUPGRADE, NULL, td);
> ! if (error == 0)
> ! map->timestamp++;
> ! return error;
> }
>
> into:
>
> ! _vm_map_lock_upgrade(vm_map_t map, const char *file, int line)
> {
> ! vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map);
> ! if (_sx_try_upgrade(&map->lock, file, line)) {
> ! map->timestamp++;
> ! return (0);
> ! }
> ! return (EWOULDBLOCK);
> }
It doesn't need LK_EXCLUPGRADE semantics, only LK_UPGRADE, if it's not
blocking. It backs out completely and unlocks the shared reference and
tries for an exclusive lock.
> It's obvious you didn't understand what's going on here.
>
> Please either fix or back this code out.
Yes, I'm sure it's so blindingly obvious from the two tracebacks you posted
which look SO MUCH like deadlocks...
--
Brian Fundakowski Feldman \'[ FreeBSD ]''''''''''\
<> green@FreeBSD.org <> bfeldman@tislabs.com \ The Power to Serve! \
Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203151121.g2FBLnj36094>