Subject: Re: VNODEOPs & SAVESTART in cn_flags
To: Chris Jepeway <jepeway@blasted-heath.com>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 01/29/2002 10:03:07
On Tue, 29 Jan 2002, Chris Jepeway wrote:

> Some vnode operations in some file systems
> will pay attention to whether the SAVESTART
> bit is set in the cn_flags member of their
> componentname structure.  Others ignore it
> and always call PNBUF_PUT().  A table of a few:
>
>     Honors SAVESTART        Always PNBUF_PUT()s
>     ----------------        -------------------
>         coda_link                ufs_link
>       msdosfs_mkdir            ext2fs_mkdir
>         ufs_mknod               nfs_mknod
>
> The following patch to v 1.82 of ufs/ufs/ufs_vnops.c
> illustrates the issue for ufs_link() (the patch is built
> from a private repository, so ignore its version numbers):
>
> Index: ufs/ufs/ufs_vnops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
> retrieving revision 1.1
> retrieving revision 1.2
> diff -u -r1.1 -r1.2
> --- ufs/ufs/ufs_vnops.c	2001/10/25 21:49:09	1.1
> +++ ufs/ufs/ufs_vnops.c	2002/01/27 18:49:29	1.2
> @@ -679,7 +679,8 @@
>  		if (DOINGSOFTDEP(vp))
>  			softdep_change_linkcnt(ip);
>  	}
> -	PNBUF_PUT(cnp->cn_pnbuf);
> +	if ((cnp->cn_flags & SAVESTART) == 0)
> +		PNBUF_PUT(cnp->cn_pnbuf);
>   out1:
>  	if (dvp != vp)
>  		VOP_UNLOCK(vp, 0);
>
> One might argue this sorta thing doesn't matter much:
> after all, VOP_LINK() is only called in places that don't
> set SAVESTART.  However, differences like these across

Hmmm.. That means that fixing the problems probably won't break things.
:-)

> the various file systems make introducing new file systems
> a bit harder.  Two examples:
>
>     o  a stacked f/s might want to use the pathname
>        after a direct VOP_() call to its lower f/s
>        succeeds
>
>     o  expanding the VOP_() interface to support a f/s
>        that detects soft failures withe a return code
>        that means "please retry the VOP_(), it might
>        succeed now" isn't as fun if you can't use the
>        FFS as a baseline
>
> Regardless, it seems like every f/s should handle a
> given VOP_() the same way.  Question is, well, what's
> that same way?  I'd argue that at least all VOP_LINK()s
> should honor SAVESTART, since most of the VOP_CREATE()s
> seem to.

I agree that all f/s should behave the same. The alternative to having all
VOPs honor SAVESTART would be to document which do and don't.

Just honoring it across the board would probaby be easiest.

> Thoughts?  Would a PR about inconsistencies wrt to
> SAVESTART be in order?

PR sounds good.

Take care,

Bill