[Port-solaris] Re: vn_setpath

[Frank Batschulat (private)]

On Fri, 04 Jan 2013 20:55:48 +0100, Andrew Deason <adeason@sinenomine.net> wrote:

> Solaris 11.1 appears to have gained a new vn_setpath argument. Before,
> in Solaris 11, we had:
>
> void vn_setpath(vnode_t *rootvp, struct vnode *startvp, struct vnode *vp,
>     const char *path, size_t plen);
>
> And now there is:
>
> void vn_setpath(vnode_t *rootvp, struct vnode *startvp, struct vnode *vp,
>     const char *path, size_t plen, boolean_t force);
>
> I can't find any information on what this is. Could we get a hint as to
> what this is for, or what it does, ...? I mean, I assume it somehow
> 'forces' setting the path for the vnode, but I have no idea what the
> effects are, why I would set or clear it, etc.
>
> Also, does this mean that a kernel module compiled for Solaris 11 will
> have undefined behavior wrt this on a Solaris 11.1 machine? I tried
> loading such a module, and it seems to work fine, but I don't know if
> there's a problem lurking.

Hey Andrew, you caught us by surprise here. 

We've discussed your issue, and I must admit we've not anticipated some file system
implementations making use of vn_setpath(). looking at the one occurrence of
vn_setpath() usage in OpenAFS its not much surprising to see it being placed in gafs_rename(),
the VOP_RENAME() implementation. Usually vn_setpath() is only expected to be called by
the pathname resolution and lookup code in the kernel. I suppose your code pre-dates
the introduction of vn_renamepath() which is what our file system implementations actually
use to deal with the update of the vnodes path after a successful rename.

http://git.openafs.org/?p=openafs.git;a=blob;f=src/afs/SOLARIS/osi_vnodeops.c

so in gasf_rename() we have:

1561             mutex_enter(&vp->v_lock);
1562             if (vp->v_path != NULL) {
1563                 kmem_free(vp->v_path, strlen(vp->v_path) + 1);
1564                 vp->v_path = NULL;
1565             }
1566             mutex_exit(&vp->v_lock);
1567             vn_setpath(afs_globalVp, pvp, vp, aname2, strlen(aname2));

As far as a possible impact of a AFS module compiled on Solaris 11 without the force flag
to vn_setpath() goes, it should be innocuous on Solaris 11 Update one and onwards.
It'll possibly just replace the existing path.

However, the way the above code is in gasf_rename() today, ie, freeing the existing 
vnode path upfront before calling vn_setpath(), vn_setpath() will always set v_path anyways
regardless of whether the force flag is true or false.  So no harm will happen with the current
compiled modules.

For Solaris 11 (as of build 94 and as for Solaris 10 since update 8) vn_renamepath()
is available and should be used instead, there's no reason to call vn_setpath() other then
indirectly thru vn_renamepath().

eg. a patch similar to this should be done:

--- ./SOLARIS/osi_vnodeops.c.org	Mon Dec 10 00:31:37 2012
+++ ./SOLARIS/osi_vnodeops.c	Mon Jan  7 13:49:05 2013
@@ -1558,6 +1558,9 @@
 	if (avcp) {
 	    struct vnode *vp = AFSTOV(avcp), *pvp = AFSTOV(andp);
 	    
+#if defined(AFS_SUN511_ENV)
+	    vn_renamepath(pvp, vp, aname2, strlen(aname2));
+#else
 	    mutex_enter(&vp->v_lock);
 	    if (vp->v_path != NULL) {
 		kmem_free(vp->v_path, strlen(vp->v_path) + 1);
@@ -1565,6 +1568,7 @@
 	    }
 	    mutex_exit(&vp->v_lock);
 	    vn_setpath(afs_globalVp, pvp, vp, aname2, strlen(aname2));
+#endif
 
 	    AFS_RELE(avcp);
 	}

Btw, we've noticed something else you want to fix in afs_inactive():

1674     /*
1675      * Solaris calls VOP_OPEN on exec, but doesn't call VOP_CLOSE when
1676      * the executable exits.  So we clean up the open count here.
1677      *
1678      * Only do this for mvstat 0 vnodes: when using fakestat, we can't
1679      * lose the open count for volume roots (mvstat 2), even though they
1680      * will get VOP_INACTIVE'd when released by afs_PutFakeStat().
1681      */

We have fixed the overall VOP_CLOSE() is missing in the exec path problem since Solaris 11 build 135.
This code above should be disabled in Solaris 11 and later. VOP_CLOSE() is called on exec (on the old executable),
and exit and we also call VOP_OPEN on fork(). However it is not fixed in Solaris 10.

hth, and a happy and successful year 2013 to the OpenAFS world.

cheers
frankB