svn commit: r365810 - head/sys/fs/tmpfs

Konstantin Belousov kib at FreeBSD.org
Wed Sep 16 21:28:20 UTC 2020


Author: kib
Date: Wed Sep 16 21:28:18 2020
New Revision: 365810
URL: https://svnweb.freebsd.org/changeset/base/365810

Log:
  tmpfs: restore atime updates for reads from page cache.
  
  Split TMPFS_NODE_ACCCESSED bit into dedicated byte that can be updated
  atomically without locks or (locked) atomics.
  
  tn_update_getattr() change also contains unrelated bug fix.
  
  Reported by:	lwhsu
  PR:	249362
  Reviewed by:	markj (previous version)
  Discussed with:	mjg
  Sponsored by:	The FreeBSD Foundation
  Differential revision:	https://reviews.freebsd.org/D26451

Modified:
  head/sys/fs/tmpfs/tmpfs.h
  head/sys/fs/tmpfs/tmpfs_fifoops.c
  head/sys/fs/tmpfs/tmpfs_subr.c
  head/sys/fs/tmpfs/tmpfs_vnops.c

Modified: head/sys/fs/tmpfs/tmpfs.h
==============================================================================
--- head/sys/fs/tmpfs/tmpfs.h	Wed Sep 16 21:24:34 2020	(r365809)
+++ head/sys/fs/tmpfs/tmpfs.h	Wed Sep 16 21:28:18 2020	(r365810)
@@ -173,11 +173,13 @@ struct tmpfs_node {
 	 * Node's internal status.  This is used by several file system
 	 * operations to do modifications to the node in a delayed
 	 * fashion.
+	 *
+	 * tn_accessed has a dedicated byte to allow update by store without
+	 * using atomics.  This provides a micro-optimization to e.g.
+	 * tmpfs_read_pgcache().
 	 */
-	int			tn_status;	/* (vi) */
-#define	TMPFS_NODE_ACCESSED	(1 << 1)
-#define	TMPFS_NODE_MODIFIED	(1 << 2)
-#define	TMPFS_NODE_CHANGED	(1 << 3)
+	uint8_t			tn_status;	/* (vi) */
+	uint8_t			tn_accessed;	/* unlocked */
 
 	/*
 	 * The node size.  It does not necessarily match the real amount
@@ -317,11 +319,16 @@ LIST_HEAD(tmpfs_node_list, tmpfs_node);
 #define TMPFS_ASSERT_LOCKED(node) (void)0
 #endif
 
+/* tn_vpstate */
 #define TMPFS_VNODE_ALLOCATING	1
 #define TMPFS_VNODE_WANT	2
 #define TMPFS_VNODE_DOOMED	4
 #define	TMPFS_VNODE_WRECLAIM	8
 
+/* tn_status */
+#define	TMPFS_NODE_MODIFIED	0x01
+#define	TMPFS_NODE_CHANGED	0x02
+
 /*
  * Internal representation of a tmpfs mount point.
  */
@@ -452,6 +459,7 @@ int	tmpfs_chtimes(struct vnode *, struct vattr *, stru
 void	tmpfs_itimes(struct vnode *, const struct timespec *,
 	    const struct timespec *);
 
+void	tmpfs_set_accessed(struct tmpfs_mount *tm, struct tmpfs_node *node);
 void	tmpfs_set_status(struct tmpfs_mount *tm, struct tmpfs_node *node,
 	    int status);
 int	tmpfs_truncate(struct vnode *, off_t);
@@ -551,12 +559,10 @@ static inline void
 tmpfs_update_getattr(struct vnode *vp)
 {
 	struct tmpfs_node *node;
-	int update_flags;
 
-	update_flags = TMPFS_NODE_ACCESSED | TMPFS_NODE_MODIFIED | TMPFS_NODE_CHANGED;
-
 	node = VP_TO_TMPFS_NODE(vp);
-	if (__predict_false(node->tn_status & update_flags) != 0)
+	if (__predict_false((node->tn_status & (TMPFS_NODE_MODIFIED |
+	    TMPFS_NODE_CHANGED)) != 0 || node->tn_accessed))
 		tmpfs_update(vp);
 }
 

Modified: head/sys/fs/tmpfs/tmpfs_fifoops.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_fifoops.c	Wed Sep 16 21:24:34 2020	(r365809)
+++ head/sys/fs/tmpfs/tmpfs_fifoops.c	Wed Sep 16 21:28:18 2020	(r365810)
@@ -56,8 +56,7 @@ tmpfs_fifo_close(struct vop_close_args *v)
 	struct tmpfs_node *node;
 
 	node = VP_TO_TMPFS_NODE(v->a_vp);
-	tmpfs_set_status(VFS_TO_TMPFS(v->a_vp->v_mount), node,
-	    TMPFS_NODE_ACCESSED);
+	tmpfs_set_accessed(VFS_TO_TMPFS(v->a_vp->v_mount), node);
 	tmpfs_update(v->a_vp);
 	return (fifo_specops.vop_close(v));
 }

Modified: head/sys/fs/tmpfs/tmpfs_subr.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_subr.c	Wed Sep 16 21:24:34 2020	(r365809)
+++ head/sys/fs/tmpfs/tmpfs_subr.c	Wed Sep 16 21:28:18 2020	(r365810)
@@ -88,6 +88,7 @@ tmpfs_node_ctor(void *mem, int size, void *arg, int fl
 	node->tn_gen++;
 	node->tn_size = 0;
 	node->tn_status = 0;
+	node->tn_accessed = false;
 	node->tn_flags = 0;
 	node->tn_links = 0;
 	node->tn_vnode = NULL;
@@ -1098,8 +1099,8 @@ tmpfs_dir_attach(struct vnode *vp, struct tmpfs_dirent
 		tmpfs_dir_attach_dup(dnode, &xde->ud.td_duphead, de);
 	}
 	dnode->tn_size += sizeof(struct tmpfs_dirent);
-	dnode->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_CHANGED | \
-	    TMPFS_NODE_MODIFIED;
+	dnode->tn_status |= TMPFS_NODE_CHANGED | TMPFS_NODE_MODIFIED;
+	dnode->tn_accessed = true;
 	tmpfs_update(vp);
 }
 
@@ -1145,8 +1146,8 @@ tmpfs_dir_detach(struct vnode *vp, struct tmpfs_dirent
 		RB_REMOVE(tmpfs_dir, head, de);
 
 	dnode->tn_size -= sizeof(struct tmpfs_dirent);
-	dnode->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_CHANGED | \
-	    TMPFS_NODE_MODIFIED;
+	dnode->tn_status |= TMPFS_NODE_CHANGED | TMPFS_NODE_MODIFIED;
+	dnode->tn_accessed = true;
 	tmpfs_update(vp);
 }
 
@@ -1199,7 +1200,7 @@ tmpfs_dir_getdotdent(struct tmpfs_mount *tm, struct tm
 	else
 		error = uiomove(&dent, dent.d_reclen, uio);
 
-	tmpfs_set_status(tm, node, TMPFS_NODE_ACCESSED);
+	tmpfs_set_accessed(tm, node);
 
 	return (error);
 }
@@ -1246,7 +1247,7 @@ tmpfs_dir_getdotdotdent(struct tmpfs_mount *tm, struct
 	else
 		error = uiomove(&dent, dent.d_reclen, uio);
 
-	tmpfs_set_status(tm, node, TMPFS_NODE_ACCESSED);
+	tmpfs_set_accessed(tm, node);
 
 	return (error);
 }
@@ -1391,8 +1392,8 @@ tmpfs_dir_getdents(struct tmpfs_mount *tm, struct tmpf
 	node->tn_dir.tn_readdir_lastn = off;
 	node->tn_dir.tn_readdir_lastp = de;
 
-	tmpfs_set_status(tm, node, TMPFS_NODE_ACCESSED);
-	return error;
+	tmpfs_set_accessed(tm, node);
+	return (error);
 }
 
 int
@@ -1833,7 +1834,7 @@ tmpfs_chtimes(struct vnode *vp, struct vattr *vap,
 		return (error);
 
 	if (vap->va_atime.tv_sec != VNOVAL)
-		node->tn_status |= TMPFS_NODE_ACCESSED;
+		node->tn_accessed = true;
 
 	if (vap->va_mtime.tv_sec != VNOVAL)
 		node->tn_status |= TMPFS_NODE_MODIFIED;
@@ -1861,6 +1862,14 @@ tmpfs_set_status(struct tmpfs_mount *tm, struct tmpfs_
 	TMPFS_NODE_UNLOCK(node);
 }
 
+void
+tmpfs_set_accessed(struct tmpfs_mount *tm, struct tmpfs_node *node)
+{
+	if (node->tn_accessed || tm->tm_ronly)
+		return;
+	atomic_store_8(&node->tn_accessed, true);
+}
+
 /* Sync timestamps */
 void
 tmpfs_itimes(struct vnode *vp, const struct timespec *acc,
@@ -1872,13 +1881,13 @@ tmpfs_itimes(struct vnode *vp, const struct timespec *
 	ASSERT_VOP_LOCKED(vp, "tmpfs_itimes");
 	node = VP_TO_TMPFS_NODE(vp);
 
-	if ((node->tn_status & (TMPFS_NODE_ACCESSED | TMPFS_NODE_MODIFIED |
-	    TMPFS_NODE_CHANGED)) == 0)
+	if (!node->tn_accessed &&
+	    (node->tn_status & (TMPFS_NODE_MODIFIED | TMPFS_NODE_CHANGED)) == 0)
 		return;
 
 	vfs_timestamp(&now);
 	TMPFS_NODE_LOCK(node);
-	if (node->tn_status & TMPFS_NODE_ACCESSED) {
+	if (node->tn_accessed) {
 		if (acc == NULL)
 			 acc = &now;
 		node->tn_atime = *acc;
@@ -1890,8 +1899,8 @@ tmpfs_itimes(struct vnode *vp, const struct timespec *
 	}
 	if (node->tn_status & TMPFS_NODE_CHANGED)
 		node->tn_ctime = now;
-	node->tn_status &= ~(TMPFS_NODE_ACCESSED | TMPFS_NODE_MODIFIED |
-	    TMPFS_NODE_CHANGED);
+	node->tn_status &= ~(TMPFS_NODE_MODIFIED | TMPFS_NODE_CHANGED);
+	node->tn_accessed = false;
 	TMPFS_NODE_UNLOCK(node);
 
 	/* XXX: FIX? The entropy here is desirable, but the harvesting may be expensive */

Modified: head/sys/fs/tmpfs/tmpfs_vnops.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_vnops.c	Wed Sep 16 21:24:34 2020	(r365809)
+++ head/sys/fs/tmpfs/tmpfs_vnops.c	Wed Sep 16 21:28:18 2020	(r365810)
@@ -586,7 +586,7 @@ tmpfs_read(struct vop_read_args *v)
 	if (uio->uio_offset < 0)
 		return (EINVAL);
 	node = VP_TO_TMPFS_NODE(vp);
-	tmpfs_set_status(VFS_TO_TMPFS(vp->v_mount), node, TMPFS_NODE_ACCESSED);
+	tmpfs_set_accessed(VFS_TO_TMPFS(vp->v_mount), node);
 	return (uiomove_object(node->tn_reg.tn_aobj, node->tn_size, uio));
 }
 
@@ -622,6 +622,7 @@ tmpfs_read_pgcache(struct vop_read_pgcache_args *v)
 	if (!VN_IS_DOOMED(vp)) {
 		/* size cannot become shorter due to rangelock. */
 		size = node->tn_size;
+		tmpfs_set_accessed(node->tn_reg.tn_tmp, node);
 		vfs_smr_exit();
 		error = uiomove_object(object, size, v->a_uio);
 		return (error);
@@ -667,8 +668,8 @@ tmpfs_write(struct vop_write_args *v)
 	}
 
 	error = uiomove_object(node->tn_reg.tn_aobj, node->tn_size, uio);
-	node->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_MODIFIED |
-	    TMPFS_NODE_CHANGED;
+	node->tn_status |= TMPFS_NODE_MODIFIED | TMPFS_NODE_CHANGED;
+	node->tn_accessed = true;
 	if (node->tn_mode & (S_ISUID | S_ISGID)) {
 		if (priv_check_cred(v->a_cred, PRIV_VFS_RETAINSUGID)) {
 			newmode = node->tn_mode & ~(S_ISUID | S_ISGID);
@@ -744,7 +745,8 @@ tmpfs_remove(struct vop_remove_args *v)
 	 * reclaimed. */
 	tmpfs_free_dirent(tmp, de);
 
-	node->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_CHANGED;
+	node->tn_status |= TMPFS_NODE_CHANGED;
+	node->tn_accessed = true;
 	error = 0;
 
 out:
@@ -1317,15 +1319,15 @@ tmpfs_rmdir(struct vop_rmdir_args *v)
 	TMPFS_NODE_LOCK(node);
 	node->tn_links--;
 	node->tn_dir.tn_parent = NULL;
-	node->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_CHANGED |
-	    TMPFS_NODE_MODIFIED;
+	node->tn_status |= TMPFS_NODE_CHANGED | TMPFS_NODE_MODIFIED;
+	node->tn_accessed = true;
 
 	TMPFS_NODE_UNLOCK(node);
 
 	TMPFS_NODE_LOCK(dnode);
 	dnode->tn_links--;
-	dnode->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_CHANGED |
-	    TMPFS_NODE_MODIFIED;
+	dnode->tn_status |= TMPFS_NODE_CHANGED | TMPFS_NODE_MODIFIED;
+	dnode->tn_accessed = true;
 	TMPFS_NODE_UNLOCK(dnode);
 
 	if (tmpfs_use_nc(dvp)) {
@@ -1444,7 +1446,7 @@ tmpfs_readlink(struct vop_readlink_args *v)
 
 	error = uiomove(node->tn_link, MIN(node->tn_size, uio->uio_resid),
 	    uio);
-	tmpfs_set_status(VFS_TO_TMPFS(vp->v_mount), node, TMPFS_NODE_ACCESSED);
+	tmpfs_set_accessed(VFS_TO_TMPFS(vp->v_mount), node);
 
 	return (error);
 }


More information about the svn-src-all mailing list