Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 10 Dec 2012 20:44:10 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r244095 - head/sys/kern
Message-ID:  <201212102044.qBAKiAb1009462@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Mon Dec 10 20:44:09 2012
New Revision: 244095
URL: http://svnweb.freebsd.org/changeset/base/244095

Log:
  Do not yield while owning a mutex.  The Giant reacquire in the
  kern_yield() is problematic than.
  
  The owned mutex is the mount interlock, and it is in fact not needed
  to guarantee the stability of the mount list of active vnodes, so fix
  the the issue by only taking the mount interlock for MNT_REF and
  MNT_REL operations.
  
  While there, augment the unconditional yield by some amount of
  spinning [1].
  
  Reported and tested by:	pho
  Reviewed by:	attilio
  Submitted by:	attilio [1]
  MFC after:	3 days

Modified:
  head/sys/kern/vfs_subr.c

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Mon Dec 10 20:10:20 2012	(r244094)
+++ head/sys/kern/vfs_subr.c	Mon Dec 10 20:44:09 2012	(r244095)
@@ -4717,9 +4717,8 @@ __mnt_vnode_next_active(struct vnode **m
 
 	if (should_yield())
 		kern_yield(PRI_UNCHANGED);
-	MNT_ILOCK(mp);
-restart:
 	mtx_lock(&vnode_free_list_mtx);
+restart:
 	KASSERT((*mvp)->v_mount == mp, ("marker vnode mount list mismatch"));
 	vp = TAILQ_NEXT(*mvp, v_actfreelist);
 	while (vp != NULL) {
@@ -4728,8 +4727,11 @@ restart:
 			continue;
 		}
 		if (!VI_TRYLOCK(vp)) {
-			mtx_unlock(&vnode_free_list_mtx);
-			kern_yield(PRI_UNCHANGED);
+			if (should_yield()) {
+				mtx_unlock(&vnode_free_list_mtx);
+				kern_yield(PRI_UNCHANGED);
+				mtx_lock(&vnode_free_list_mtx);
+			}
 			goto restart;
 		}
 		if (vp->v_mount == mp && vp->v_type != VMARKER &&
@@ -4744,14 +4746,12 @@ restart:
 	if (vp == NULL) {
 		mtx_unlock(&vnode_free_list_mtx);
 		__mnt_vnode_markerfree_active(mvp, mp);
-		/* MNT_IUNLOCK(mp); -- done in above function */
 		mtx_assert(MNT_MTX(mp), MA_NOTOWNED);
 		return (NULL);
 	}
 	TAILQ_REMOVE(&mp->mnt_activevnodelist, *mvp, v_actfreelist);
 	TAILQ_INSERT_AFTER(&mp->mnt_activevnodelist, vp, *mvp, v_actfreelist);
 	mtx_unlock(&vnode_free_list_mtx);
-	MNT_IUNLOCK(mp);
 	ASSERT_VI_LOCKED(vp, "active iter");
 	KASSERT((vp->v_iflag & VI_ACTIVE) != 0, ("Non-active vp %p", vp));
 	return (vp);
@@ -4765,10 +4765,12 @@ __mnt_vnode_first_active(struct vnode **
 	*mvp = malloc(sizeof(struct vnode), M_VNODE_MARKER, M_WAITOK | M_ZERO);
 	MNT_ILOCK(mp);
 	MNT_REF(mp);
+	MNT_IUNLOCK(mp);
 	(*mvp)->v_type = VMARKER;
+	(*mvp)->v_mount = mp;
 
-restart:
 	mtx_lock(&vnode_free_list_mtx);
+restart:
 	vp = TAILQ_FIRST(&mp->mnt_activevnodelist);
 	while (vp != NULL) {
 		if (vp->v_type == VMARKER) {
@@ -4776,8 +4778,11 @@ restart:
 			continue;
 		}
 		if (!VI_TRYLOCK(vp)) {
-			mtx_unlock(&vnode_free_list_mtx);
-			kern_yield(PRI_UNCHANGED);
+			if (should_yield()) {
+				mtx_unlock(&vnode_free_list_mtx);
+				kern_yield(PRI_UNCHANGED);
+				mtx_lock(&vnode_free_list_mtx);
+			}
 			goto restart;
 		}
 		if (vp->v_mount == mp && vp->v_type != VMARKER &&
@@ -4791,16 +4796,15 @@ restart:
 	/* Check if we are done */
 	if (vp == NULL) {
 		mtx_unlock(&vnode_free_list_mtx);
+		MNT_ILOCK(mp);
 		MNT_REL(mp);
 		MNT_IUNLOCK(mp);
 		free(*mvp, M_VNODE_MARKER);
 		*mvp = NULL;
 		return (NULL);
 	}
-	(*mvp)->v_mount = mp;
 	TAILQ_INSERT_AFTER(&mp->mnt_activevnodelist, vp, *mvp, v_actfreelist);
 	mtx_unlock(&vnode_free_list_mtx);
-	MNT_IUNLOCK(mp);
 	ASSERT_VI_LOCKED(vp, "active iter first");
 	KASSERT((vp->v_iflag & VI_ACTIVE) != 0, ("Non-active vp %p", vp));
 	return (vp);
@@ -4810,17 +4814,15 @@ void
 __mnt_vnode_markerfree_active(struct vnode **mvp, struct mount *mp)
 {
 
-	if (*mvp == NULL) {
-		MNT_IUNLOCK(mp);
+	if (*mvp == NULL)
 		return;
-	}
-
-	mtx_assert(MNT_MTX(mp), MA_OWNED);
 
 	KASSERT((*mvp)->v_mount == mp, ("marker vnode mount list mismatch"));
+
 	mtx_lock(&vnode_free_list_mtx);
 	TAILQ_REMOVE(&mp->mnt_activevnodelist, *mvp, v_actfreelist);
 	mtx_unlock(&vnode_free_list_mtx);
+	MNT_ILOCK(mp);
 	MNT_REL(mp);
 	MNT_IUNLOCK(mp);
 	free(*mvp, M_VNODE_MARKER);

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