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>