Date: Wed, 23 Jul 2008 13:34:25 +0300 From: Jaakko Heinonen <jh@saunalahti.fi> To: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-fs@FreeBSD.org, ighighi@gmail.com Subject: Re: birthtime initialization Message-ID: <20080723103424.GA1856@a91-153-120-204.elisa-laajakaista.fi> In-Reply-To: <20080722215249.K17453@delplex.bde.org> References: <200806020800.m528038T072838@freefall.freebsd.org> <20080722075718.GA1881@a91-153-120-204.elisa-laajakaista.fi> <20080722215249.K17453@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2008-07-22, Bruce Evans wrote:
> > + VATTR_NULL(vap);
>
> I want to initialize va_birthtime to { -1, 0 } here only. Don't
> initialize the whole vattr here. VOP_GETTATR() is supposed to initalize
> everything, but doesn't for va_birthtime. If there any other fields
> that VOP_GETTATR() doesn't initialize, then these should be searched
> for and fixed instead of setting them to the garbage value given by
> vattr_null.
At least xfs gets it wrong for several fields.
/*
* Fields with no direct equivalent in XFS
* leave initialized by VATTR_NULL
*/
#if 0
vap->va_filerev = 0;
vap->va_birthtime = va.va_ctime;
vap->va_vaflags = 0;
vap->va_flags = 0;
vap->va_spare = 0;
#endif
> > Index: sys/ufs/ufs/ufs_vnops.c
> > ...
> > Index: sys/fs/msdosfs/msdosfs_vnops.c
> > ...
> > Index: sys/nfsclient/nfs_subs.c
>
> There are a probably more file systems that have missing or slightly
> incorrect (all zero) settings of va_birthtime.
Many file systems misses settings of va_birthtime. That's the reason why
I initialized it in vn_stat(). I have seen four types of
initializations:
1) Support and set birthtime. (UFS2, tmpfs, msdosfs (not all
variants of msdosfs support birthtime), nfs4?)
2) Set birthtime to zero. (UFS1, nfs (nfs zeroes the vattr structure))
3) Initialize vattr with VATTR_NULL() but not birthtime explicitly. Thus
tv_sec and tv_nsec are set to -1 (VNOVAL). (devfs, xfs, portalfs,
pseudofs)
4) Not initialize birthtime at all. Those would be fixed by initializing the
birthtime in vn_stat(). (cd9660, hpfs, ntfs, smbfs, udf, ext2fs,
reiserfs)
I couldn't test but I suspect that also coda belongs to this group.
So I see two ways to fix:
- initialize birthtime in vn_stat() and add/fix explicit setting for group 2
and 3 file systems or
- add explicit initialization to all file systems missing it
(groups 3 and 4) and fix group 2 to initialize birthtime to correct value
> I wouldn't like VNOVAL being replaced by VNOTIMESPECVAL, VNOUIDVAL,
> ... etc.
I agree with this.
I have updated the patch per your comments and checked more file
systems. I have verified that with this patch these file systems return
correct birthtime values (real birthtime or {-1, 0} if not supported):
UFS2, UFS1, cd9660, nfs, ext2fs, smbfs, reiserfs, xfs, ntfs, devfs,
procfs, linprocfs, tmpfs, msdosfs, portalfs, udf.
For pseudofs I set birthtime to current time.
%%%
Index: sys/kern/vfs_vnops.c
===================================================================
--- sys/kern/vfs_vnops.c (revision 180729)
+++ sys/kern/vfs_vnops.c (working copy)
@@ -703,6 +703,13 @@ vn_stat(vp, sb, active_cred, file_cred,
#endif
vap = &vattr;
+
+ /*
+ * Not all file systems initialize birthtime.
+ */
+ vap->va_birthtime.tv_sec = -1;
+ vap->va_birthtime.tv_nsec = 0;
+
error = VOP_GETATTR(vp, vap, active_cred, td);
if (error)
return (error);
Index: sys/ufs/ufs/ufs_vnops.c
===================================================================
--- sys/ufs/ufs/ufs_vnops.c (revision 180729)
+++ sys/ufs/ufs/ufs_vnops.c (working copy)
@@ -410,7 +410,7 @@ ufs_getattr(ap)
vap->va_mtime.tv_nsec = ip->i_din1->di_mtimensec;
vap->va_ctime.tv_sec = ip->i_din1->di_ctime;
vap->va_ctime.tv_nsec = ip->i_din1->di_ctimensec;
- vap->va_birthtime.tv_sec = 0;
+ vap->va_birthtime.tv_sec = -1;
vap->va_birthtime.tv_nsec = 0;
vap->va_bytes = dbtob((u_quad_t)ip->i_din1->di_blocks);
} else {
Index: sys/nfsclient/nfs_subs.c
===================================================================
--- sys/nfsclient/nfs_subs.c (revision 180729)
+++ sys/nfsclient/nfs_subs.c (working copy)
@@ -628,6 +628,8 @@ nfs_loadattrcache(struct vnode **vpp, st
vap->va_rdev = rdev;
mtime_save = vap->va_mtime;
vap->va_mtime = mtime;
+ vap->va_birthtime.tv_sec = -1;
+ vap->va_birthtime.tv_nsec = 0;
vap->va_fsid = vp->v_mount->mnt_stat.f_fsid.val[0];
if (v3) {
vap->va_nlink = fxdr_unsigned(u_short, fp->fa_nlink);
Index: sys/fs/pseudofs/pseudofs_vnops.c
===================================================================
--- sys/fs/pseudofs/pseudofs_vnops.c (revision 180729)
+++ sys/fs/pseudofs/pseudofs_vnops.c (working copy)
@@ -200,7 +200,7 @@ pfs_getattr(struct vop_getattr_args *va)
vap->va_fsid = vn->v_mount->mnt_stat.f_fsid.val[0];
vap->va_nlink = 1;
nanotime(&vap->va_ctime);
- vap->va_atime = vap->va_mtime = vap->va_ctime;
+ vap->va_atime = vap->va_mtime = vap->va_birthtime = vap->va_ctime;
switch (pn->pn_type) {
case pfstype_procdir:
Index: sys/fs/portalfs/portal_vnops.c
===================================================================
--- sys/fs/portalfs/portal_vnops.c (revision 180729)
+++ sys/fs/portalfs/portal_vnops.c (working copy)
@@ -462,6 +462,8 @@ portal_getattr(ap)
nanotime(&vap->va_atime);
vap->va_mtime = vap->va_atime;
vap->va_ctime = vap->va_mtime;
+ vap->va_birthtime.tv_sec = -1;
+ vap->va_birthtime.tv_nsec = 0;
vap->va_gen = 0;
vap->va_flags = 0;
vap->va_rdev = 0;
Index: sys/fs/devfs/devfs_vnops.c
===================================================================
--- sys/fs/devfs/devfs_vnops.c (revision 180729)
+++ sys/fs/devfs/devfs_vnops.c (working copy)
@@ -543,6 +543,8 @@ devfs_getattr(struct vop_getattr_args *a
vap->va_rdev = cdev2priv(dev)->cdp_inode;
}
+ vap->va_birthtime.tv_sec = -1;
+ vap->va_birthtime.tv_nsec = 0;
vap->va_gen = 0;
vap->va_flags = 0;
vap->va_nlink = de->de_links;
Index: sys/gnu/fs/xfs/FreeBSD/xfs_vnops.c
===================================================================
--- sys/gnu/fs/xfs/FreeBSD/xfs_vnops.c (revision 180729)
+++ sys/gnu/fs/xfs/FreeBSD/xfs_vnops.c (working copy)
@@ -263,6 +263,8 @@ _xfs_getattr(
vap->va_atime = va.va_atime;
vap->va_mtime = va.va_mtime;
vap->va_ctime = va.va_ctime;
+ vap->va_birthtime.tv_sec = -1;
+ vap->va_birthtime.tv_nsec = 0;
vap->va_gen = va.va_gen;
vap->va_rdev = va.va_rdev;
vap->va_bytes = (va.va_nblocks << BBSHIFT);
%%%
--
Jaakko
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080723103424.GA1856>