Date: Thu, 6 Oct 2016 14:53:53 -0700 From: Ngie Cooper <yaneurabeya@gmail.com> To: Alexander Motin <mav@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Mark Johnston <markj@FreeBSD.org> Subject: Re: svn commit: r306762 - head/sys/geom/mirror Message-ID: <11B3DF25-71B6-4F0C-A11A-9ADD2CB81AE4@gmail.com> In-Reply-To: <201610061520.u96FK5uH037120@repo.freebsd.org> References: <201610061520.u96FK5uH037120@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Oct 6, 2016, at 08:20, Alexander Motin <mav@FreeBSD.org> wrote: >=20 > Author: mav > Date: Thu Oct 6 15:20:05 2016 > New Revision: 306762 > URL: https://svnweb.freebsd.org/changeset/base/306762 >=20 > Log: > Fix possible geom destruction before final provider close. >=20 > Introduce internal counter to track opens. Using provider's counters is > not very successfull after calling g_wither_provider(). >=20 > MFC after: 2 weeks > Sponsored by: iXsystems, Inc. Could you please include Mark and me on all reviews for gmirror? Thanks! -Ngie > Modified: > head/sys/geom/mirror/g_mirror.c > head/sys/geom/mirror/g_mirror.h > head/sys/geom/mirror/g_mirror_ctl.c >=20 > Modified: head/sys/geom/mirror/g_mirror.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D > --- head/sys/geom/mirror/g_mirror.c Thu Oct 6 14:55:15 2016 (r30676= 1) > +++ head/sys/geom/mirror/g_mirror.c Thu Oct 6 15:20:05 2016 (r30676= 2) > @@ -2153,10 +2153,9 @@ g_mirror_destroy_provider(struct g_mirro > } > } > mtx_unlock(&sc->sc_queue_mtx); > - G_MIRROR_DEBUG(0, "Device %s: provider %s destroyed.", sc->sc_name, > - sc->sc_provider->name); > g_wither_provider(sc->sc_provider, ENXIO); > sc->sc_provider =3D NULL; > + G_MIRROR_DEBUG(0, "Device %s: provider destroyed.", sc->sc_name); > g_topology_unlock(); > LIST_FOREACH(disk, &sc->sc_disks, d_next) { > if (disk->d_state =3D=3D G_MIRROR_DISK_STATE_SYNCHRONIZING) > @@ -2889,7 +2888,7 @@ static int > g_mirror_access(struct g_provider *pp, int acr, int acw, int ace) > { > struct g_mirror_softc *sc; > - int dcr, dcw, dce, error =3D 0; > + int error =3D 0; >=20 > g_topology_assert(); > G_MIRROR_DEBUG(2, "Access request for %s: r%dw%de%d.", pp->name, acr, > @@ -2900,30 +2899,21 @@ g_mirror_access(struct g_provider *pp, i > return (0); > KASSERT(sc !=3D NULL, ("NULL softc (provider=3D%s).", pp->name)); >=20 > - dcr =3D pp->acr + acr; > - dcw =3D pp->acw + acw; > - dce =3D pp->ace + ace; > - > g_topology_unlock(); > sx_xlock(&sc->sc_lock); > if ((sc->sc_flags & G_MIRROR_DEVICE_FLAG_DESTROY) !=3D 0 || > + (sc->sc_flags & G_MIRROR_DEVICE_FLAG_DESTROYING) !=3D 0 || > LIST_EMPTY(&sc->sc_disks)) { > if (acr > 0 || acw > 0 || ace > 0) > error =3D ENXIO; > goto end; > } > - if (dcw =3D=3D 0) > - g_mirror_idle(sc, dcw); > - if ((sc->sc_flags & G_MIRROR_DEVICE_FLAG_DESTROYING) !=3D 0) { > - if (acr > 0 || acw > 0 || ace > 0) { > - error =3D ENXIO; > - goto end; > - } > - if (dcr =3D=3D 0 && dcw =3D=3D 0 && dce =3D=3D 0) { > - g_post_event(g_mirror_destroy_delayed, sc, M_WAITOK, > - sc, NULL); > - } > - } > + sc->sc_provider_open +=3D acr + acw + ace; > + if (pp->acw + acw =3D=3D 0) > + g_mirror_idle(sc, 0); > + if ((sc->sc_flags & G_MIRROR_DEVICE_FLAG_DESTROYING) !=3D 0 && > + sc->sc_provider_open =3D=3D 0) > + g_post_event(g_mirror_destroy_delayed, sc, M_WAITOK, sc, NULL); > end: > sx_xunlock(&sc->sc_lock); > g_topology_lock(); > @@ -2980,6 +2970,7 @@ g_mirror_create(struct g_class *mp, cons > gp->softc =3D sc; > sc->sc_geom =3D gp; > sc->sc_provider =3D NULL; > + sc->sc_provider_open =3D 0; > /* > * Synchronization geom. > */ > @@ -3020,26 +3011,23 @@ int > g_mirror_destroy(struct g_mirror_softc *sc, int how) > { > struct g_mirror_disk *disk; > - struct g_provider *pp; >=20 > g_topology_assert_not(); > if (sc =3D=3D NULL) > return (ENXIO); > sx_assert(&sc->sc_lock, SX_XLOCKED); >=20 > - pp =3D sc->sc_provider; > - if (pp !=3D NULL && (pp->acr !=3D 0 || pp->acw !=3D 0 || pp->ace !=3D= 0 || > - SCHEDULER_STOPPED())) { > + if (sc->sc_provider_open !=3D 0 || SCHEDULER_STOPPED()) { > switch (how) { > case G_MIRROR_DESTROY_SOFT: > G_MIRROR_DEBUG(1, > - "Device %s is still open (r%dw%de%d).", pp->name, > - pp->acr, pp->acw, pp->ace); > + "Device %s is still open (%d).", sc->sc_name, > + sc->sc_provider_open); > return (EBUSY); > case G_MIRROR_DESTROY_DELAYED: > G_MIRROR_DEBUG(1, > "Device %s will be destroyed on last close.", > - pp->name); > + sc->sc_name); > LIST_FOREACH(disk, &sc->sc_disks, d_next) { > if (disk->d_state =3D=3D > G_MIRROR_DISK_STATE_SYNCHRONIZING) { > @@ -3050,7 +3038,7 @@ g_mirror_destroy(struct g_mirror_softc * > return (EBUSY); > case G_MIRROR_DESTROY_HARD: > G_MIRROR_DEBUG(1, "Device %s is still open, so it " > - "can't be definitely removed.", pp->name); > + "can't be definitely removed.", sc->sc_name); > } > } >=20 >=20 > Modified: head/sys/geom/mirror/g_mirror.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D > --- head/sys/geom/mirror/g_mirror.h Thu Oct 6 14:55:15 2016 (r30676= 1) > +++ head/sys/geom/mirror/g_mirror.h Thu Oct 6 15:20:05 2016 (r30676= 2) > @@ -179,6 +179,7 @@ struct g_mirror_softc { >=20 > struct g_geom *sc_geom; > struct g_provider *sc_provider; > + int sc_provider_open; >=20 > uint32_t sc_id; /* Mirror unique ID. */ >=20 >=20 > Modified: head/sys/geom/mirror/g_mirror_ctl.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D > --- head/sys/geom/mirror/g_mirror_ctl.c Thu Oct 6 14:55:15 2016 (r3= 06761) > +++ head/sys/geom/mirror/g_mirror_ctl.c Thu Oct 6 15:20:05 2016 (r3= 06762) > @@ -658,8 +658,7 @@ g_mirror_ctl_resize(struct gctl_req *req > return; > } > /* Deny shrinking of an opened provider */ > - if ((g_debugflags & 16) =3D=3D 0 && (sc->sc_provider->acr > 0 || > - sc->sc_provider->acw > 0 || sc->sc_provider->ace > 0)) { > + if ((g_debugflags & 16) =3D=3D 0 && sc->sc_provider_open > 0) { > if (sc->sc_mediasize > mediasize) { > gctl_error(req, "Device %s is busy.", > sc->sc_provider->name); >=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?11B3DF25-71B6-4F0C-A11A-9ADD2CB81AE4>