From owner-freebsd-fs@FreeBSD.ORG Wed Sep 3 16:25:29 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 409921067103 for ; Wed, 3 Sep 2008 16:25:29 +0000 (UTC) (envelope-from jh@saunalahti.fi) Received: from emh06.mail.saunalahti.fi (emh06.mail.saunalahti.fi [62.142.5.116]) by mx1.freebsd.org (Postfix) with ESMTP id A80608FC24 for ; Wed, 3 Sep 2008 16:25:28 +0000 (UTC) (envelope-from jh@saunalahti.fi) Received: from saunalahti-vams (vs3-11.mail.saunalahti.fi [62.142.5.95]) by emh06-2.mail.saunalahti.fi (Postfix) with SMTP id 08926C89CB; Wed, 3 Sep 2008 19:07:40 +0300 (EEST) Received: from emh01.mail.saunalahti.fi ([62.142.5.107]) by vs3-11.mail.saunalahti.fi ([62.142.5.95]) with SMTP (gateway) id A0288404E27; Wed, 03 Sep 2008 19:07:40 +0300 Received: from a91-153-122-179.elisa-laajakaista.fi (a91-153-122-179.elisa-laajakaista.fi [91.153.122.179]) by emh01.mail.saunalahti.fi (Postfix) with SMTP id 8DF6E4BB46; Wed, 3 Sep 2008 19:07:37 +0300 (EEST) Date: Wed, 3 Sep 2008 19:07:37 +0300 From: Jaakko Heinonen To: Bruce Evans Message-ID: <20080903160736.GA12587@a91-153-122-179.elisa-laajakaista.fi> References: <200806020800.m528038T072838@freefall.freebsd.org> <20080722075718.GA1881@a91-153-120-204.elisa-laajakaista.fi> <20080722215249.K17453@delplex.bde.org> <20080723103424.GA1856@a91-153-120-204.elisa-laajakaista.fi> <20080724000618.Q16961@besplex.bde.org> <20080725072314.GA807@a91-153-120-204.elisa-laajakaista.fi> <20080725192315.D27178@delplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080725192315.D27178@delplex.bde.org> User-Agent: Mutt/1.5.18 (2008-05-17) X-Antivirus: VAMS Cc: freebsd-fs@freebsd.org 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, 03 Sep 2008 16:25:29 -0000 Hi, I further updated the patch. On 2008-07-25, Bruce Evans wrote: > On Fri, 25 Jul 2008, Jaakko Heinonen wrote: > > On 2008-07-24, Bruce Evans wrote: > >> First, the fields shouldn't be initialized using VATTR_NULL() in > >> VOP_GETATTR(). I removed VATTR_NULL() from xfs, mqueuefs, pseudofs, tmpfs, devfs fdescfs and vattr_null() from devfs and portalfs. I also removed zeroing from nfs. > > What do you think that is a proper default value for va_rdev? Some file > > systems set it to 0 and some to VNOVAL. > > Either NODEV or VNOVAL explicitly translated late to NODEV. I changed following file systems to initialize va_rdev to NODEV instead of 0 or VNOVAL (when appropriate): mqueuefs, tmpfs, portalfs, smbfs, ntfs, fdescfs and msdosfs. Also in vn_stat() va_rdev is now initialized to VNOVAL and explicitly translated to NODEV after the VOP_GETATTR() call. I have tested the patch with these file systems: UFS2, UFS1, ext2fs, ntfs, cd9660, udf, procfs, devfs, xfs, reiserfs, fdescfs, msdosfs, mqueuefs, nfs, smbfs, portalfs. %%% Index: sys/nfsclient/nfs_vnops.c =================================================================== --- sys/nfsclient/nfs_vnops.c (revision 182592) +++ sys/nfsclient/nfs_vnops.c (working copy) @@ -631,6 +631,8 @@ nfs_getattr(struct vop_getattr_args *ap) struct vnode *vp = ap->a_vp; struct nfsnode *np = VTONFS(vp); struct thread *td = curthread; + struct vattr *vap = ap->a_vap; + struct vattr vattr; caddr_t bpos, dpos; int error = 0; struct mbuf *mreq, *mrep, *md, *mb; @@ -646,12 +648,12 @@ nfs_getattr(struct vop_getattr_args *ap) /* * First look in the cache. */ - if (nfs_getattrcache(vp, ap->a_vap) == 0) + if (nfs_getattrcache(vp, &vattr) == 0) goto nfsmout; if (v3 && nfsaccess_cache_timeout > 0) { nfsstats.accesscache_misses++; nfs3_access_otw(vp, NFSV3ACCESS_ALL, td, ap->a_cred); - if (nfs_getattrcache(vp, ap->a_vap) == 0) + if (nfs_getattrcache(vp, &vattr) == 0) goto nfsmout; } nfsstats.rpccnt[NFSPROC_GETATTR]++; @@ -661,10 +663,27 @@ nfs_getattr(struct vop_getattr_args *ap) nfsm_fhtom(vp, v3); nfsm_request(vp, NFSPROC_GETATTR, td, ap->a_cred); if (!error) { - nfsm_loadattr(vp, ap->a_vap); + nfsm_loadattr(vp, &vattr); } m_freem(mrep); nfsmout: + vap->va_type = vattr.va_type; + vap->va_mode = vattr.va_mode; + vap->va_nlink = vattr.va_nlink; + vap->va_uid = vattr.va_uid; + vap->va_gid = vattr.va_gid; + vap->va_fsid = vattr.va_fsid; + vap->va_fileid = vattr.va_fileid; + vap->va_size = vattr.va_size; + vap->va_blocksize = vattr.va_blocksize; + vap->va_atime = vattr.va_atime; + vap->va_mtime = vattr.va_mtime; + vap->va_ctime = vattr.va_ctime; + vap->va_gen = vattr.va_gen; + vap->va_flags = vattr.va_flags; + vap->va_rdev = vattr.va_rdev; + vap->va_bytes = vattr.va_bytes; + return (error); } Index: sys/gnu/fs/xfs/FreeBSD/xfs_vnops.c =================================================================== --- sys/gnu/fs/xfs/FreeBSD/xfs_vnops.c (revision 182592) +++ sys/gnu/fs/xfs/FreeBSD/xfs_vnops.c (working copy) @@ -240,7 +240,6 @@ _xfs_getattr( /* extract the xfs vnode from the private data */ //xfs_vnode_t *xvp = (xfs_vnode_t *)vp->v_data; - VATTR_NULL(vap); memset(&va,0,sizeof(xfs_vattr_t)); va.va_mask = XFS_AT_STAT|XFS_AT_GENCOUNT|XFS_AT_XFLAGS; @@ -273,15 +272,9 @@ _xfs_getattr( /* * 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 return (0); } Index: sys/ufs/ufs/ufs_vnops.c =================================================================== --- sys/ufs/ufs/ufs_vnops.c (revision 182592) +++ sys/ufs/ufs/ufs_vnops.c (working copy) @@ -434,8 +434,6 @@ 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_nsec = 0; vap->va_bytes = dbtob((u_quad_t)ip->i_din1->di_blocks); } else { vap->va_rdev = ip->i_din2->di_rdev; Index: sys/kern/uipc_mqueue.c =================================================================== --- sys/kern/uipc_mqueue.c (revision 182592) +++ sys/kern/uipc_mqueue.c (working copy) @@ -1128,7 +1128,6 @@ mqfs_getattr(struct vop_getattr_args *ap struct vattr *vap = ap->a_vap; int error = 0; - VATTR_NULL(vap); vap->va_type = vp->v_type; vap->va_mode = pn->mn_mode; vap->va_nlink = 1; @@ -1145,7 +1144,7 @@ mqfs_getattr(struct vop_getattr_args *ap vap->va_birthtime = pn->mn_birth; vap->va_gen = 0; vap->va_flags = 0; - vap->va_rdev = 0; + vap->va_rdev = NODEV; vap->va_bytes = 0; vap->va_filerev = 0; vap->va_vaflags = 0; Index: sys/kern/vfs_vnops.c =================================================================== --- sys/kern/vfs_vnops.c (revision 182592) +++ sys/kern/vfs_vnops.c (working copy) @@ -703,6 +703,17 @@ vn_stat(vp, sb, active_cred, file_cred, #endif vap = &vattr; + + /* + * Initialize defaults for new and unusual fields, so that file + * systems which don't support these fields don't need to know + * about them. + */ + vap->va_birthtime.tv_sec = -1; + vap->va_birthtime.tv_nsec = 0; + vap->va_fsid = VNOVAL; + vap->va_rdev = VNOVAL; + error = VOP_GETATTR(vp, vap, active_cred); if (error) return (error); @@ -750,7 +761,10 @@ vn_stat(vp, sb, active_cred, file_cred, sb->st_nlink = vap->va_nlink; sb->st_uid = vap->va_uid; sb->st_gid = vap->va_gid; - sb->st_rdev = vap->va_rdev; + if (vap->va_rdev == VNOVAL) + sb->st_rdev = NODEV; + else + sb->st_rdev = vap->va_rdev; if (vap->va_size > OFF_MAX) return (EOVERFLOW); sb->st_size = vap->va_size; Index: sys/fs/pseudofs/pseudofs_vnops.c =================================================================== --- sys/fs/pseudofs/pseudofs_vnops.c (revision 182592) +++ sys/fs/pseudofs/pseudofs_vnops.c (working copy) @@ -191,7 +191,6 @@ pfs_getattr(struct vop_getattr_args *va) if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) PFS_RETURN (ENOENT); - VATTR_NULL(vap); vap->va_type = vn->v_type; vap->va_fileid = pn_fileno(pn, pvd->pvd_pid); vap->va_flags = 0; Index: sys/fs/tmpfs/tmpfs_vnops.c =================================================================== --- sys/fs/tmpfs/tmpfs_vnops.c (revision 182592) +++ sys/fs/tmpfs/tmpfs_vnops.c (working copy) @@ -351,8 +351,6 @@ tmpfs_getattr(struct vop_getattr_args *v node = VP_TO_TMPFS_NODE(vp); - VATTR_NULL(vap); - tmpfs_update(vp); vap->va_type = vp->v_type; @@ -371,11 +369,9 @@ tmpfs_getattr(struct vop_getattr_args *v vap->va_gen = node->tn_gen; vap->va_flags = node->tn_flags; vap->va_rdev = (vp->v_type == VBLK || vp->v_type == VCHR) ? - node->tn_rdev : VNOVAL; + node->tn_rdev : NODEV; vap->va_bytes = round_page(node->tn_size); - vap->va_filerev = VNOVAL; - vap->va_vaflags = 0; - vap->va_spare = VNOVAL; /* XXX */ + vap->va_filerev = 0; return 0; } Index: sys/fs/portalfs/portal_vnops.c =================================================================== --- sys/fs/portalfs/portal_vnops.c (revision 182592) +++ sys/fs/portalfs/portal_vnops.c (working copy) @@ -452,8 +452,6 @@ portal_getattr(ap) struct vnode *vp = ap->a_vp; struct vattr *vap = ap->a_vap; - bzero(vap, sizeof(*vap)); - vattr_null(vap); vap->va_uid = 0; vap->va_gid = 0; vap->va_size = DEV_BSIZE; @@ -463,7 +461,7 @@ portal_getattr(ap) vap->va_ctime = vap->va_mtime; vap->va_gen = 0; vap->va_flags = 0; - vap->va_rdev = 0; + vap->va_rdev = NODEV; /* vap->va_qbytes = 0; */ vap->va_bytes = 0; /* vap->va_qsize = 0; */ Index: sys/fs/devfs/devfs_vnops.c =================================================================== --- sys/fs/devfs/devfs_vnops.c (revision 182592) +++ sys/fs/devfs/devfs_vnops.c (working copy) @@ -499,8 +499,6 @@ devfs_getattr(struct vop_getattr_args *a KASSERT(de != NULL, ("Null dir dirent in devfs_getattr vp=%p", vp)); } - bzero((caddr_t) vap, sizeof(*vap)); - vattr_null(vap); vap->va_uid = de->de_uid; vap->va_gid = de->de_gid; vap->va_mode = de->de_mode; Index: sys/fs/smbfs/smbfs_node.c =================================================================== --- sys/fs/smbfs/smbfs_node.c (revision 182592) +++ sys/fs/smbfs/smbfs_node.c (working copy) @@ -438,7 +438,7 @@ smbfs_attr_cachelookup(struct vnode *vp, va->va_atime = va->va_ctime = va->va_mtime; /* time file changed */ va->va_gen = VNOVAL; /* generation number of file */ va->va_flags = 0; /* flags defined for file */ - va->va_rdev = VNOVAL; /* device the special file represents */ + va->va_rdev = NODEV; /* device the special file represents */ va->va_bytes = va->va_size; /* bytes of disk space held by file */ va->va_filerev = 0; /* file modification number */ va->va_vaflags = 0; /* operations flags */ Index: sys/fs/ntfs/ntfs_vnops.c =================================================================== --- sys/fs/ntfs/ntfs_vnops.c (revision 182592) +++ sys/fs/ntfs/ntfs_vnops.c (working copy) @@ -191,7 +191,7 @@ ntfs_getattr(ap) vap->va_nlink = (ip->i_nlink || ip->i_flag & IN_LOADED ? ip->i_nlink : 1); vap->va_uid = ip->i_mp->ntm_uid; vap->va_gid = ip->i_mp->ntm_gid; - vap->va_rdev = 0; /* XXX UNODEV ? */ + vap->va_rdev = NODEV; vap->va_size = fp->f_size; vap->va_bytes = fp->f_allocated; vap->va_atime = ntfs_nttimetounix(fp->f_times.t_access); Index: sys/fs/fdescfs/fdesc_vnops.c =================================================================== --- sys/fs/fdescfs/fdesc_vnops.c (revision 182592) +++ sys/fs/fdescfs/fdesc_vnops.c (working copy) @@ -391,8 +391,6 @@ fdesc_getattr(ap) switch (VTOFDESC(vp)->fd_type) { case Froot: - VATTR_NULL(vap); - vap->va_mode = S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH; vap->va_type = VDIR; vap->va_nlink = 2; @@ -407,7 +405,7 @@ fdesc_getattr(ap) vap->va_ctime = vap->va_mtime; vap->va_gen = 0; vap->va_flags = 0; - vap->va_rdev = 0; + vap->va_rdev = NODEV; vap->va_bytes = 0; break; @@ -421,7 +419,6 @@ fdesc_getattr(ap) error = fo_stat(fp, &stb, td->td_ucred, td); fdrop(fp, td); if (error == 0) { - VATTR_NULL(vap); vap->va_type = IFTOVT(stb.st_mode); vap->va_mode = stb.st_mode; #define FDRX (VREAD|VEXEC) Index: sys/fs/msdosfs/msdosfs_vnops.c =================================================================== --- sys/fs/msdosfs/msdosfs_vnops.c (revision 182592) +++ sys/fs/msdosfs/msdosfs_vnops.c (working copy) @@ -334,7 +334,7 @@ msdosfs_getattr(ap) vap->va_uid = pmp->pm_uid; vap->va_gid = pmp->pm_gid; vap->va_nlink = 1; - vap->va_rdev = 0; + vap->va_rdev = NODEV; vap->va_size = dep->de_FileSize; fattime2timespec(dep->de_MDate, dep->de_MTime, 0, 0, &vap->va_mtime); vap->va_ctime = vap->va_mtime; %%% -- Jaakko