From owner-svn-src-stable@freebsd.org Fri Apr 6 12:24:01 2018 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B88D1F806FA; Fri, 6 Apr 2018 12:24:00 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 6AF4C781EE; Fri, 6 Apr 2018 12:24:00 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 65CF511BD5; Fri, 6 Apr 2018 12:24:00 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w36CO0B5009952; Fri, 6 Apr 2018 12:24:00 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w36CO0me009950; Fri, 6 Apr 2018 12:24:00 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201804061224.w36CO0me009950@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Fri, 6 Apr 2018 12:24:00 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r332096 - stable/10/sys/geom X-SVN-Group: stable-10 X-SVN-Commit-Author: avg X-SVN-Commit-Paths: stable/10/sys/geom X-SVN-Commit-Revision: 332096 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Apr 2018 12:24:01 -0000 Author: avg Date: Fri Apr 6 12:23:59 2018 New Revision: 332096 URL: https://svnweb.freebsd.org/changeset/base/332096 Log: MFC r330977: g_access: deal with races created by geoms that drop the topology lock PR: 225960 Modified: stable/10/sys/geom/geom.h stable/10/sys/geom/geom_subr.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/geom/geom.h ============================================================================== --- stable/10/sys/geom/geom.h Fri Apr 6 12:13:32 2018 (r332095) +++ stable/10/sys/geom/geom.h Fri Apr 6 12:23:59 2018 (r332096) @@ -147,8 +147,10 @@ struct g_geom { void *spare1; void *softc; unsigned flags; -#define G_GEOM_WITHER 1 -#define G_GEOM_VOLATILE_BIO 2 +#define G_GEOM_WITHER 0x01 +#define G_GEOM_VOLATILE_BIO 0x02 +#define G_GEOM_IN_ACCESS 0x04 +#define G_GEOM_ACCESS_WAIT 0x08 }; /* Modified: stable/10/sys/geom/geom_subr.c ============================================================================== --- stable/10/sys/geom/geom_subr.c Fri Apr 6 12:13:32 2018 (r332095) +++ stable/10/sys/geom/geom_subr.c Fri Apr 6 12:23:59 2018 (r332096) @@ -859,7 +859,11 @@ int g_access(struct g_consumer *cp, int dcr, int dcw, int dce) { struct g_provider *pp; - int pr,pw,pe; + struct g_geom *gp; + int pw, pe; +#ifdef INVARIANTS + int sr, sw, se; +#endif int error; g_topology_assert(); @@ -867,6 +871,7 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int pp = cp->provider; KASSERT(pp != NULL, ("access but not attached")); G_VALID_PROVIDER(pp); + gp = pp->geom; g_trace(G_T_ACCESS, "g_access(%p(%s), %d, %d, %d)", cp, pp->name, dcr, dcw, dce); @@ -875,7 +880,7 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int KASSERT(cp->acw + dcw >= 0, ("access resulting in negative acw")); KASSERT(cp->ace + dce >= 0, ("access resulting in negative ace")); KASSERT(dcr != 0 || dcw != 0 || dce != 0, ("NOP access request")); - KASSERT(pp->geom->access != NULL, ("NULL geom->access")); + KASSERT(gp->access != NULL, ("NULL geom->access")); /* * If our class cares about being spoiled, and we have been, we @@ -887,10 +892,30 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int return (ENXIO); /* + * A number of GEOM classes either need to perform an I/O on the first + * open or to acquire a different subsystem's lock. To do that they + * may have to drop the topology lock. + * Other GEOM classes perform special actions when opening a lower rank + * geom for the first time. As a result, more than one thread may + * end up performing the special actions. + * So, we prevent concurrent "first" opens by marking the consumer with + * special flag. + * + * Note that if the geom's access method never drops the topology lock, + * then we will never see G_GEOM_IN_ACCESS here. + */ + while ((gp->flags & G_GEOM_IN_ACCESS) != 0) { + g_trace(G_T_ACCESS, + "%s: race on geom %s via provider %s and consumer of %s", + __func__, gp->name, pp->name, cp->geom->name); + gp->flags |= G_GEOM_ACCESS_WAIT; + g_topology_sleep(gp, 0); + } + + /* * Figure out what counts the provider would have had, if this * consumer had (r0w0e0) at this time. */ - pr = pp->acr - cp->acr; pw = pp->acw - cp->acw; pe = pp->ace - cp->ace; @@ -902,7 +927,7 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int pp, pp->name); /* If foot-shooting is enabled, any open on rank#1 is OK */ - if ((g_debugflags & 16) && pp->geom->rank == 1) + if ((g_debugflags & 16) && gp->rank == 1) ; /* If we try exclusive but already write: fail */ else if (dce > 0 && pw > 0) @@ -916,11 +941,27 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int /* Ok then... */ - error = pp->geom->access(pp, dcr, dcw, dce); +#ifdef INVARIANTS + sr = cp->acr; + sw = cp->acw; + se = cp->ace; +#endif + gp->flags |= G_GEOM_IN_ACCESS; + error = gp->access(pp, dcr, dcw, dce); KASSERT(dcr > 0 || dcw > 0 || dce > 0 || error == 0, ("Geom provider %s::%s dcr=%d dcw=%d dce=%d error=%d failed " - "closing ->access()", pp->geom->class->name, pp->name, dcr, dcw, + "closing ->access()", gp->class->name, pp->name, dcr, dcw, dce, error)); + + g_topology_assert(); + gp->flags &= ~G_GEOM_IN_ACCESS; + KASSERT(cp->acr == sr && cp->acw == sw && cp->ace == se, + ("Access counts changed during geom->access")); + if ((gp->flags & G_GEOM_ACCESS_WAIT) != 0) { + gp->flags &= ~G_GEOM_ACCESS_WAIT; + wakeup(gp); + } + if (!error) { /* * If we open first write, spoil any partner consumers. @@ -930,7 +971,7 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int if (pp->acw == 0 && dcw != 0) g_spoil(pp, cp); else if (pp->acw != 0 && pp->acw == -dcw && pp->error == 0 && - !(pp->geom->flags & G_GEOM_WITHER)) + !(gp->flags & G_GEOM_WITHER)) g_post_event(g_new_provider_event, pp, M_WAITOK, pp, NULL);