From 016b7c7e39a60c7d20399f111ba12c9bfb1c4bd7 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Wed, 16 Sep 2020 21:28:18 +0000 Subject: [PATCH] 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 --- sys/fs/tmpfs/tmpfs.h | 22 +++++++++++++-------- sys/fs/tmpfs/tmpfs_fifoops.c | 3 +-- sys/fs/tmpfs/tmpfs_subr.c | 37 ++++++++++++++++++++++-------------- sys/fs/tmpfs/tmpfs_vnops.c | 20 ++++++++++--------- 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/sys/fs/tmpfs/tmpfs.h b/sys/fs/tmpfs/tmpfs.h index 5b966635c486..8c6e51e62fe5 100644 --- a/sys/fs/tmpfs/tmpfs.h +++ b/sys/fs/tmpfs/tmpfs.h @@ -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 *, struct ucred *cred, 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); } diff --git a/sys/fs/tmpfs/tmpfs_fifoops.c b/sys/fs/tmpfs/tmpfs_fifoops.c index de5c46224314..9b66f26fa14a 100644 --- a/sys/fs/tmpfs/tmpfs_fifoops.c +++ b/sys/fs/tmpfs/tmpfs_fifoops.c @@ -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)); } diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c index b0e3c2506709..0068656968b5 100644 --- a/sys/fs/tmpfs/tmpfs_subr.c +++ b/sys/fs/tmpfs/tmpfs_subr.c @@ -88,6 +88,7 @@ tmpfs_node_ctor(void *mem, int size, void *arg, int flags) 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 *de) 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 *de) 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 tmpfs_node *node, 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 tmpfs_node *node, 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 tmpfs_node *node, 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_node *node, int status) 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 *acc, 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 *acc, } 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 */ diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c index 6a836d2b5fc9..3ca98bbb21a4 100644 --- a/sys/fs/tmpfs/tmpfs_vnops.c +++ b/sys/fs/tmpfs/tmpfs_vnops.c @@ -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); }