nullfs: vop_rename: fdvp is locked but should not be

[Kostik Belousov]

On Tue, Jun 01, 2010 at 04:57:57PM +0300, Mikolaj Golub wrote:
> Hi,
> 
> Having a kernel compiled with DEBUG_VFS_LOCKS and doing something like below:
> 
>   mount -t nullfs /tmp/1 /tmp/2 
>   mv /tmp/1/test.file /tmp/2/

cp /tmp/1/test.file /tmp/2/test.file is even more spectacular, but is
the different story.

> 
> I have a panic (checked on 8-STABLE and CURRENT):
> 
> vop_rename: fdvp locked: 0xc70c82b0 is locked but should not be
> KDB: enter: lock violation
> 
> (kgdb) bt
> #0  doadump () at pcpu.h:246
> #1  0xc04ed519 in db_fncall (dummy1=1, dummy2=0, dummy3=-1056863104, dummy4=0xe858a8d4 "")
>     at /usr/src/sys/ddb/db_command.c:548
> #2  0xc04ed911 in db_command (last_cmdp=0xc0e195dc, cmd_table=0x0, dopager=1)
>     at /usr/src/sys/ddb/db_command.c:445
> #3  0xc04eda6a in db_command_loop () at /usr/src/sys/ddb/db_command.c:498
> #4  0xc04ef90d in db_trap (type=3, code=0) at /usr/src/sys/ddb/db_main.c:229
> #5  0xc08e9f66 in kdb_trap (type=3, code=0, tf=0xe858aa7c) at /usr/src/sys/kern/subr_kdb.c:535
> #6  0xc0bfcc3b in trap (frame=0xe858aa7c) at /usr/src/sys/i386/i386/trap.c:690
> #7  0xc0bddffb in calltrap () at /usr/src/sys/i386/i386/exception.s:165
> #8  0xc08ea0ea in kdb_enter (why=0xc0ce2dbf "vfslock", msg=0xc0ce2db0 "lock violation") at cpufunc.h:71
> #9  0xc094a881 in vfs_badlock (msg=0xc0ce2ddc "is locked but should not be", 
>     str=0xc0ce38b9 "vop_rename: fdvp locked", vp=0xc70c82b0) at /usr/src/sys/kern/vfs_subr.c:3701
> #10 0xc094e4f5 in assert_vop_unlocked (vp=0xc70c82b0, str=0xc0ce38b9 "vop_rename: fdvp locked")
>     at /usr/src/sys/kern/vfs_subr.c:3733
> #11 0xc094e7d7 in vop_rename_pre (ap=0xe858ac1c) at /usr/src/sys/kern/vfs_subr.c:3792
> #12 0xc0c16a46 in VOP_RENAME_APV (vop=0xc0df8960, a=0xe858ac1c) at vnode_if.c:1472
> #13 0xc0956d47 in kern_renameat (td=0xc6596a00, oldfd=-100, 
>     old=0xbfbfe953 <Address 0xbfbfe953 out of bounds>, newfd=-100, 
>     new=0xbfbfe968 <Address 0xbfbfe968 out of bounds>, pathseg=UIO_USERSPACE) at vnode_if.h:636
> #14 0xc0956ef6 in kern_rename (td=0xc6596a00, from=0xbfbfe953 <Address 0xbfbfe953 out of bounds>, 
>     to=0xbfbfe968 <Address 0xbfbfe968 out of bounds>, pathseg=UIO_USERSPACE)
>     at /usr/src/sys/kern/vfs_syscalls.c:3569
> #15 0xc0956f29 in rename (td=0xc6596a00, uap=0xe858acf8) at /usr/src/sys/kern/vfs_syscalls.c:3546
> #16 0xc0bfc370 in syscall (frame=0xe858ad38) at /usr/src/sys/i386/i386/trap.c:1111
> #17 0xc0bde090 in Xint0x80_syscall () at /usr/src/sys/i386/i386/exception.s:261
> #18 0x00000033 in ?? ()
> Previous frame inner to this frame (corrupt stack?)
> (kgdb) fr 11
> #11 0xc094e7d7 in vop_rename_pre (ap=0xe858ac1c) at /usr/src/sys/kern/vfs_subr.c:3792
> 3792                    ASSERT_VOP_UNLOCKED(a->a_fdvp, "vop_rename: fdvp locked");
> (kgdb) list
> 3787            ASSERT_VI_UNLOCKED(a->a_fvp, "VOP_RENAME");
> 3788            ASSERT_VI_UNLOCKED(a->a_fdvp, "VOP_RENAME");
> 3789
> 3790            /* Check the source (from). */
> 3791            if (a->a_tdvp != a->a_fdvp && a->a_tvp != a->a_fdvp)
> 3792                    ASSERT_VOP_UNLOCKED(a->a_fdvp, "vop_rename: fdvp locked");
> 3793            if (a->a_tvp != a->a_fvp)
> 3794                    ASSERT_VOP_UNLOCKED(a->a_fvp, "vop_rename: fvp locked");
> 3795
> 3796            /* Check the target. */
> 
> On kernels without DEBUG_VFS_LOCKS it looks like mv does not cause any
> problems, although the behavior differs from moving the file to itself on ufs:
> on ufs the file remains unchanged, while when moving like in the example above
> the file disappears.
> 
> So, do we have false ASSERT here and additional check should be performed
> before calling ASSERT_VOP_UNLOCKED(), e.g. checking that
> 
>  ((struct null_node*)(a->a_fdvp->v_data))->null_lowervp != a->a_fdvp?
> 
> Is this worth adding such check?
I am curious to look at the final patch. Note that you proposing to add
fs-specific check to vfs_subr.c. Checking that the the vnode locks are
different instead of that vnodes itself are different might be better
approach for vop_rename_pre.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20100601/650408bb/attachment.pgp