Date: Fri, 10 Aug 2007 11:35:06 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Xin LI <delphij@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: [src] cvs commit: src/sys/fs/tmpfs tmpfs.h tmpfs_subr.c tmpfs_vfsops.c tmpfs_vnops.c Message-ID: <20070810083506.GS2738@deviant.kiev.zoral.com.ua> In-Reply-To: <20070810052455.0B4B716A4F4@hub.freebsd.org> References: <200708100524.l7A5OnE4065132@repoman.freebsd.org> <20070810052455.0B4B716A4F4@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--UPvCt0eiAD9X+G9u
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Aug 10, 2007 at 05:24:54AM +0000, Xin LI wrote:
> delphij 2007-08-10 05:24:49 UTC
>=20
> FreeBSD src repository
>=20
> Modified files:
> sys/fs/tmpfs tmpfs.h tmpfs_subr.c tmpfs_vfsops.c=20
> tmpfs_vnops.c=20
> Log:
> MFp4:
> =20
> - Respect cnflag and don't lock vnode always as LK_EXCLUSIVE [1]
> - Properly lock around tn_vnode to avoid NULL deference
> - Be more careful handling vnodes (*)
> =20
> (*) This is a WIP
> [1] by pjd via howardsu
> =20
> Thanks kib@ for his valuable VFS related comments.
> =20
> Tested with: fsx, fstest, tmpfs regression test set
> Found by: pho's stress2 suite
> Approved by: re (tmpfs blanket)
> =20
> Revision Changes Path
> 1.10 +2 -2 src/sys/fs/tmpfs/tmpfs.h
> 1.11 +28 -19 src/sys/fs/tmpfs/tmpfs_subr.c
> 1.9 +5 -2 src/sys/fs/tmpfs/tmpfs_vfsops.c
> 1.9 +34 -13 src/sys/fs/tmpfs/tmpfs_vnops.c
> Index: src/sys/fs/tmpfs/tmpfs_subr.c
> diff -u src/sys/fs/tmpfs/tmpfs_subr.c:1.10 src/sys/fs/tmpfs/tmpfs_subr.c:=
1.11
> --- src/sys/fs/tmpfs/tmpfs_subr.c:1.10 Fri Aug 3 06:24:31 2007
> +++ src/sys/fs/tmpfs/tmpfs_subr.c Fri Aug 10 05:24:49 2007
> @@ -302,15 +308,20 @@
> * Returns zero on success or an appropriate error code on failure.
> */
> int
> -tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, struct vnode *=
*vpp,
> - struct thread *td)
> +tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, int lkflag,
> + struct vnode **vpp, struct thread *td)
> {
> int error;
> struct vnode *vp;
> =20
> loop:
> + TMPFS_NODE_LOCK(node);
> if ((vp =3D node->tn_vnode) !=3D NULL) {
> - error =3D vget(vp, LK_EXCLUSIVE | LK_RETRY, td);
> + VI_LOCK(vp);
> + TMPFS_NODE_UNLOCK(node);
> + vholdl(vp);
> + error =3D vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, td);
> + vdrop(vp);
> if (error)
> return error;
LK_RETRY prohibits the vget() and vn_lock() to return error.
> Index: src/sys/fs/tmpfs/tmpfs_vnops.c
> diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.8 src/sys/fs/tmpfs/tmpfs_vnops.c=
:1.9
> --- src/sys/fs/tmpfs/tmpfs_vnops.c:1.8 Thu Jul 19 03:34:50 2007
> +++ src/sys/fs/tmpfs/tmpfs_vnops.c Fri Aug 10 05:24:49 2007
> @@ -95,12 +95,20 @@
> !(cnp->cn_flags & ISDOTDOT)));
> =20
> if (cnp->cn_flags & ISDOTDOT) {
> - VOP_UNLOCK(dvp, 0, td);
> + int ltype =3D 0;
> =20
> + ltype =3D VOP_ISLOCKED(dvp, td);
> + vholdl(dvp);
vholdl() assumes that vnode interlock is locked, not vnode itself. You shall
use vhold() when not holding vnode interlock.
Later, you use dvp->v_mount, that could change to NULL once you dropped the
dvp lock. Moreover, filesystem may be unmounted after you dropped vnode
lock. UFS has the same problem.
> + VOP_UNLOCK(dvp, 0, td);
> /* Allocate a new vnode on the matching entry. */
> - error =3D tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent, vpp, t=
d);
> + error =3D tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent,
> + cnp->cn_lkflags, vpp, td);
> =20
> - vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
> + vn_lock(dvp, ltype | LK_RETRY, td);
> + if (ltype & LK_INTERLOCK)
> + vdropl(dvp);
> + else
> + vdrop(dvp);
ltype cannot contain LK_INTERLOCK flag. Moreover, vn_lock() always returns
with interlock being dropped. You shall simply vdrop(dvp) there.
> =20
> dnode->tn_dir.tn_parent->tn_lookup_dirent =3D NULL;
> } else if (cnp->cn_namelen =3D=3D 1 && cnp->cn_nameptr[0] =3D=3D '.') {
--UPvCt0eiAD9X+G9u
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (FreeBSD)
iD8DBQFGvCM6C3+MBN1Mb4gRAvnKAJ4yBFu81yAdm3/aRM/mTEACvDJMXgCgpqJZ
i3Ykfautlu8xg7/BDasOMYk=
=rpuw
-----END PGP SIGNATURE-----
--UPvCt0eiAD9X+G9u--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070810083506.GS2738>