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
--Tig5KXJwt3YyVVnR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 01, 2010 at 04:57:57PM +0300, Mikolaj Golub wrote: > Hi, >=20 > Having a kernel compiled with DEBUG_VFS_LOCKS and doing something like be= low: >=20 > mount -t nullfs /tmp/1 /tmp/2=20 > 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. >=20 > I have a panic (checked on 8-STABLE and CURRENT): >=20 > vop_rename: fdvp locked: 0xc70c82b0 is locked but should not be > KDB: enter: lock violation >=20 > (kgdb) bt > #0 doadump () at pcpu.h:246 > #1 0xc04ed519 in db_fncall (dummy1=3D1, dummy2=3D0, dummy3=3D-1056863104= , dummy4=3D0xe858a8d4 "") > at /usr/src/sys/ddb/db_command.c:548 > #2 0xc04ed911 in db_command (last_cmdp=3D0xc0e195dc, cmd_table=3D0x0, do= pager=3D1) > 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=3D3, code=3D0) at /usr/src/sys/ddb/db_mai= n.c:229 > #5 0xc08e9f66 in kdb_trap (type=3D3, code=3D0, tf=3D0xe858aa7c) at /usr/= src/sys/kern/subr_kdb.c:535 > #6 0xc0bfcc3b in trap (frame=3D0xe858aa7c) at /usr/src/sys/i386/i386/tra= p.c:690 > #7 0xc0bddffb in calltrap () at /usr/src/sys/i386/i386/exception.s:165 > #8 0xc08ea0ea in kdb_enter (why=3D0xc0ce2dbf "vfslock", msg=3D0xc0ce2db0= "lock violation") at cpufunc.h:71 > #9 0xc094a881 in vfs_badlock (msg=3D0xc0ce2ddc "is locked but should not= be",=20 > str=3D0xc0ce38b9 "vop_rename: fdvp locked", vp=3D0xc70c82b0) at /usr/= src/sys/kern/vfs_subr.c:3701 > #10 0xc094e4f5 in assert_vop_unlocked (vp=3D0xc70c82b0, str=3D0xc0ce38b9 = "vop_rename: fdvp locked") > at /usr/src/sys/kern/vfs_subr.c:3733 > #11 0xc094e7d7 in vop_rename_pre (ap=3D0xe858ac1c) at /usr/src/sys/kern/v= fs_subr.c:3792 > #12 0xc0c16a46 in VOP_RENAME_APV (vop=3D0xc0df8960, a=3D0xe858ac1c) at vn= ode_if.c:1472 > #13 0xc0956d47 in kern_renameat (td=3D0xc6596a00, oldfd=3D-100,=20 > old=3D0xbfbfe953 <Address 0xbfbfe953 out of bounds>, newfd=3D-100,=20 > new=3D0xbfbfe968 <Address 0xbfbfe968 out of bounds>, pathseg=3DUIO_US= ERSPACE) at vnode_if.h:636 > #14 0xc0956ef6 in kern_rename (td=3D0xc6596a00, from=3D0xbfbfe953 <Addres= s 0xbfbfe953 out of bounds>,=20 > to=3D0xbfbfe968 <Address 0xbfbfe968 out of bounds>, pathseg=3DUIO_USE= RSPACE) > at /usr/src/sys/kern/vfs_syscalls.c:3569 > #15 0xc0956f29 in rename (td=3D0xc6596a00, uap=3D0xe858acf8) at /usr/src/= sys/kern/vfs_syscalls.c:3546 > #16 0xc0bfc370 in syscall (frame=3D0xe858ad38) 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=3D0xe858ac1c) at /usr/src/sys/kern/v= fs_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 !=3D a->a_fdvp && a->a_tvp !=3D a->a_fdvp) > 3792 ASSERT_VOP_UNLOCKED(a->a_fdvp, "vop_rename: fdvp = locked"); > 3793 if (a->a_tvp !=3D a->a_fvp) > 3794 ASSERT_VOP_UNLOCKED(a->a_fvp, "vop_rename: fvp lo= cked"); > 3795 > 3796 /* Check the target. */ >=20 > 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. >=20 > So, do we have false ASSERT here and additional check should be performed > before calling ASSERT_VOP_UNLOCKED(), e.g. checking that >=20 > ((struct null_node*)(a->a_fdvp->v_data))->null_lowervp !=3D a->a_fdvp? >=20 > 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. --Tig5KXJwt3YyVVnR Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkwFFBkACgkQC3+MBN1Mb4jlcwCdFXZQnk/fPdrpX6P1xe5Bl8ik p88AoJsE9mky1vU4FleX0h93muiRzhlR =TrO7 -----END PGP SIGNATURE----- --Tig5KXJwt3YyVVnR--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100601140721.GH83316>