Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 Mar 2002 21:41:13 -0800
From:      Alfred Perlstein <bright@mu.org>
To:        Brian Feldman <green@FreeBSD.org>
Cc:        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:  <20020315054113.GC4857@elvis.mu.org>
In-Reply-To: <200203132348.g2DNmAE12640@freefall.freebsd.org>
References:  <200203132348.g2DNmAE12640@freefall.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
* 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?

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's obvious you didn't understand what's going on here.

Please either fix or back this code out.

-- 
-Alfred Perlstein [alfred@freebsd.org]
'Instead of asking why a piece of software is using "1970s technology,"
 start asking why software is ignoring 30 years of accumulated wisdom.'
Tax deductible donations for FreeBSD: http://www.freebsdfoundation.org/

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?20020315054113.GC4857>