Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Nov 2019 12:05:59 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r354890 - in head/sys: fs/devfs kern sys
Message-ID:  <201911201205.xAKC5xfj021902@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mjg
Date: Wed Nov 20 12:05:59 2019
New Revision: 354890
URL: https://svnweb.freebsd.org/changeset/base/354890

Log:
  vfs: change si_usecount management to count used vnodes
  
  Currently si_usecount is effectively a sum of usecounts from all associated
  vnodes. This is maintained by special-casing for VCHR every time usecount is
  modified. Apart from complicating the code a little bit, it has a scalability
  impact since it forces a read from a cacheline shared with said count.
  
  There are no consumers of the feature in the ports tree. In head there are only
  2: revoke and devfs_close. Both can get away with a weaker requirement than the
  exact usecount, namely just the count of active vnodes. Changing the meaning to
  the latter means we only need to modify it on 0<->1 transitions, avoiding the
  check plenty of times (and entirely in something like vrefact).
  
  Reviewed by:	kib, jeff
  Tested by:	pho
  Differential Revision:	https://reviews.freebsd.org/D22202

Modified:
  head/sys/fs/devfs/devfs_vnops.c
  head/sys/kern/vfs_subr.c
  head/sys/kern/vfs_syscalls.c
  head/sys/sys/conf.h

Modified: head/sys/fs/devfs/devfs_vnops.c
==============================================================================
--- head/sys/fs/devfs/devfs_vnops.c	Wed Nov 20 11:12:19 2019	(r354889)
+++ head/sys/fs/devfs/devfs_vnops.c	Wed Nov 20 12:05:59 2019	(r354890)
@@ -481,7 +481,7 @@ loop:
 		vp->v_rdev = dev;
 		KASSERT(vp->v_usecount == 1,
 		    ("%s %d (%d)\n", __func__, __LINE__, vp->v_usecount));
-		dev->si_usecount += vp->v_usecount;
+		dev->si_usecount++;
 		/* Special casing of ttys for deadfs.  Probably redundant. */
 		dsw = dev->si_devsw;
 		if (dsw != NULL && (dsw->d_flags & D_TTY) != 0)
@@ -581,7 +581,7 @@ devfs_close(struct vop_close_args *ap)
 	 * if the reference count is 2 (this last descriptor
 	 * plus the session), release the reference from the session.
 	 */
