Re: Weird case in vfs_bio.c bwrite

[David Holland <dholland-tech%netbsd.org@localhost>]

On Thu, Jul 21, 2016 at 01:21:57PM +0000, coypu%SDF.ORG@localhost wrote:
 > I've been reading the vfs code for no reason.
 > 
 > in vfs_bio.c:802 we have:
 >   vp = bp->b_vp;
 > 
 > then we have a test if it's NULL, but strangely, we do not leave the
 > function, we continue with it.
 > 
 > there is even a call in vfs_bio.c:873
 >   VOP_STRATEGY(vp, bp);
 > which will still happen for the vp=NULL case, if it is possible.
 > 
 > what is this case?

I think superstition/paranoia.

That VOP_STRATEGY call will crash if the vnode is null. In fact, this
appears to be the case going all the way back to 4.4lite2; while
VOP_STRATEGY used to be special-cased, until hannken@ cleaned it out
in 2004, as far back as I can readily check in CVS the special-case
case code would still crash if the vnode pointer coming from the
buffer was null.

In the 4.4lite2 version, or as close as the cvs history has (it
appears that someone zapped one or two versions too many when clearing
out the lawsuit-tainted stuff) there is no null check on vp; it was
added in -r1.37/1.38 by cgd, probably out of caution, and then
reinforced later by additional similar logic added when the
softupdates code was first imported. After that I think everyone
touching the function just assumed it was a possible case and
proceeded accordingly.

I think the logic can go away. I will add an assertion and do an anita
run, and if that passes I'll commit the assertion, and then if nobody
sees it going off for a while someone can prune the logic.

(Note that while the vnode should not be null, the resulting mp can
reasonably be null, e.g. if operating directly on a device with no fs
mounted.)

-- 
David A. Holland
dholland%netbsd.org@localhost