From owner-svn-src-projects@FreeBSD.ORG Fri Feb 4 09:15:12 2011 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C57971065674; Fri, 4 Feb 2011 09:15:12 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 7DC258FC17; Fri, 4 Feb 2011 09:15:12 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id p149FCrK075821; Fri, 4 Feb 2011 09:15:12 GMT (envelope-from mav@svn.freebsd.org) Received: (from mav@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id p149FCDU075819; Fri, 4 Feb 2011 09:15:12 GMT (envelope-from mav@svn.freebsd.org) Message-Id: <201102040915.p149FCDU075819@svn.freebsd.org> From: Alexander Motin Date: Fri, 4 Feb 2011 09:15:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org X-SVN-Group: projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r218257 - projects/graid/head/sys/geom X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Feb 2011 09:15:12 -0000 Author: mav Date: Fri Feb 4 09:15:12 2011 New Revision: 218257 URL: http://svn.freebsd.org/changeset/base/218257 Log: Do not use provider private field for storing softc pointer. Provider can be destroyed by disk_gone(), which is called while holding lock and makes impossible to destroy sysctl context or reliably queue destruction. New implementation adds one more pointer dereference, but IMHO it is better then expose internal kitchen to the public API. Modified: projects/graid/head/sys/geom/geom_disk.c Modified: projects/graid/head/sys/geom/geom_disk.c ============================================================================== --- projects/graid/head/sys/geom/geom_disk.c Fri Feb 4 08:51:45 2011 (r218256) +++ projects/graid/head/sys/geom/geom_disk.c Fri Feb 4 09:15:12 2011 (r218257) @@ -129,9 +129,8 @@ g_disk_access(struct g_provider *pp, int g_trace(G_T_ACCESS, "g_disk_access(%s, %d, %d, %d)", pp->name, r, w, e); g_topology_assert(); - dp = pp->geom->softc; - sc = pp->private; - if (dp == NULL || dp->d_destroyed || sc == NULL) { + sc = pp->geom->softc; + if (sc == NULL || (dp = sc->dp) == NULL || dp->d_destroyed) { /* * Allow decreasing access count even if disk is not * avaliable anymore. @@ -204,31 +203,27 @@ g_disk_kerneldump(struct bio *bp, struct } static void -g_disk_setstate(struct bio *bp, struct disk *dp) +g_disk_setstate(struct bio *bp, struct g_disk_softc *sc) { - struct g_disk_softc *sc; const char *cmd; - sc = bp->bio_to->private; - if (sc != NULL) { - memcpy(&sc->state, bp->bio_data, sizeof(sc->state)); - if (sc->led[0] != 0) { - switch (sc->state) { - case G_STATE_FAILED: - cmd = "1"; - break; - case G_STATE_REBUILD: - cmd = "f5"; - break; - case G_STATE_RESYNC: - cmd = "f1"; - break; - default: - cmd = "0"; - break; - } - led_set(sc->led, cmd); + memcpy(&sc->state, bp->bio_data, sizeof(sc->state)); + if (sc->led[0] != 0) { + switch (sc->state) { + case G_STATE_FAILED: + cmd = "1"; + break; + case G_STATE_REBUILD: + cmd = "f5"; + break; + case G_STATE_RESYNC: + cmd = "f1"; + break; + default: + cmd = "0"; + break; } + led_set(sc->led, cmd); } g_io_deliver(bp, 0); } @@ -238,6 +233,7 @@ g_disk_done(struct bio *bp) { struct bio *bp2; struct disk *dp; + struct g_disk_softc *sc; /* See "notes" for why we need a mutex here */ /* XXX: will witness accept a mix of Giant/unGiant drivers here ? */ @@ -249,7 +245,8 @@ g_disk_done(struct bio *bp) bp2->bio_error = bp->bio_error; bp2->bio_completed += bp->bio_completed; if ((bp->bio_cmd & (BIO_READ|BIO_WRITE|BIO_DELETE)) && - (dp = bp2->bio_to->geom->softc)) { + (sc = bp2->bio_to->geom->softc) && + (dp = sc->dp)) { devstat_end_transaction_bio(dp->d_devstat, bp); } g_destroy_bio(bp); @@ -266,10 +263,12 @@ g_disk_ioctl(struct g_provider *pp, u_lo { struct g_geom *gp; struct disk *dp; + struct g_disk_softc *sc; int error; gp = pp->geom; - dp = gp->softc; + sc = gp->softc; + dp = sc->dp; if (dp->d_ioctl == NULL) return (ENOIOCTL); @@ -284,11 +283,12 @@ g_disk_start(struct bio *bp) { struct bio *bp2, *bp3; struct disk *dp; + struct g_disk_softc *sc; int error; off_t off; - dp = bp->bio_to->geom->softc; - if (dp == NULL || dp->d_destroyed) { + sc = bp->bio_to->geom->softc; + if (sc == NULL || (dp = sc->dp) == NULL || dp->d_destroyed) { g_io_deliver(bp, ENXIO); return; } @@ -368,7 +368,7 @@ g_disk_start(struct bio *bp) else if (!strcmp(bp->bio_attribute, "GEOM::kerneldump")) g_disk_kerneldump(bp, dp); else if (!strcmp(bp->bio_attribute, "GEOM::setstate")) - g_disk_setstate(bp, dp); + g_disk_setstate(bp, sc); else error = ENOIOCTL; break; @@ -403,9 +403,10 @@ static void g_disk_dumpconf(struct sbuf *sb, const char *indent, struct g_geom *gp, struct g_consumer *cp, struct g_provider *pp) { struct disk *dp; + struct g_disk_softc *sc; - dp = gp->softc; - if (dp == NULL) + sc = gp->softc; + if (sc == NULL || (dp = sc->dp) == NULL) return; if (indent == NULL) { sbuf_printf(sb, " hd %u", dp->d_fwheads); @@ -433,8 +434,10 @@ g_disk_create(void *arg, int flag) return; g_topology_assert(); dp = arg; + sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO); + sc->dp = dp; gp = g_new_geomf(&g_disk_class, "%s%d", dp->d_name, dp->d_unit); - gp->softc = dp; + gp->softc = sc; pp = g_new_providerf(gp, "%s", gp->name); pp->mediasize = dp->d_mediasize; pp->sectorsize = dp->d_sectorsize; @@ -444,8 +447,6 @@ g_disk_create(void *arg, int flag) pp->stripesize = dp->d_stripesize; if (bootverbose) printf("GEOM: new disk %s\n", gp->name); - sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO); - sc->dp = dp; sysctl_ctx_init(&sc->sysctl_ctx); snprintf(tmpstr, sizeof(tmpstr), "GEOM disk %s", gp->name); sc->sysctl_tree = SYSCTL_ADD_NODE(&sc->sysctl_ctx, @@ -476,15 +477,16 @@ g_disk_destroy(void *ptr, int flag) dp = ptr; gp = dp->d_geom; if (gp != NULL) { - sc = LIST_FIRST(&gp->provider)->private; + sc = gp->softc; if (sc->sysctl_tree != NULL) { sysctl_ctx_free(&sc->sysctl_ctx); sc->sysctl_tree = NULL; } - if (sc->led[0] != 0) + if (sc->led[0] != 0) { led_set(sc->led, "0"); + sc->led[0] = 0; + } g_free(sc); - LIST_FIRST(&gp->provider)->private = NULL; gp->softc = NULL; g_wither_geom(gp, ENXIO); }