svn commit: r337974 - head/sys/kern

[Mark Johnston]

Author: markj
Date: Fri Aug 17 15:41:01 2018
New Revision: 337974
URL: https://svnweb.freebsd.org/changeset/base/337974

Log:
  Add INVARIANTS-only fences around lockless vnode refcount updates.
  
  Some internal KASSERTs access the v_iflag field without the vnode
  interlock held after such a refcount update.  The fences are needed for
  the assertions to be correct in the face of store reordering.
  
  Reported and tested by:	jhibbits
  Reviewed by:	kib, mjg
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D16756

Modified:
  head/sys/kern/vfs_subr.c

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Fri Aug 17 15:18:57 2018	(r337973)
+++ head/sys/kern/vfs_subr.c	Fri Aug 17 15:41:01 2018	(r337974)
@@ -118,6 +118,22 @@ static void	vnlru_return_batches(struct vfsops *mnt_op
 static void	destroy_vpollinfo(struct vpollinfo *vi);
 
 /*
+ * These fences are intended for cases where some synchronization is
+ * needed between access of v_iflags and lockless vnode refcount (v_holdcnt
+ * and v_usecount) updates.  Access to v_iflags is generally synchronized
+ * by the interlock, but we have some internal assertions that check vnode
+ * flags * without acquiring the lock.  Thus, these fences are INVARIANTS-only
+ * for now.
+ */
+#ifdef INVARIANTS
+#define	VNODE_REFCOUNT_FENCE_ACQ()	atomic_thread_fence_acq()
+#define	VNODE_REFCOUNT_FENCE_REL()	atomic_thread_fence_rel()
+#else
+#define	VNODE_REFCOUNT_FENCE_ACQ()
+#define	VNODE_REFCOUNT_FENCE_REL()
+#endif
+
+/*
  * Number of vnodes in existence.  Increased whenever getnewvnode()
  * allocates a new vnode, decreased in vdropl() for VI_DOOMED vnode.
  */
@@ -1018,6 +1034,7 @@ vnlru_free_locked(int count, struct vfsops *mnt_op)
 		 */
 		freevnodes--;
 		vp->v_iflag &= ~VI_FREE;
+		VNODE_REFCOUNT_FENCE_REL();
 		refcount_acquire(&vp->v_holdcnt);
 
 		mtx_unlock(&vnode_free_list_mtx);
@@ -2495,6 +2512,7 @@ v_incr_usecount(struct vnode *vp)
 
 	if (vp->v_type != VCHR &&
 	    refcount_acquire_if_not_zero(&vp->v_usecount)) {
+		VNODE_REFCOUNT_FENCE_ACQ();
 		VNASSERT((vp->v_iflag & VI_OWEINACT) == 0, vp,
 		    ("vnode with usecount and VI_OWEINACT set"));
 	} else {
@@ -2593,6 +2611,7 @@ vget(struct vnode *vp, int flags, struct thread *td)
 		} else {
 			oweinact = 1;
 			vp->v_iflag &= ~VI_OWEINACT;
+			VNODE_REFCOUNT_FENCE_REL();
 		}
 		refcount_acquire(&vp->v_usecount);
 		v_incr_devcount(vp);
@@ -2807,6 +2826,7 @@ _vhold(struct vnode *vp, bool locked)
 	CTR2(KTR_VFS, "%s: vp %p", __func__, vp);
 	if (!locked) {
 		if (refcount_acquire_if_not_zero(&vp->v_holdcnt)) {
+			VNODE_REFCOUNT_FENCE_ACQ();
 			VNASSERT((vp->v_iflag & VI_FREE) == 0, vp,
 			    ("_vhold: vnode with holdcnt is free"));
 			return;