Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Nov 1995 09:05:04 -0700 (MST)
From:      Terry Lambert <terry@lambert.org>
To:        rashid@rk.ios.com (Rashid Karimov)
Cc:        terry@lambert.org, jgreco@brasil.moneng.mei.com, jkh@time.cdrom.com, hackers@freebsd.org
Subject:   Re: Odd crash after inode depletion..
Message-ID:  <199511141605.JAA19934@phaeton.artisoft.com>
In-Reply-To: <199511141522.KAA02266@rk.ios.com> from "Rashid Karimov" at Nov 14, 95 10:22:23 am

next in thread | previous in thread | raw e-mail | index | archive | help
> > > I've noticed an infrequent but consistent tendency for the system to panic
> > > with "free vnode isn't" some time after hitting inode exhaustion.  With a
> > > sample size of only a few instances, all I can establish is that I have NOT
> > > seen a "free vnode isn't" panic without first having run out of inodes.
> > > 
> > > (system is running 1026-SNAP, by the way, which may be significant - I never
> > > saw this panic under 2.0.5R, but then again, I can't recall ever having run
> > > out of inodes under 2.0.5R)
> > 
> > This is endemic to the move from regualr to circular queues.  I don't know
> > why, but going back to the old queues fixes it.
> 
> 	Terry , could you pls post here the patch which fixes the
> 	thing ? The problem doesn't bug us for some time - but if
> 	it will return - I'll need the fix

Going back isn't the correct fix.

The correct fix is to find where the tail wraps and fix it.

It would be difficult for me to find this; I've changed large portions
of this code locally... I don't even allocate nodes the same way any
more because I want to vary cache discard rates by file system type
(so I have a watermarked pool on a per FS basis for node allocation).

My guess from the -curent code is:

/sys/kern/vfs_subr.c:
===========================================================================
/*
 * Return the next vnode from the free list.
 */
int
getnewvnode(tag, mp, vops, vpp)
	enum vtagtype tag;
	struct mount *mp;
	vop_t **vops;
	struct vnode **vpp;
{
	register struct vnode *vp;

	vp = vnode_free_list.tqh_first; <<<<<<<-------------******************
	/*
	 * we allocate a new vnode if
	 * 	1. we don't have any free
	 *		Pretty obvious, we actually used to panic, but that
	 *		is a silly thing to do.
	 *	2. we havn't filled our pool yet
	 *		We don't want to trash the incore (VM-)vnodecache.
	 *	3. if less that 1/4th of our vnodes are free.
	 *		We don't want to trash the namei cache either.
	 */
	if (freevnodes < (numvnodes >> 2) ||
	    numvnodes < desiredvnodes ||
	    vp == NULL) { <<<<<<<-----------------****************************
		vp = (struct vnode *) malloc((u_long) sizeof *vp,
		    M_VNODE, M_WAITOK);
		bzero((char *) vp, sizeof *vp);
		numvnodes++;
	} else {
		TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
		freevnodes--;

		if (vp->v_usecount)
			panic("free vnode isn't");
===========================================================================

This is a pretty obvious bug.  When the circular queue wraps, vp will not
be NULL.

This is probably most easily fixed if you change:

	    numvnodes < desiredvnodes ||
To:
	    numvnodes < desiredvnodes + 1 ||

It's probably most correctly fixed by changing:

	    vp == NULL)
To:
	    vp == NULL ||		/* list empty*/
	    vp->v_usecount)		/* queue wrapped*/

Or something similar using one of the circular queue macros.  Then
remove the stupid:

		if (vp->v_usecount)
			panic("free vnode isn't");


The move to circular queues is one of those half-cocked changes that
you see made for purely aesthetic reasons instead of technical ones.
The move to circular queues for the mountlists is one of the things
that made my FS layering patches not apply cleanly.  Mountlists are
effectively initialization code, and one should not optimize such
code unduly: its execution time is irrelevant.  Vnode allocation is
more time critical, but a circular queue is a bad deal all around
in terms of doubling the object protection hits and cache coherency
hassles in an MP/MT environment.  It is a pessimization in that
situation.  8-(.


					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199511141605.JAA19934>