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