Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Sep 2008 19:07:37 +0300
From:      Jaakko Heinonen <jh@saunalahti.fi>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        freebsd-fs@freebsd.org
Subject:   Re: birthtime initialization
Message-ID:  <20080903160736.GA12587@a91-153-122-179.elisa-laajakaista.fi>
In-Reply-To: <20080725192315.D27178@delplex.bde.org>
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>

next in thread | previous in thread | raw e-mail | index | archive | help

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080903160736.GA12587>