-	if (td != NULL) {
+	if (vp->v_usecount == 2 && td != NULL) {
 		p = td->td_proc;
 		PROC_LOCK(p);
 		if (vp == p->p_session->s_ttyvp) {
@@ -591,7 +591,7 @@ devfs_close(struct vop_close_args *ap)
 			if (vp == p->p_session->s_ttyvp) {
 				SESS_LOCK(p->p_session);
 				VI_LOCK(vp);
-				if (count_dev(dev) == 2 &&
+				if (vp->v_usecount == 2 && vcount(vp) == 1 &&
 				    (vp->v_iflag & VI_DOOMED) == 0) {
 					p->p_session->s_ttyvp = NULL;
 					p->p_session->s_ttydp = NULL;
@@ -620,19 +620,19 @@ devfs_close(struct vop_close_args *ap)
 		return (ENXIO);
 	dflags = 0;
 	VI_LOCK(vp);
+	if (vp->v_usecount == 1 && vcount(vp) == 1)
+		dflags |= FLASTCLOSE;
 	if (vp->v_iflag & VI_DOOMED) {
 		/* Forced close. */
 		dflags |= FREVOKE | FNONBLOCK;
 	} else if (dsw->d_flags & D_TRACKCLOSE) {
 		/* Keep device updated on status. */
-	} else if (count_dev(dev) > 1) {
+	} else if ((dflags & FLASTCLOSE) == 0) {
 		VI_UNLOCK(vp);
 		dev_relthread(dev, ref);
 		return (0);
 	}
-	if (count_dev(dev) == 1)
-		dflags |= FLASTCLOSE;
-	vholdl(vp);
+	vholdnz(vp);
 	VI_UNLOCK(vp);
 	vp_locked = VOP_ISLOCKED(vp);
 	VOP_UNLOCK(vp, 0);
@@ -1425,7 +1425,7 @@ devfs_reclaim_vchr(struct vop_reclaim_args *ap)
 	dev = vp->v_rdev;
 	vp->v_rdev = NULL;
 	if (dev != NULL)
-		dev->si_usecount -= vp->v_usecount;
+		dev->si_usecount -= (vp->v_usecount > 0);
 	dev_unlock();
 	VI_UNLOCK(vp);
 	if (dev != NULL)

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Wed Nov 20 11:12:19 2019	(r354889)
+++ head/sys/kern/vfs_subr.c	Wed Nov 20 12:05:59 2019	(r354890)
@@ -2714,26 +2714,11 @@ _vget_prep(struct vnode *vp, bool interlock)
 {
 	enum vgetstate vs;
 
-	if (__predict_true(vp->v_type != VCHR)) {
-		if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
-			vs = VGET_USECOUNT;
-		} else {
-			_vhold(vp, interlock);
-			vs = VGET_HOLDCNT;
-		}
+	if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
+		vs = VGET_USECOUNT;
 	} else {
-		if (!interlock)
-			VI_LOCK(vp);
-		if (vp->v_usecount == 0) {
-			vholdl(vp);
-			vs = VGET_HOLDCNT;
-		} else {
-			v_incr_devcount(vp);
-			refcount_acquire(&vp->v_usecount);
-			vs = VGET_USECOUNT;
-		}
-		if (!interlock)
-			VI_UNLOCK(vp);
+		_vhold(vp, interlock);
+		vs = VGET_HOLDCNT;
 	}
 	return (vs);
 }
@@ -2796,8 +2781,7 @@ vget_finish(struct vnode *vp, int flags, enum vgetstat
 	 * the vnode around. Otherwise someone else lended their hold count and
 	 * we have to drop ours.
 	 */
-	if (vp->v_type != VCHR &&
-	    refcount_acquire_if_not_zero(&vp->v_usecount)) {
+	if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
 #ifdef INVARIANTS
 		int old = atomic_fetchadd_int(&vp->v_holdcnt, -1) - 1;
 		VNASSERT(old > 0, vp, ("%s: wrong hold count", __func__));
@@ -2823,24 +2807,19 @@ vget_finish(struct vnode *vp, int flags, enum vgetstat
 	 * See the previous section. By the time we get here we may find
 	 * ourselves in the same spot.
 	 */
-	if (vp->v_type != VCHR) {
-		if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
+	if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
 #ifdef INVARIANTS
-			int old = atomic_fetchadd_int(&vp->v_holdcnt, -1) - 1;
-			VNASSERT(old > 0, vp, ("%s: wrong hold count", __func__));
+		int old = atomic_fetchadd_int(&vp->v_holdcnt, -1) - 1;
+		VNASSERT(old > 0, vp, ("%s: wrong hold count", __func__));
 #else
-			refcount_release(&vp->v_holdcnt);
+		refcount_release(&vp->v_holdcnt);
 #endif
-			VNODE_REFCOUNT_FENCE_ACQ();
-			VNASSERT((vp->v_iflag & VI_OWEINACT) == 0, vp,
-			    ("%s: vnode with usecount and VI_OWEINACT set",
-			    __func__));
-			VI_UNLOCK(vp);
-			return (0);
-		}
-	} else {
-		if (vp->v_usecount > 0)
-			refcount_release(&vp->v_holdcnt);
+		VNODE_REFCOUNT_FENCE_ACQ();
+		VNASSERT((vp->v_iflag & VI_OWEINACT) == 0, vp,
+		    ("%s: vnode with usecount and VI_OWEINACT set",
+		    __func__));
+		VI_UNLOCK(vp);
+		return (0);
 	}
 	if ((vp->v_iflag & VI_OWEINACT) == 0) {
 		oweinact = 0;
@@ -2868,8 +2847,7 @@ vref(struct vnode *vp)
 
 	ASSERT_VI_UNLOCKED(vp, __func__);
 	CTR2(KTR_VFS, "%s: vp %p", __func__, vp);
-	if (vp->v_type != VCHR &&
-	    refcount_acquire_if_not_zero(&vp->v_usecount)) {
+	if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
 		VNODE_REFCOUNT_FENCE_ACQ();
 		VNASSERT(vp->v_holdcnt > 0, vp,
 		    ("%s: active vnode not held", __func__));
@@ -2888,8 +2866,7 @@ vrefl(struct vnode *vp)
 
 	ASSERT_VI_LOCKED(vp, __func__);
 	CTR2(KTR_VFS, "%s: vp %p", __func__, vp);
-	if (vp->v_type != VCHR &&
-	    refcount_acquire_if_not_zero(&vp->v_usecount)) {
+	if (refcount_acquire_if_not_zero(&vp->v_usecount)) {
 		VNODE_REFCOUNT_FENCE_ACQ();
 		VNASSERT(vp->v_holdcnt > 0, vp,
 		    ("%s: active vnode not held", __func__));
@@ -2897,8 +2874,7 @@ vrefl(struct vnode *vp)
 		    ("%s: vnode with usecount and VI_OWEINACT set", __func__));
 		return;
 	}
-	if (vp->v_usecount == 0)
-		vholdl(vp);
+	vholdl(vp);
 	if ((vp->v_iflag & VI_OWEINACT) != 0) {
 		vp->v_iflag &= ~VI_OWEINACT;
 		VNODE_REFCOUNT_FENCE_REL();
@@ -2912,12 +2888,6 @@ vrefact(struct vnode *vp)
 {
 
 	CTR2(KTR_VFS, "%s: vp %p", __func__, vp);
-	if (__predict_false(vp->v_type == VCHR)) {
-		VNASSERT(vp->v_holdcnt > 0 && vp->v_usecount > 0, vp,
-		    ("%s: wrong ref counts", __func__));
-		vref(vp);
-		return;
-	}
 #ifdef INVARIANTS
 	int old = atomic_fetchadd_int(&vp->v_usecount, 1);
 	VNASSERT(old > 0, vp, ("%s: wrong use count", __func__));
@@ -2986,31 +2956,22 @@ vputx(struct vnode *vp, int func)
 	 * We want to hold the vnode until the inactive finishes to
 	 * prevent vgone() races.  We drop the use count here and the
 	 * hold count below when we're done.
+	 *
+	 * If we release the last usecount we take ownership of the hold
+	 * count which provides liveness of the vnode, in which case we
+	 * have to vdrop.
 	 */
-	if (vp->v_type != VCHR) {
-		/*
-		 * If we release the last usecount we take ownership of the hold
-		 * count which provides liveness of the vnode, in which case we
-		 * have to vdrop.
-		 */
-		if (!refcount_release(&vp->v_usecount))
-			return;
-		VI_LOCK(vp);
-		/*
-		 * By the time we got here someone else might have transitioned
-		 * the count back to > 0.
-		 */
-		if (vp->v_usecount > 0) {
-			vdropl(vp);
-			return;
-		}
-	} else {
-		VI_LOCK(vp);
-		v_decr_devcount(vp);
-		if (!refcount_release(&vp->v_usecount)) {
-			VI_UNLOCK(vp);
-			return;
-		}
+	if (!refcount_release(&vp->v_usecount))
+		return;
+	VI_LOCK(vp);
+	v_decr_devcount(vp);
+	/*
+	 * By the time we got here someone else might have transitioned
+	 * the count back to > 0.
+	 */
+	if (vp->v_usecount > 0) {
+		vdropl(vp);
+		return;
 	}
 	if (vp->v_iflag & VI_DOINGINACT) {
 		vdropl(vp);
@@ -3737,20 +3698,6 @@ vcount(struct vnode *vp)
 	count = vp->v_rdev->si_usecount;
 	dev_unlock();
 	return (count);
-}
-
-/*
- * Same as above, but using the struct cdev *as argument
- */
-int
-count_dev(struct cdev *dev)
-{
-	int count;
-
-	dev_lock();
-	count = dev->si_usecount;
-	dev_unlock();
-	return(count);
 }
 
 /*

Modified: head/sys/kern/vfs_syscalls.c
==============================================================================
--- head/sys/kern/vfs_syscalls.c	Wed Nov 20 11:12:19 2019	(r354889)
+++ head/sys/kern/vfs_syscalls.c	Wed Nov 20 12:05:59 2019	(r354890)
@@ -4148,7 +4148,7 @@ sys_revoke(struct thread *td, struct revoke_args *uap)
 		if (error != 0)
 			goto out;
 	}
-	if (vcount(vp) > 1)
+	if (vp->v_usecount > 1 || vcount(vp) > 1)
 		VOP_REVOKE(vp, REVOKEALL);
 out:
 	vput(vp);

Modified: head/sys/sys/conf.h
==============================================================================
--- head/sys/sys/conf.h	Wed Nov 20 11:12:19 2019	(r354889)
+++ head/sys/sys/conf.h	Wed Nov 20 12:05:59 2019	(r354890)
@@ -256,7 +256,6 @@ void make_dev_args_init_impl(struct make_dev_args *_ar
 #define	make_dev_args_init(a) \
     make_dev_args_init_impl((a), sizeof(struct make_dev_args))
 	
-int	count_dev(struct cdev *_dev);
 void	delist_dev(struct cdev *_dev);
 void	destroy_dev(struct cdev *_dev);
 int	destroy_dev_sched(struct cdev *dev);



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