Date: Fri, 29 Aug 1997 12:20:44 +0200 From: Poul-Henning Kamp <phk@critter.freebsd.dk> To: "Jordan K. Hubbard" <jkh@time.cdrom.com> Cc: current@freebsd.org Subject: Re: Another SNAPshot CD of 3.0 coming up.. Message-ID: <8608.872850044@critter.freebsd.dk> In-Reply-To: Your message of "Fri, 29 Aug 1997 02:16:49 PDT." <344.872846209@time.cdrom.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <344.872846209@time.cdrom.com>, "Jordan K. Hubbard" writes:
>The last 3.0 SNAP CD was done on May 22 and we're slightly overdue for
>the next one. Therefore, assuming that things aren't too badly broken
>or can't be fixed with a couple of week's work, consider this my
>early-warning announcement that another 3.0 SNAPshot for CD will be
>done in 10 days or so. "Smoke 'em if you've got 'em."
>
> Jordan
I have just sent the following vfs patch out for review to jdp,bde,
dyson & davidg. Considering this, I would like if more people would
jump on this patch and test it. I've run on it for weeks now with
no ill effects on my laptop
Please report any observations to me.
Change the 0xdeadb hack to a flag called VDOOMED.
Introduce VFREE which indicates that vnode is on freelist.
Rename vholdrele() to vdrop().
Create vfree() and vbusy() to add/delete vnode from freelist.
Add vfree()/vbusy() to keep (v_holdcnt != 0 || v_usecount != 0)
vnodes off the freelist.
Generalize vhold()/v_holdcnt to mean "do not recycle".
Fix reassignbuf()s lack of use of vhold().
Use vhold() instead of checking v_cache_src list.
Remove vtouch(), the vnodes are always vget'ed soon enough
after for it to have any measuable effect.
Add sysctl debug.freevnodes to keep track of things.
Move cache_purge() up in getnewvnodes to avoid race.
Decrement v_usecount after VOP_INACTIVE(), put a vhold() on
it during VOP_INACTIVE()
Unmacroize vhold()/vdrop()
Print out VDOOMED and VFREE flags (XXX: should use %b)
Poul-Henning
Index: kern/vfs_cache.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_cache.c,v
retrieving revision 1.27
diff -u -r1.27 vfs_cache.c
--- vfs_cache.c 1997/08/26 07:32:34 1.27
+++ vfs_cache.c 1997/08/27 14:24:59
@@ -103,6 +103,8 @@
{
LIST_REMOVE(ncp, nc_hash);
LIST_REMOVE(ncp, nc_src);
+ if (LIST_EMPTY(&ncp->nc_dvp->v_cache_src))
+ vdrop(ncp->nc_dvp);
if (ncp->nc_vp) {
TAILQ_REMOVE(&ncp->nc_vp->v_cache_dst, ncp, nc_dst);
} else {
@@ -180,7 +182,6 @@
/* We found a "positive" match, return the vnode */
if (ncp->nc_vp) {
nchstats.ncs_goodhits++;
- vtouch(ncp->nc_vp);
*vpp = ncp->nc_vp;
return (-1);
}
@@ -239,8 +240,10 @@
malloc(sizeof *ncp + cnp->cn_namelen, M_CACHE, M_WAITOK);
bzero((char *)ncp, sizeof *ncp);
numcache++;
- if (!vp)
+ if (!vp) {
numneg++;
+ ncp->nc_flag = cnp->cn_flags & ISWHITEOUT ? NCF_WHITE : 0;
+ }
/*
* Fill in cache info, if vp is NULL this is a "negative" cache entry.
@@ -249,15 +252,13 @@
* otherwise unused.
*/
ncp->nc_vp = vp;
- if (vp)
- vtouch(vp);
- else
- ncp->nc_flag = cnp->cn_flags & ISWHITEOUT ? NCF_WHITE : 0;
ncp->nc_dvp = dvp;
ncp->nc_nlen = cnp->cn_namelen;
bcopy(cnp->cn_nameptr, ncp->nc_name, (unsigned)ncp->nc_nlen);
ncpp = NCHHASH(dvp, cnp);
LIST_INSERT_HEAD(ncpp, ncp, nc_hash);
+ if (LIST_EMPTY(&dvp->v_cache_src))
+ vhold(dvp);
LIST_INSERT_HEAD(&dvp->v_cache_src, ncp, nc_src);
if (vp) {
TAILQ_INSERT_HEAD(&vp->v_cache_dst, ncp, nc_dst);
Index: kern/vfs_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.95
diff -u -r1.95 vfs_subr.c
--- vfs_subr.c 1997/08/26 11:59:20 1.95
+++ vfs_subr.c 1997/08/27 14:28:38
@@ -103,6 +103,7 @@
}
TAILQ_HEAD(freelst, vnode) vnode_free_list; /* vnode free list */
static u_long freevnodes = 0;
+SYSCTL_INT(_debug, OID_AUTO, freevnodes, CTLFLAG_RD, &freevnodes, 0, "");
struct mntlist mountlist; /* mounted filesystem list */
struct simplelock mountlist_slock;
@@ -380,11 +381,11 @@
}
if (vp) {
+ vp->v_flag |= VDOOMED;
TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
freevnodes--;
- /* see comment on why 0xdeadb is set at end of vgone (below) */
- vp->v_freelist.tqe_prev = (struct vnode **) 0xdeadb;
simple_unlock(&vnode_free_list_slock);
+ cache_purge(vp);
vp->v_lease = NULL;
if (vp->v_type != VBAD)
vgonel(vp, p);
@@ -418,13 +419,13 @@
M_VNODE, M_WAITOK);
bzero((char *) vp, sizeof *vp);
vp->v_dd = vp;
+ cache_purge(vp);
LIST_INIT(&vp->v_cache_src);
TAILQ_INIT(&vp->v_cache_dst);
numvnodes++;
}
vp->v_type = VNON;
- cache_purge(vp);
vp->v_tag = tag;
vp->v_op = vops;
insmntque(vp, mp);
@@ -582,7 +583,7 @@
if (bp->b_vp)
panic("bgetvp: not free");
- VHOLD(vp);
+ vhold(vp);
bp->b_vp = vp;
if (vp->v_type == VBLK || vp->v_type == VCHR)
bp->b_dev = vp->v_rdev;
@@ -618,7 +619,7 @@
vp = bp->b_vp;
bp->b_vp = (struct vnode *) 0;
- HOLDRELE(vp);
+ vdrop(vp);
}
/*
@@ -678,8 +679,10 @@
/*
* Delete from old vnode list, if on one.
*/
- if (bp->b_vnbufs.le_next != NOLIST)
+ if (bp->b_vnbufs.le_next != NOLIST) {
bufremvn(bp);
+ vdrop(bp->b_vp);
+ }
/*
* If dirty, put on list of dirty buffers; otherwise insert onto list
* of clean buffers.
@@ -700,6 +703,8 @@
} else {
bufinsvn(bp, &newvp->v_cleanblkhd);
}
+ bp->b_vp = newvp;
+ vhold(bp->b_vp);
splx(s);
}
@@ -836,13 +841,9 @@
tsleep((caddr_t)vp, PINOD, "vget", 0);
return (ENOENT);
}
- if (vp->v_usecount == 0) {
- simple_lock(&vnode_free_list_slock);
- TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
- simple_unlock(&vnode_free_list_slock);
- freevnodes--;
- }
vp->v_usecount++;
+ if (VSHOULDBUSY(vp))
+ vbusy(vp);
/*
* Create the VM object, if needed
*/
@@ -1089,11 +1090,11 @@
panic("vputrele: null vp");
#endif
simple_lock(&vp->v_interlock);
- vp->v_usecount--;
- if ((vp->v_usecount == 1) &&
+ if ((vp->v_usecount == 2) &&
vp->v_object &&
(vp->v_object->flags & OBJ_VFS_REF)) {
+ vp->v_usecount--;
vp->v_object->flags &= ~OBJ_VFS_REF;
if (put) {
VOP_UNLOCK(vp, LK_INTERLOCK, p);
@@ -1104,7 +1105,8 @@
return;
}
- if (vp->v_usecount > 0) {
+ if (vp->v_usecount > 1) {
+ vp->v_usecount--;
if (put) {
VOP_UNLOCK(vp, LK_INTERLOCK, p);
} else {
@@ -1113,23 +1115,14 @@
return;
}
- if (vp->v_usecount < 0) {
+ if (vp->v_usecount < 1) {
#ifdef DIAGNOSTIC
vprint("vputrele: negative ref count", vp);
#endif
panic("vputrele: negative ref cnt");
}
- simple_lock(&vnode_free_list_slock);
- if (vp->v_flag & VAGE) {
- vp->v_flag &= ~VAGE;
- if(vp->v_tag != VT_TFS)
- TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist);
- } else {
- if(vp->v_tag != VT_TFS)
- TAILQ_INSERT_TAIL(&vnode_free_list, vp, v_freelist);
- }
- freevnodes++;
- simple_unlock(&vnode_free_list_slock);
+
+ vp->v_holdcnt++; /* Make sure vnode isn't recycled */
/*
* If we are doing a vput, the node is already locked, and we must
@@ -1139,8 +1132,18 @@
if (put) {
simple_unlock(&vp->v_interlock);
VOP_INACTIVE(vp, p);
+ simple_lock(&vp->v_interlock);
+ vp->v_usecount--;
+ vp->v_holdcnt--;
+ if (VSHOULDFREE(vp))
+ vfree(vp);
+ simple_unlock(&vp->v_interlock);
} else if (vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK, p) == 0) {
VOP_INACTIVE(vp, p);
+ vp->v_usecount--;
+ vp->v_holdcnt--;
+ if (VSHOULDFREE(vp))
+ vfree(vp);
}
}
@@ -1161,9 +1164,8 @@
vputrele(vp, 0);
}
-#ifdef DIAGNOSTIC
/*
- * Page or buffer structure gets a reference.
+ * Somebody doesn't want the vnode recycled.
*/
void
vhold(vp)
@@ -1172,14 +1174,16 @@
simple_lock(&vp->v_interlock);
vp->v_holdcnt++;
+ if (VSHOULDBUSY(vp))
+ vbusy(vp);
simple_unlock(&vp->v_interlock);
}
/*
- * Page or buffer structure frees a reference.
+ * One less who cares about this vnode.
*/
void
-holdrele(vp)
+vdrop(vp)
register struct vnode *vp;
{
@@ -1187,9 +1191,10 @@
if (vp->v_holdcnt <= 0)
panic("holdrele: holdcnt");
vp->v_holdcnt--;
+ if (VSHOULDFREE(vp))
+ vfree(vp);
simple_unlock(&vp->v_interlock);
}
-#endif /* DIAGNOSTIC */
/*
* Remove any vnodes in the vnode table belonging to mount point mp.
@@ -1572,17 +1577,11 @@
* after calling vgone. If the reference count were
* incremented first, vgone would (incorrectly) try to
* close the previous instance of the underlying object.
- * So, the back pointer is explicitly set to `0xdeadb' in
- * getnewvnode after removing it from the freelist to ensure
- * that we do not try to move it here.
*/
- if (vp->v_usecount == 0) {
+ if (vp->v_usecount == 0 && !(vp->v_flag & VDOOMED)) {
simple_lock(&vnode_free_list_slock);
- if ((vp->v_freelist.tqe_prev != (struct vnode **)0xdeadb) &&
- vnode_free_list.tqh_first != vp) {
- TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
- TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist);
- }
+ TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
+ TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist);
simple_unlock(&vnode_free_list_slock);
}
@@ -1680,6 +1679,10 @@
strcat(buf, "|VBWAIT");
if (vp->v_flag & VALIASED)
strcat(buf, "|VALIASED");
+ if (vp->v_flag & VDOOMED)
+ strcat(buf, "|VDOOMED");
+ if (vp->v_flag & VFREE)
+ strcat(buf, "|VFREE");
if (buf[0] != '\0')
printf(" flags (%s)", &buf[1]);
if (vp->v_data == NULL) {
@@ -2255,20 +2258,28 @@
}
void
-vtouch(vp)
+vfree(vp)
struct vnode *vp;
{
- simple_lock(&vp->v_interlock);
- if (vp->v_usecount) {
- simple_unlock(&vp->v_interlock);
- return;
- }
- if (simple_lock_try(&vnode_free_list_slock)) {
- if (vp->v_freelist.tqe_prev != (struct vnode **)0xdeadb) {
- TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
- TAILQ_INSERT_TAIL(&vnode_free_list, vp, v_freelist);
- }
- simple_unlock(&vnode_free_list_slock);
+ simple_lock(&vnode_free_list_slock);
+ if (vp->v_flag & VAGE) {
+ TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist);
+ } else {
+ TAILQ_INSERT_TAIL(&vnode_free_list, vp, v_freelist);
}
- simple_unlock(&vp->v_interlock);
+ freevnodes++;
+ simple_unlock(&vnode_free_list_slock);
+ vp->v_flag &= ~VAGE;
+ vp->v_flag |= VFREE;
+}
+
+void
+vbusy(vp)
+ struct vnode *vp;
+{
+ simple_lock(&vnode_free_list_slock);
+ TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
+ freevnodes--;
+ simple_unlock(&vnode_free_list_slock);
+ vp->v_flag &= ~VFREE;
}
Index: sys/vnode.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/vnode.h,v
retrieving revision 1.46
diff -u -r1.46 vnode.h
--- vnode.h 1997/08/26 07:32:46 1.46
+++ vnode.h 1997/08/26 22:29:57
@@ -141,6 +141,8 @@
#define VAGE 0x08000 /* Insert vnode at head of free list */
#define VOLOCK 0x10000 /* vnode is locked waiting for an object */
#define VOWANT 0x20000 /* a process is waiting for VOLOCK */
+#define VDOOMED 0x40000 /* This vnode is being recycled */
+#define VFREE 0x80000 /* This vnode is on the freelist */
/*
* Vnode attributes. A field value of VNOVAL represents a field whose value
@@ -221,35 +223,12 @@
#define V_SAVEMETA 0x0002 /* vinvalbuf: leave indirect blocks */
#define REVOKEALL 0x0001 /* vop_revoke: revoke all aliases */
-#ifdef DIAGNOSTIC
-#define HOLDRELE(vp) holdrele(vp)
-#define VATTR_NULL(vap) vattr_null(vap)
-#define VHOLD(vp) vhold(vp)
#define VREF(vp) vref(vp)
-void holdrele __P((struct vnode *));
-void vhold __P((struct vnode *));
-void vref __P((struct vnode *vp));
+#ifdef DIAGNOSTIC
+#define VATTR_NULL(vap) vattr_null(vap)
#else
#define VATTR_NULL(vap) (*(vap) = va_null) /* initialize a vattr */
-#define HOLDRELE(vp) holdrele(vp) /* decrease buf or page ref */
-static __inline void
-holdrele(struct vnode *vp)
-{
- simple_lock(&vp->v_interlock);
- vp->v_holdcnt--;
- simple_unlock(&vp->v_interlock);
-}
-#define VHOLD(vp) vhold(vp) /* increase buf or page ref */
-static __inline void
-vhold(struct vnode *vp)
-{
- simple_lock(&vp->v_interlock);
- vp->v_holdcnt++;
- simple_unlock(&vp->v_interlock);
-}
-#define VREF(vp) vref(vp) /* increase reference */
-void vref __P((struct vnode *vp));
#endif /* DIAGNOSTIC */
#define NULLVP ((struct vnode *)NULL)
@@ -288,6 +267,15 @@
do { if(lease_updatetime) lease_updatetime(dt); } while(0)
#endif /* NFS */
+#define VSHOULDFREE(vp) \
+ (!((vp)->v_flag & (VFREE|VDOOMED)) && \
+ !(vp)->v_holdcnt && !(vp)->v_usecount)
+
+#define VSHOULDBUSY(vp) \
+ (((vp)->v_flag & VFREE) && \
+ ((vp)->v_holdcnt || (vp)->v_usecount))
+
+
#endif /* KERNEL */
@@ -482,12 +470,16 @@
void insmntque __P((struct vnode *vp, struct mount *mp));
int lease_check __P((struct vop_lease_args *ap));
void vattr_null __P((struct vattr *vap));
+void vbusy __P((struct vnode *));
int vcount __P((struct vnode *vp));
+void vdrop __P((struct vnode *));
int vfinddev __P((dev_t dev, enum vtype type, struct vnode **vpp));
+void vfree __P((struct vnode *));
void vfs_opv_init __P((struct vnodeopv_desc **them));
int vflush __P((struct mount *mp, struct vnode *skipvp, int flags));
int vget __P((struct vnode *vp, int lockflag, struct proc *p));
void vgone __P((struct vnode *vp));
+void vhold __P((struct vnode *));
int vinvalbuf __P((struct vnode *vp, int save, struct ucred *cred,
struct proc *p, int slpflag, int slptimeo));
void vprint __P((char *label, struct vnode *vp));
@@ -514,8 +506,8 @@
struct vnode *
checkalias __P((struct vnode *vp, dev_t nvp_rdev, struct mount *mp));
void vput __P((struct vnode *vp));
+void vref __P((struct vnode *vp));
void vrele __P((struct vnode *vp));
-void vtouch __P((struct vnode *vp));
#endif /* KERNEL */
#endif /* !_SYS_VNODE_H_ */
Index: ufs/lfs/lfs_segment.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/lfs/lfs_segment.c,v
retrieving revision 1.23
diff -u -r1.23 lfs_segment.c
--- lfs_segment.c 1997/08/02 14:33:20 1.23
+++ lfs_segment.c 1997/08/26 21:52:22
@@ -1222,7 +1222,7 @@
if ((vp->v_flag & VXLOCK) || /* XXX */
(vp->v_usecount == 0 &&
- vp->v_freelist.tqe_prev == (struct vnode **)0xdeadb))
+ vp->v_flag & VDOOMED))
return(1);
return (vget(vp, 0, p));
}
Index: vm/vm_swap.c
===================================================================
RCS file: /home/ncvs/src/sys/vm/vm_swap.c,v
retrieving revision 1.43
diff -u -r1.43 vm_swap.c
--- vm_swap.c 1997/03/23 03:37:54 1.43
+++ vm_swap.c 1997/08/26 22:03:54
@@ -136,7 +136,7 @@
biodone(bp);
return;
}
- VHOLD(sp->sw_vp);
+ vhold(sp->sw_vp);
if ((bp->b_flags & B_READ) == 0) {
vp = bp->b_vp;
if (vp) {
--
Poul-Henning Kamp FreeBSD coreteam member
phk@FreeBSD.ORG "Real hackers run -current on their laptop."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8608.872850044>