Date: Thu, 18 Apr 2013 11:13:48 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r249621 - stable/9/sys/geom Message-ID: <201304181113.r3IBDm11023317@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Thu Apr 18 11:13:48 2013 New Revision: 249621 URL: http://svnweb.freebsd.org/changeset/base/249621 Log: MFC r248694: In GEOM DISK: - Replace single done mutex with per-disk ones. On system with several disks on several HBAs that removes small, but measurable lock congestion. - Modify disk destruction process to not destroy the mutex prematurely. - Remove some extra pointer derefences. Modified: stable/9/sys/geom/geom_disk.c Modified: stable/9/sys/geom/geom_disk.c ============================================================================== --- stable/9/sys/geom/geom_disk.c Thu Apr 18 10:57:56 2013 (r249620) +++ stable/9/sys/geom/geom_disk.c Thu Apr 18 11:13:48 2013 (r249621) @@ -60,6 +60,7 @@ __FBSDID("$FreeBSD$"); #include <dev/led/led.h> struct g_disk_softc { + struct mtx done_mtx; struct disk *dp; struct sysctl_ctx_list sysctl_ctx; struct sysctl_oid *sysctl_tree; @@ -67,11 +68,7 @@ struct g_disk_softc { uint32_t state; }; -static struct mtx g_disk_done_mtx; - static g_access_t g_disk_access; -static g_init_t g_disk_init; -static g_fini_t g_disk_fini; static g_start_t g_disk_start; static g_ioctl_t g_disk_ioctl; static g_dumpconf_t g_disk_dumpconf; @@ -80,8 +77,6 @@ static g_provgone_t g_disk_providergone; static struct g_class g_disk_class = { .name = "DISK", .version = G_VERSION, - .init = g_disk_init, - .fini = g_disk_fini, .start = g_disk_start, .access = g_disk_access, .ioctl = g_disk_ioctl, @@ -93,20 +88,6 @@ SYSCTL_DECL(_kern_geom); static SYSCTL_NODE(_kern_geom, OID_AUTO, disk, CTLFLAG_RW, 0, "GEOM_DISK stuff"); -static void -g_disk_init(struct g_class *mp __unused) -{ - - mtx_init(&g_disk_done_mtx, "g_disk_done", NULL, MTX_DEF); -} - -static void -g_disk_fini(struct g_class *mp __unused) -{ - - mtx_destroy(&g_disk_done_mtx); -} - DECLARE_GEOM_CLASS(g_disk_class, g_disk); static void __inline @@ -135,7 +116,7 @@ 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(); - sc = pp->geom->softc; + sc = pp->private; if (sc == NULL || (dp = sc->dp) == NULL || dp->d_destroyed) { /* * Allow decreasing access count even if disk is not @@ -244,42 +225,36 @@ static void 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 ? */ - mtx_lock(&g_disk_done_mtx); - bp->bio_completed = bp->bio_length - bp->bio_resid; - bp2 = bp->bio_parent; + sc = bp2->bio_to->private; + bp->bio_completed = bp->bio_length - bp->bio_resid; + mtx_lock(&sc->done_mtx); if (bp2->bio_error == 0) bp2->bio_error = bp->bio_error; bp2->bio_completed += bp->bio_completed; - if ((bp->bio_cmd & (BIO_READ|BIO_WRITE|BIO_DELETE)) != 0 && - (sc = bp2->bio_to->geom->softc) != NULL && - (dp = sc->dp) != NULL) { - devstat_end_transaction_bio(dp->d_devstat, bp); - } + if ((bp->bio_cmd & (BIO_READ|BIO_WRITE|BIO_DELETE)) != 0) + devstat_end_transaction_bio(sc->dp->d_devstat, bp); g_destroy_bio(bp); bp2->bio_inbed++; if (bp2->bio_children == bp2->bio_inbed) { bp2->bio_resid = bp2->bio_bcount - bp2->bio_completed; g_io_deliver(bp2, bp2->bio_error); } - mtx_unlock(&g_disk_done_mtx); + mtx_unlock(&sc->done_mtx); } static int g_disk_ioctl(struct g_provider *pp, u_long cmd, void * data, int fflag, struct thread *td) { - struct g_geom *gp; struct disk *dp; struct g_disk_softc *sc; int error; - gp = pp->geom; - sc = gp->softc; + sc = pp->private; dp = sc->dp; if (dp->d_ioctl == NULL) @@ -299,7 +274,7 @@ g_disk_start(struct bio *bp) int error; off_t off; - sc = bp->bio_to->geom->softc; + sc = bp->bio_to->private; if (sc == NULL || (dp = sc->dp) == NULL || dp->d_destroyed) { g_io_deliver(bp, ENXIO); return; @@ -458,6 +433,7 @@ g_disk_create(void *arg, int flag) g_topology_assert(); dp = arg; sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO); + mtx_init(&sc->done_mtx, "g_disk_done", NULL, MTX_DEF); sc->dp = dp; gp = g_new_geomf(&g_disk_class, "%s%d", dp->d_name, dp->d_unit); gp->softc = sc; @@ -501,15 +477,7 @@ g_disk_providergone(struct g_provider *p struct disk *dp; struct g_disk_softc *sc; - sc = (struct g_disk_softc *)pp->geom->softc; - - /* - * If the softc is already NULL, then we've probably been through - * g_disk_destroy already; there is nothing for us to do anyway. - */ - if (sc == NULL) - return; - + sc = (struct g_disk_softc *)pp->private; dp = sc->dp; /* @@ -519,8 +487,21 @@ g_disk_providergone(struct g_provider *p * in g_disk_create for VERSION_01 and avoid touching the d_gone * field for old consumers. */ - if (!(dp->d_flags & DISKFLAG_LACKS_GONE) && dp->d_gone != NULL) + if (dp != NULL && (dp->d_flags & DISKFLAG_LACKS_GONE) == 0 && + dp->d_gone != NULL) dp->d_gone(dp); + if (sc->sysctl_tree != NULL) { + sysctl_ctx_free(&sc->sysctl_ctx); + sc->sysctl_tree = NULL; + } + if (sc->led[0] != 0) { + led_set(sc->led, "0"); + sc->led[0] = 0; + } + pp->private = NULL; + pp->geom->softc = NULL; + mtx_destroy(&sc->done_mtx); + g_free(sc); } static void @@ -535,16 +516,9 @@ g_disk_destroy(void *ptr, int flag) gp = dp->d_geom; if (gp != NULL) { sc = gp->softc; - if (sc->sysctl_tree != NULL) { - sysctl_ctx_free(&sc->sysctl_ctx); - sc->sysctl_tree = NULL; - } - if (sc->led[0] != 0) { - led_set(sc->led, "0"); - sc->led[0] = 0; - } - g_free(sc); - gp->softc = NULL; + if (sc != NULL) + sc->dp = NULL; + dp->d_geom = NULL; g_wither_geom(gp, ENXIO); } g_free(dp);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201304181113.r3IBDm11023317>