Re: vn_fullpath: 0xc85e24a0 is not locked but should be

On 12 Dec, Jun Kuriyama wrote:
> 
> As of today's -current with SCHED_4BSD.  I saved "ps" output on ddb,
> so I can place it somewhere if somebody wants.
> 
> # Why I got so many panics?  :-(
> 
> 
> ----- console
> vn_fullpath: 0xc85e24a0 is not locked but should be
> Debugger("Lock violation.
> ")
> Stopped at      Debugger+0x55:  xchgl   %ebx,in_Debugger.0
> db> trace
> Debugger(c0739a77,c0739010,c85e24a0,c0739ab8,ece05710) at Debugger+0x55
> vfs_badlock(c0739ab8,c0739010,c85e24a0,c07a38c0,c85e24a0) at vfs_badlock+0x45
> assert_vop_locked(c85e24a0,c0739010,2,246,c93e0200) at assert_vop_locked+0x62
> vn_fullpath(c8e12500,c85e24a0,ece05754,ece05758,c0750da7) at vn_fullpath+0x81
> procfs_doprocfile(c8e12500,c9c7054c,c8295e00,ece0577c,0) at procfs_doprocfile+0x3a
> pfs_readlink(ece05bd0,c0739563,c0739548,a6,c8e12500) at pfs_readlink+0x11b
> namei(ece05c28,c07b4bb8,c07b4b40,c07b2d00,607) at namei+0x43a
> stat(c8e12500,ece05d14,c0752fc5,3ee,2) at stat+0x52
> syscall(2f,2f,2f,bfbfe270,bfbfe350) at syscall+0x2c0
> Xint0x80_syscall() at Xint0x80_syscall+0x1d
> --- syscall (188, FreeBSD ELF32, stat), eip = 0x280cb47f, esp = 0xbfbfe24c, ebp = 0xbfbfe7c8 ---
> db> show locks
> exclusive sleep mutex Giant r = 0 (0xc07ae560) locked _at_ i386/i386/trap.c:1000
> db> show lockedvnods
> Locked vnodes
> 0xc9d1e5c8: tag pseudofs, type VLNK, usecount 1, writecount 0, refcount 0, lock type pseudofs: EXCL (count 1) by thread 0xc8e12500 (pid:19220)
> 
> db> panic
> panic: from debugger

It is safe to tell the debugger to continue at this point.  The lock
assertion violations are not fatal.

BTW, I reported this same problem back in June ...

------ Forwarded message ------
    From: Don Lewis <truckman_at_freebsd.org>
 Subject: vnode locking problem in pseudofs/procfs
    Date: Sun, 1 Jun 2003 02:42:30 -0700 (PDT)
      To: current_at_freebsd.org

I just stumbled across this vnode locking problem in procfs()

db> tr
Debugger(c05215d4,c0520b94,c669b000,c0521615,e6d77764) at Debugger+0x54
vfs_badlock(c0521615,c0520b94,c669b000,c05b4340,c669b000) at vfs_badlock+0x45
assert_vop_locked(c669b000,c0520b94,c0520adf,358,c6a35400) at assert_vop_locked+
0x62
vn_fullpath(c66ce390,c669b000,e6d777a8,e6d777ac,c04ea6fb) at vn_fullpath+0xbc
procfs_doprocfile(c66ce390,c61c2960,c6272000,e6d777d4,0) at procfs_doprocfile+0x
3a
pfs_readlink(e6d77c10,c05210fd,c05b49c0,c6b2e920,e6d77c94) at pfs_readlink+0x116
VOP_READLINK(c6b2e920,e6d77c94,c6911d80,bfbed240,400) at VOP_READLINK+0x59
kern_readlink(c66ce390,bfbed650,0,bfbed240,0) at kern_readlink+0xc1
readlink(c66ce390,e6d77d10,c0535a6a,3fb,3) at readlink+0x38
syscall(2f,2f,2f,8134400,bfbedfd0) at syscall+0x26e
Xint0x80_syscall() at Xint0x80_syscall+0x1d
--- syscall (58, FreeBSD ELF32, readlink), eip = 0x8058a73, esp = 0xbfbed21c, eb
p = 0xbfbeda68 ---


when I ran
	find / -print0 | xargs -0 ls -l
with the DEBUG_VFS_LOCKS kernel option.

The obvious part of the fix is to lock the vnode in procfs_doprocfile()
before calling vn_fullpath().  The more interesting question is if this
means that all callers of (pn->pn_func)() need to drop their vnode locks
in order to prevent a potential deadlock.  It looks to me like this is
necessary ...