Date: Fri, 4 Feb 2011 09:15:12 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r218257 - projects/graid/head/sys/geom Message-ID: <201102040915.p149FCDU075819@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201102040915.p149FCDU075819>