Date: Tue, 1 Jun 2010 17:07:21 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Mikolaj Golub <to.my.trociny@gmail.com> Cc: freebsd-fs@freebsd.org Subject: Re: nullfs: vop_rename: fdvp is locked but should not be Message-ID: <20100601140721.GH83316@deviant.kiev.zoral.com.ua> In-Reply-To: <86fx16onx6.fsf@zhuzha.ua1> References: <86fx16onx6.fsf@zhuzha.ua1>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] 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. [-- Attachment #2 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkwFFBkACgkQC3+MBN1Mb4jlcwCdFXZQnk/fPdrpX6P1xe5Bl8ik p88AoJsE9mky1vU4FleX0h93muiRzhlR =TrO7 -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100601140721.GH83316>