From owner-svn-src-head@FreeBSD.ORG Sat Dec 15 02:04:47 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 9BC0C98E; Sat, 15 Dec 2012 02:04:47 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 7FF328FC0A; Sat, 15 Dec 2012 02:04:47 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.5/8.14.5) with ESMTP id qBF24luu014328; Sat, 15 Dec 2012 02:04:47 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.5/8.14.5/Submit) id qBF24lZd014326; Sat, 15 Dec 2012 02:04:47 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201212150204.qBF24lZd014326@svn.freebsd.org> From: Konstantin Belousov Date: Sat, 15 Dec 2012 02:04:47 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r244240 - in head/sys: kern sys X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Dec 2012 02:04:47 -0000 Author: kib Date: Sat Dec 15 02:04:46 2012 New Revision: 244240 URL: http://svnweb.freebsd.org/changeset/base/244240 Log: When mnt_vnode_next_active iterator cannot lock the next vnode and yields, specify the user priority for the yield. Otherwise, a higher-priority (kernel) thread could fall into the priority-inversion with the thread owning the mutex lock. On single-processor machines or UP kernels, do not loop adaptively when the next vnode cannot be locked, instead yield unconditionally. Restructure the iteration initializer and the iterator to remove code duplication. Put the code to fetch and lock a vnode next to the current marker, into the mnt_vnode_next_active() function, and use it instead of repeating the loop. Reported by: hrs, rmacklem Tested by: pho Sponsored by: The FreeBSD Foundation MFC after: 3 days Modified: head/sys/kern/vfs_subr.c head/sys/sys/mount.h Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Sat Dec 15 02:03:59 2012 (r244239) +++ head/sys/kern/vfs_subr.c Sat Dec 15 02:04:46 2012 (r244240) @@ -69,6 +69,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -4710,32 +4711,54 @@ __mnt_vnode_markerfree_all(struct vnode * These are helper functions for filesystems to traverse their * active vnodes. See MNT_VNODE_FOREACH_ACTIVE() in sys/mount.h */ -struct vnode * -__mnt_vnode_next_active(struct vnode **mvp, struct mount *mp) +static void +mnt_vnode_markerfree_active(struct vnode **mvp, struct mount *mp) +{ + + KASSERT((*mvp)->v_mount == mp, ("marker vnode mount list mismatch")); + + MNT_ILOCK(mp); + MNT_REL(mp); + MNT_IUNLOCK(mp); + free(*mvp, M_VNODE_MARKER); + *mvp = NULL; +} + +#ifdef SMP +#define ALWAYS_YIELD (mp_ncpus == 1) +#else +#define ALWAYS_YIELD 1 +#endif + +static struct vnode * +mnt_vnode_next_active(struct vnode **mvp, struct mount *mp) { struct vnode *vp, *nvp; - if (should_yield()) - kern_yield(PRI_UNCHANGED); - mtx_lock(&vnode_free_list_mtx); -restart: + mtx_assert(&vnode_free_list_mtx, MA_OWNED); KASSERT((*mvp)->v_mount == mp, ("marker vnode mount list mismatch")); +restart: vp = TAILQ_NEXT(*mvp, v_actfreelist); + TAILQ_REMOVE(&mp->mnt_activevnodelist, *mvp, v_actfreelist); while (vp != NULL) { if (vp->v_type == VMARKER) { vp = TAILQ_NEXT(vp, v_actfreelist); continue; } if (!VI_TRYLOCK(vp)) { - if (should_yield()) { + if (ALWAYS_YIELD || should_yield()) { + TAILQ_INSERT_BEFORE(vp, *mvp, v_actfreelist); mtx_unlock(&vnode_free_list_mtx); - kern_yield(PRI_UNCHANGED); + kern_yield(PRI_USER); mtx_lock(&vnode_free_list_mtx); + goto restart; } - goto restart; + continue; } - if (vp->v_mount == mp && vp->v_type != VMARKER && - (vp->v_iflag & VI_DOOMED) == 0) + KASSERT(vp->v_type != VMARKER, ("locked marker %p", vp)); + KASSERT(vp->v_mount == mp || vp->v_mount == NULL, + ("alien vnode on the active list %p %p", vp, mp)); + if (vp->v_mount == mp && (vp->v_iflag & VI_DOOMED) == 0) break; nvp = TAILQ_NEXT(vp, v_actfreelist); VI_UNLOCK(vp); @@ -4745,22 +4768,31 @@ restart: /* Check if we are done */ if (vp == NULL) { mtx_unlock(&vnode_free_list_mtx); - __mnt_vnode_markerfree_active(mvp, mp); - mtx_assert(MNT_MTX(mp), MA_NOTOWNED); + mnt_vnode_markerfree_active(mvp, mp); 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); ASSERT_VI_LOCKED(vp, "active iter"); KASSERT((vp->v_iflag & VI_ACTIVE) != 0, ("Non-active vp %p", vp)); return (vp); } +#undef ALWAYS_YIELD + +struct vnode * +__mnt_vnode_next_active(struct vnode **mvp, struct mount *mp) +{ + + if (should_yield()) + kern_yield(PRI_UNCHANGED); + mtx_lock(&vnode_free_list_mtx); + return (mnt_vnode_next_active(mvp, mp)); +} struct vnode * __mnt_vnode_first_active(struct vnode **mvp, struct mount *mp) { - struct vnode *vp, *nvp; + struct vnode *vp; *mvp = malloc(sizeof(struct vnode), M_VNODE_MARKER, M_WAITOK | M_ZERO); MNT_ILOCK(mp); @@ -4770,44 +4802,14 @@ __mnt_vnode_first_active(struct vnode ** (*mvp)->v_mount = mp; mtx_lock(&vnode_free_list_mtx); -restart: vp = TAILQ_FIRST(&mp->mnt_activevnodelist); - while (vp != NULL) { - if (vp->v_type == VMARKER) { - vp = TAILQ_NEXT(vp, v_actfreelist); - continue; - } - if (!VI_TRYLOCK(vp)) { - 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 && - (vp->v_iflag & VI_DOOMED) == 0) - break; - nvp = TAILQ_NEXT(vp, v_actfreelist); - VI_UNLOCK(vp); - vp = nvp; - } - - /* 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; + mnt_vnode_markerfree_active(mvp, mp); return (NULL); } - TAILQ_INSERT_AFTER(&mp->mnt_activevnodelist, vp, *mvp, v_actfreelist); - mtx_unlock(&vnode_free_list_mtx); - ASSERT_VI_LOCKED(vp, "active iter first"); - KASSERT((vp->v_iflag & VI_ACTIVE) != 0, ("Non-active vp %p", vp)); - return (vp); + TAILQ_INSERT_BEFORE(vp, *mvp, v_actfreelist); + return (mnt_vnode_next_active(mvp, mp)); } void @@ -4817,14 +4819,8 @@ __mnt_vnode_markerfree_active(struct vno if (*mvp == NULL) return; - 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); - *mvp = NULL; + mnt_vnode_markerfree_active(mvp, mp); } Modified: head/sys/sys/mount.h ============================================================================== --- head/sys/sys/mount.h Sat Dec 15 02:03:59 2012 (r244239) +++ head/sys/sys/mount.h Sat Dec 15 02:04:46 2012 (r244240) @@ -218,17 +218,12 @@ struct vnode *__mnt_vnode_next_active(st struct vnode *__mnt_vnode_first_active(struct vnode **mvp, struct mount *mp); void __mnt_vnode_markerfree_active(struct vnode **mvp, struct mount *); -#define MNT_VNODE_FOREACH_ACTIVE(vp, mp, mvp) \ - for (vp = __mnt_vnode_first_active(&(mvp), (mp)); \ +#define MNT_VNODE_FOREACH_ACTIVE(vp, mp, mvp) \ + for (vp = __mnt_vnode_first_active(&(mvp), (mp)); \ (vp) != NULL; vp = __mnt_vnode_next_active(&(mvp), (mp))) #define MNT_VNODE_FOREACH_ACTIVE_ABORT(mp, mvp) \ - do { \ - MNT_ILOCK(mp); \ - __mnt_vnode_markerfree_active(&(mvp), (mp)); \ - /* MNT_IUNLOCK(mp); -- done in above function */ \ - mtx_assert(MNT_MTX(mp), MA_NOTOWNED); \ - } while (0) + __mnt_vnode_markerfree_active(&(mvp), (mp)) /* * Definitions for MNT_VNODE_FOREACH.