Date: Fri, 8 Nov 1996 11:46:13 +0900 (JST) From: Michael Hancock <michaelh@cet.co.jp> To: Terry Lambert <terry@lambert.org> Cc: dg@root.com, ponds!rivers@dg-rtp.dg.com, dyson@freefall.freebsd.org, freebsd-hackers@freefall.freebsd.org Subject: Re: More info on the daily panics... Message-ID: <Pine.SV4.3.95.961108111353.760C-100000@parkplace.cet.co.jp> In-Reply-To: <199611071813.LAA10315@phaeton.artisoft.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 7 Nov 1996, Terry Lambert wrote:
> If you are really concerned that this will mask a future multiple vrele()
> problem, I suggest you put the assert in vrele() and prevent the queue
> from ever getting corrupted that way in the first place.
We really need to engineer asserts into the kernel. They're kind of there
with the #ifdef DIAGNOSTICS stuff, but this is ugly I hate looking at
#ifdef's. I'm happy that we *have* a model.
In our code they are often in the wrong place. In the case of vrele()
they are too far into the function body, they should be the first things
checked for, near the parameter list.
The current model for parameter checking is this:
1) #ifdef DIAGNOSTICS ... #endif for parameter checks we assume the caller
does correctly, but want to check sometime.
2) if ( expr ) { ... } for checks we think are important enough to require
in it runtime code.
We also have stuff embedded, but I won't talk about that.
I like to improve upon the model like this:
Improved use of assertions
====================================================================
void
vrele(vp)
register struct vnode *vp;
{
ASSUME(vp != NULL, "vrele: vp shouldn't be null");
REQUIRE(vp->v_usecount > 0, "vrele: ref count shouldn't already be
zero");
vp->v_usecount--;
if ((vp->v_usecount == 1) &&
vp->v_object &&
(vp->v_object->flags & OBJ_VFS_REF)) {
vp->v_object->flags &= ~OBJ_VFS_REF;
vm_object_deallocate(vp->v_object);
return;
}
if (vp->v_usecount > 0)
return;
[ .. ]
Current
====================================================================
void
vrele(vp)
register struct vnode *vp;
{
#ifdef DIAGNOSTIC
if (vp == NULL)
panic("vrele: null vp");
#endif
vp->v_usecount--;
if ((vp->v_usecount == 1) &&
vp->v_object &&
(vp->v_object->flags & OBJ_VFS_REF)) {
vp->v_object->flags &= ~OBJ_VFS_REF;
vm_object_deallocate(vp->v_object);
return;
}
if (vp->v_usecount > 0)
return;
if (vp->v_usecount < 0) {
#ifdef DIAGNOSTIC
vprint("vrele: negative ref count", vp);
#endif
panic("vrele: negative reference cnt");
}
[ ... ]
Regards,
Mike Hancock
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.SV4.3.95.961108111353.760C-100000>