From 43211ce3dce00fd4d037457c8a71917ce611e616 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Tue, 15 Apr 2014 05:35:59 +0000 Subject: [PATCH 14/14] Kill TMPFS_RECLAIMING_BIT hack. If this really remains an issue, the problem is in the vnode life cycle, not in tmpfs, and should be fixed properly there by guaranteeing exclusive access to a vnode in VOP_RECLAIM and refusing to reanimate a vnode that the system has chosen to reclaim. --- sys/fs/tmpfs/tmpfs.h | 10 ++++------ sys/fs/tmpfs/tmpfs_subr.c | 5 ----- sys/fs/tmpfs/tmpfs_vnops.c | 23 +++++++++++++---------- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/sys/fs/tmpfs/tmpfs.h b/sys/fs/tmpfs/tmpfs.h index b94a671..a2879b8 100644 --- a/sys/fs/tmpfs/tmpfs.h +++ b/sys/fs/tmpfs/tmpfs.h @@ -186,16 +186,14 @@ CTASSERT(TMPFS_MAXNAMLEN < UINT16_MAX); #define TMPFS_UPDATE_CTIME 0x04 /* - * Bits indicating vnode reclamation and whiteout use for the directory. - * We abuse tmpfs_node_t::tn_gen for that. + * Bit indicating whiteout use for the directory. We abuse + * tmpfs_node_t::tn_gen for that. The unused high bit was formerly + * part of a locking dance for an insane vnode life cycle. */ -#define TMPFS_RECLAIMING_BIT (1U << 31) +#define TMPFS_UNUSED0_BIT (1U << 31) #define TMPFS_WHITEOUT_BIT (1U << 30) #define TMPFS_NODE_GEN_MASK (TMPFS_WHITEOUT_BIT - 1) -#define TMPFS_NODE_RECLAIMING(node) \ - (((node)->tn_gen & TMPFS_RECLAIMING_BIT) != 0) - #define TMPFS_NODE_GEN(node) \ ((node)->tn_gen & TMPFS_NODE_GEN_MASK) diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c index 792fd82..0dde1f3 100644 --- a/sys/fs/tmpfs/tmpfs_subr.c +++ b/sys/fs/tmpfs/tmpfs_subr.c @@ -278,20 +278,15 @@ again: /* If there is already a vnode, try to reclaim it. */ mutex_enter(&node->tn_vlock); if ((vp = node->tn_vnode) != NULL) { - atomic_or_32(&node->tn_gen, TMPFS_RECLAIMING_BIT); mutex_enter(vp->v_interlock); mutex_exit(&node->tn_vlock); error = vget(vp, 0); if (error == ENOENT) goto again; - atomic_and_32(&node->tn_gen, ~TMPFS_RECLAIMING_BIT); *vpp = vp; return error; } mutex_exit(&node->tn_vlock); - if (TMPFS_NODE_RECLAIMING(node)) { - atomic_and_32(&node->tn_gen, ~TMPFS_RECLAIMING_BIT); - } /* * Get a new vnode and associate it with our inode. Share the diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c index bedc0eb..20c7ce5 100644 --- a/sys/fs/tmpfs/tmpfs_vnops.c +++ b/sys/fs/tmpfs/tmpfs_vnops.c @@ -1062,22 +1062,25 @@ tmpfs_reclaim(void *v) vnode_t *vp = ap->a_vp; tmpfs_mount_t *tmp = VFS_TO_TMPFS(vp->v_mount); tmpfs_node_t *node = VP_TO_TMPFS_NODE(vp); - bool recycle; + bool recycle = (node->tn_links == 0); - mutex_enter(&node->tn_vlock); + /* + * XXX If we are going to free it, we must disassociate it from + * the fhtovp index now, before setting node->tn_vnode = NULL, + * rather than later in tmpfs_free_node, to prevent NFS from + * swooping in and trying to use it after we've decided to free + * it. + */ - /* Disassociate inode from vnode. */ + mutex_enter(&node->tn_vlock); node->tn_vnode = NULL; - vp->v_data = NULL; - - /* If inode is not referenced, i.e. no links, then destroy it. */ - recycle = node->tn_links == 0 && TMPFS_NODE_RECLAIMING(node) == 0; - mutex_exit(&node->tn_vlock); - if (recycle) { + if (recycle) tmpfs_free_node(tmp, node); - } + + vp->v_data = NULL; + return 0; } -- 1.8.3.1