From owner-freebsd-fs@FreeBSD.ORG Wed Jul 23 10:34:31 2008 Return-Path: Delivered-To: freebsd-fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 55EE4106567B for ; Wed, 23 Jul 2008 10:34:31 +0000 (UTC) (envelope-from jh@saunalahti.fi) Received: from emh07.mail.saunalahti.fi (emh07.mail.saunalahti.fi [62.142.5.117]) by mx1.freebsd.org (Postfix) with ESMTP id D2C058FC0A for ; Wed, 23 Jul 2008 10:34:30 +0000 (UTC) (envelope-from jh@saunalahti.fi) Received: from saunalahti-vams (vs3-12.mail.saunalahti.fi [62.142.5.96]) by emh07-2.mail.saunalahti.fi (Postfix) with SMTP id 9D44961A49; Wed, 23 Jul 2008 13:34:29 +0300 (EEST) Received: from emh04.mail.saunalahti.fi ([62.142.5.110]) by vs3-12.mail.saunalahti.fi ([62.142.5.96]) with SMTP (gateway) id A06BFC935DF; Wed, 23 Jul 2008 13:34:29 +0300 Received: from a91-153-120-204.elisa-laajakaista.fi (a91-153-120-204.elisa-laajakaista.fi [91.153.120.204]) by emh04.mail.saunalahti.fi (Postfix) with SMTP id 3827C41C9B; Wed, 23 Jul 2008 13:34:25 +0300 (EEST) Date: Wed, 23 Jul 2008 13:34:25 +0300 From: Jaakko Heinonen To: Bruce Evans Message-ID: <20080723103424.GA1856@a91-153-120-204.elisa-laajakaista.fi> References: <200806020800.m528038T072838@freefall.freebsd.org> <20080722075718.GA1881@a91-153-120-204.elisa-laajakaista.fi> <20080722215249.K17453@delplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080722215249.K17453@delplex.bde.org> User-Agent: Mutt/1.5.18 (2008-05-17) X-Antivirus: VAMS Cc: freebsd-fs@FreeBSD.org, ighighi@gmail.com Subject: Re: birthtime initialization X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jul 2008 10:34:31 -0000 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