On Tue, 13 Mar 2001, der Mouse wrote:

> Actually, I can't help wondering how this *ever* worked.  There was a
> time when that diskless machine worked fine with my NFS server, and I
> haven't changed anything since then that I can see affecting this.

Except you upgraded. :-)

It looks like a botch in my locking changes from when I was at NAS. Before
then, locks were in the fs-specific area of the vnode. So that when the
private data were copied from the old vnode to the new one, the lock moved
too. With the locking changes I made, the lock was kept in the vnode. So I
changed a vrele() into a vput(). I forgot that spec_vnodep_p vnodes do
fake locking (or I forgot to change it).

> > Could you try changing the genfs_no{,is,un}lock{,ed} calls into the
> > real-lock varieties and see what happens?
> 
> I didn't try that, since I don't know what else uses those routines,
> and some uses of them may depend on their semantics.  (If you really
> want me to, I can try that, but it wouldn't surprise me if it broke
> something else, something that depends on the genfs_no* behavior being
> what the name implies.)

If you can, please try it.

There are three ways that a vnode ends up with the spec_vnodep_p op
table. 1) someone does a getdevvp to get a vnode for a device. 2) vflush
gets called on a file system, and there are device nodes sitting on
it. They get vcleaned and then set to spec_vnodep_p. 3) the routine you
see (which is copied in nfs and cd9660fs and maybe a few others).

> What I did do, and it seems to have made the problem go away, is
> 
> --- /sources/latest-usr-src/sys/ufs/ufs/ufs_vnops.c	Tue Mar  7 18:19:42 2000
> +++ ufs_vnops.c	Tue Mar 13 01:00:13 2001
> @@ -1917,6 +1917,9 @@
>  			 */
>  			nvp->v_data = vp->v_data;
>  			vp->v_data = NULL;
> +			/* With v_op bashed, vput's VOP_UNLOCK is a noop.
> +			   But at this point vp is locked, so.... */
> +			VOP_UNLOCK(vp,0);
>  			vp->v_op = spec_vnodeop_p;
>  			vput(vp);
>  			vgone(vp);
> 
> I can't pretend to believe that this is the right fix.  But it made my
> symptoms go away; if it does likewise for the other person who had the
> problem, I'd be inclined to say it is the locking problem I outlined.
> What the proper fix is, that question I'll leave up to those who
> actually understand the code.

That actually is the other way to fix the problem, though with doing this
the vput really should be a vrele() (since otherwise you're implicitly
relying on specfs using the nolock routines :-)

We either: 1) make sure that both ops (the fs ops and spec_vnodep_p ops)
unlock vnodes the same way (that would mean using the real-lock routine
versions), 2) unlock the node before changing ops (what you did above), or
3) move the locking info out of struct vnode so that since spec_vnodep_p
doesn't play with locking, nothing is wrong. :-)

Take care,

Bill