Date: Wed, 24 Jan 2018 15:15:18 +0000 (UTC) From: Mark Johnston <markj@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r328333 - stable/11/sys/geom/mirror Message-ID: <201801241515.w0OFFI74083795@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: markj Date: Wed Jan 24 15:15:18 2018 New Revision: 328333 URL: https://svnweb.freebsd.org/changeset/base/328333 Log: MFC r327496, r327760: Fix some I/O ordering issues in gmirror. Modified: stable/11/sys/geom/mirror/g_mirror.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/geom/mirror/g_mirror.c ============================================================================== --- stable/11/sys/geom/mirror/g_mirror.c Wed Jan 24 14:24:17 2018 (r328332) +++ stable/11/sys/geom/mirror/g_mirror.c Wed Jan 24 15:15:18 2018 (r328333) @@ -109,7 +109,8 @@ static void g_mirror_update_device(struct g_mirror_sof static void g_mirror_dumpconf(struct sbuf *sb, const char *indent, struct g_geom *gp, struct g_consumer *cp, struct g_provider *pp); static void g_mirror_sync_stop(struct g_mirror_disk *disk, int type); -static void g_mirror_register_request(struct bio *bp); +static void g_mirror_register_request(struct g_mirror_softc *sc, + struct bio *bp); static void g_mirror_sync_release(struct g_mirror_softc *sc); @@ -890,27 +891,6 @@ g_mirror_unidle(struct g_mirror_softc *sc) } static void -g_mirror_flush_done(struct bio *bp) -{ - struct g_mirror_softc *sc; - struct bio *pbp; - - pbp = bp->bio_parent; - sc = pbp->bio_to->private; - mtx_lock(&sc->sc_done_mtx); - if (pbp->bio_error == 0) - pbp->bio_error = bp->bio_error; - pbp->bio_completed += bp->bio_completed; - pbp->bio_inbed++; - if (pbp->bio_children == pbp->bio_inbed) { - mtx_unlock(&sc->sc_done_mtx); - g_io_deliver(pbp, pbp->bio_error); - } else - mtx_unlock(&sc->sc_done_mtx); - g_destroy_bio(bp); -} - -static void g_mirror_done(struct bio *bp) { struct g_mirror_softc *sc; @@ -924,16 +904,46 @@ g_mirror_done(struct bio *bp) } static void -g_mirror_regular_request(struct bio *bp) +g_mirror_regular_request_error(struct g_mirror_softc *sc, + struct g_mirror_disk *disk, struct bio *bp) { - struct g_mirror_softc *sc; + + if (bp->bio_cmd == BIO_FLUSH && bp->bio_error == EOPNOTSUPP) + return; + + if ((disk->d_flags & G_MIRROR_DISK_FLAG_BROKEN) == 0) { + disk->d_flags |= G_MIRROR_DISK_FLAG_BROKEN; + G_MIRROR_LOGREQ(0, bp, "Request failed (error=%d).", + bp->bio_error); + } else { + G_MIRROR_LOGREQ(1, bp, "Request failed (error=%d).", + bp->bio_error); + } + if (g_mirror_disconnect_on_failure && + g_mirror_ndisks(sc, G_MIRROR_DISK_STATE_ACTIVE) > 1) { + if (bp->bio_error == ENXIO && + bp->bio_cmd == BIO_READ) + sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID; + else if (bp->bio_error == ENXIO) + sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID_NOW; + else + sc->sc_bump_id |= G_MIRROR_BUMP_GENID; + g_mirror_event_send(disk, G_MIRROR_DISK_STATE_DISCONNECTED, + G_MIRROR_EVENT_DONTWAIT); + } +} + +static void +g_mirror_regular_request(struct g_mirror_softc *sc, struct bio *bp) +{ struct g_mirror_disk *disk; struct bio *pbp; g_topology_assert_not(); + KASSERT(sc->sc_provider == bp->bio_parent->bio_to, + ("regular request %p with unexpected origin", bp)); pbp = bp->bio_parent; - sc = pbp->bio_to->private; bp->bio_from->index--; if (bp->bio_cmd == BIO_WRITE || bp->bio_cmd == BIO_DELETE) sc->sc_writes--; @@ -944,12 +954,24 @@ g_mirror_regular_request(struct bio *bp) g_topology_unlock(); } - if (bp->bio_cmd == BIO_READ) + switch (bp->bio_cmd) { + case BIO_READ: KFAIL_POINT_ERROR(DEBUG_FP, g_mirror_regular_request_read, bp->bio_error); - else if (bp->bio_cmd == BIO_WRITE) + break; + case BIO_WRITE: KFAIL_POINT_ERROR(DEBUG_FP, g_mirror_regular_request_write, bp->bio_error); + break; + case BIO_DELETE: + KFAIL_POINT_ERROR(DEBUG_FP, g_mirror_regular_request_delete, + bp->bio_error); + break; + case BIO_FLUSH: + KFAIL_POINT_ERROR(DEBUG_FP, g_mirror_regular_request_flush, + bp->bio_error); + break; + } pbp->bio_inbed++; KASSERT(pbp->bio_inbed <= pbp->bio_children, @@ -973,35 +995,12 @@ g_mirror_regular_request(struct bio *bp) } else if (bp->bio_error != 0) { if (pbp->bio_error == 0) pbp->bio_error = bp->bio_error; - if (disk != NULL) { - if ((disk->d_flags & G_MIRROR_DISK_FLAG_BROKEN) == 0) { - disk->d_flags |= G_MIRROR_DISK_FLAG_BROKEN; - G_MIRROR_LOGREQ(0, bp, - "Request failed (error=%d).", - bp->bio_error); - } else { - G_MIRROR_LOGREQ(1, bp, - "Request failed (error=%d).", - bp->bio_error); - } - if (g_mirror_disconnect_on_failure && - g_mirror_ndisks(sc, G_MIRROR_DISK_STATE_ACTIVE) > 1) - { - if (bp->bio_error == ENXIO && - bp->bio_cmd == BIO_READ) - sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID; - else if (bp->bio_error == ENXIO) - sc->sc_bump_id |= G_MIRROR_BUMP_SYNCID_NOW; - else - sc->sc_bump_id |= G_MIRROR_BUMP_GENID; - g_mirror_event_send(disk, - G_MIRROR_DISK_STATE_DISCONNECTED, - G_MIRROR_EVENT_DONTWAIT); - } - } + if (disk != NULL) + g_mirror_regular_request_error(sc, disk, bp); switch (pbp->bio_cmd) { case BIO_DELETE: case BIO_WRITE: + case BIO_FLUSH: pbp->bio_inbed--; pbp->bio_children--; break; @@ -1026,6 +1025,7 @@ g_mirror_regular_request(struct bio *bp) break; case BIO_DELETE: case BIO_WRITE: + case BIO_FLUSH: if (pbp->bio_children == 0) { /* * All requests failed. @@ -1038,9 +1038,11 @@ g_mirror_regular_request(struct bio *bp) pbp->bio_error = 0; pbp->bio_completed = pbp->bio_length; } - TAILQ_REMOVE(&sc->sc_inflight, pbp, bio_queue); - /* Release delayed sync requests if possible. */ - g_mirror_sync_release(sc); + if (pbp->bio_cmd == BIO_WRITE || pbp->bio_cmd == BIO_DELETE) { + TAILQ_REMOVE(&sc->sc_inflight, pbp, bio_queue); + /* Release delayed sync requests if possible. */ + g_mirror_sync_release(sc); + } g_io_deliver(pbp, pbp->bio_error); break; default: @@ -1113,47 +1115,6 @@ g_mirror_kernel_dump(struct bio *bp) } static void -g_mirror_flush(struct g_mirror_softc *sc, struct bio *bp) -{ - struct bio_queue queue; - struct g_mirror_disk *disk; - struct g_consumer *cp; - struct bio *cbp; - - TAILQ_INIT(&queue); - LIST_FOREACH(disk, &sc->sc_disks, d_next) { - if (disk->d_state != G_MIRROR_DISK_STATE_ACTIVE) - continue; - cbp = g_clone_bio(bp); - if (cbp == NULL) { - while ((cbp = TAILQ_FIRST(&queue)) != NULL) { - TAILQ_REMOVE(&queue, cbp, bio_queue); - g_destroy_bio(cbp); - } - if (bp->bio_error == 0) - bp->bio_error = ENOMEM; - g_io_deliver(bp, bp->bio_error); - return; - } - TAILQ_INSERT_TAIL(&queue, cbp, bio_queue); - cbp->bio_done = g_mirror_flush_done; - cbp->bio_caller1 = disk; - cbp->bio_to = disk->d_consumer->provider; - } - while ((cbp = TAILQ_FIRST(&queue)) != NULL) { - TAILQ_REMOVE(&queue, cbp, bio_queue); - G_MIRROR_LOGREQ(3, cbp, "Sending request."); - disk = cbp->bio_caller1; - cbp->bio_caller1 = NULL; - cp = disk->d_consumer; - KASSERT(cp->acr >= 1 && cp->acw >= 1 && cp->ace >= 1, - ("Consumer %s not opened (r%dw%de%d).", cp->provider->name, - cp->acr, cp->acw, cp->ace)); - g_io_request(cbp, disk->d_consumer); - } -} - -static void g_mirror_start(struct bio *bp) { struct g_mirror_softc *sc; @@ -1172,10 +1133,8 @@ g_mirror_start(struct bio *bp) case BIO_READ: case BIO_WRITE: case BIO_DELETE: - break; case BIO_FLUSH: - g_mirror_flush(sc, bp); - return; + break; case BIO_GETATTR: if (!strcmp(bp->bio_attribute, "GEOM::candelete")) { g_mirror_candelete(bp); @@ -1257,14 +1216,14 @@ g_mirror_regular_collision(struct g_mirror_softc *sc, } /* - * Puts request onto delayed queue. + * Puts regular request onto delayed queue. */ static void g_mirror_regular_delay(struct g_mirror_softc *sc, struct bio *bp) { G_MIRROR_LOGREQ(2, bp, "Delaying request."); - TAILQ_INSERT_HEAD(&sc->sc_regular_delayed, bp, bio_queue); + TAILQ_INSERT_TAIL(&sc->sc_regular_delayed, bp, bio_queue); } /* @@ -1279,23 +1238,23 @@ g_mirror_sync_delay(struct g_mirror_softc *sc, struct } /* - * Releases delayed regular requests which don't collide anymore with sync - * requests. + * Requeue delayed regular requests. */ static void g_mirror_regular_release(struct g_mirror_softc *sc) { - struct bio *bp, *bp2; + struct bio *bp; - TAILQ_FOREACH_SAFE(bp, &sc->sc_regular_delayed, bio_queue, bp2) { - if (g_mirror_sync_collision(sc, bp)) - continue; - TAILQ_REMOVE(&sc->sc_regular_delayed, bp, bio_queue); - G_MIRROR_LOGREQ(2, bp, "Releasing delayed request (%p).", bp); - mtx_lock(&sc->sc_queue_mtx); - TAILQ_INSERT_HEAD(&sc->sc_queue, bp, bio_queue); - mtx_unlock(&sc->sc_queue_mtx); - } + if ((bp = TAILQ_FIRST(&sc->sc_regular_delayed)) == NULL) + return; + if (g_mirror_sync_collision(sc, bp)) + return; + + G_MIRROR_DEBUG(2, "Requeuing regular requests after collision."); + mtx_lock(&sc->sc_queue_mtx); + TAILQ_CONCAT(&sc->sc_regular_delayed, &sc->sc_queue, bio_queue); + TAILQ_SWAP(&sc->sc_regular_delayed, &sc->sc_queue, bio, bio_queue); + mtx_unlock(&sc->sc_queue_mtx); } /* @@ -1343,14 +1302,17 @@ g_mirror_sync_request_free(struct g_mirror_disk *disk, * send. */ static void -g_mirror_sync_request(struct bio *bp) +g_mirror_sync_request(struct g_mirror_softc *sc, struct bio *bp) { - struct g_mirror_softc *sc; struct g_mirror_disk *disk; struct g_mirror_disk_sync *sync; + KASSERT((bp->bio_cmd == BIO_READ && + bp->bio_from->geom == sc->sc_sync.ds_geom) || + (bp->bio_cmd == BIO_WRITE && bp->bio_from->geom == sc->sc_geom), + ("Sync BIO %p with unexpected origin", bp)); + bp->bio_from->index--; - sc = bp->bio_from->geom->softc; disk = bp->bio_from->private; if (disk == NULL) { sx_xunlock(&sc->sc_lock); /* Avoid recursion on sc_lock. */ @@ -1455,7 +1417,7 @@ g_mirror_sync_request(struct bio *bp) else g_io_request(bp, sync->ds_consumer); - /* Release delayed requests if possible. */ + /* Requeue delayed requests if possible. */ g_mirror_regular_release(sc); /* Find the smallest offset */ @@ -1683,11 +1645,26 @@ g_mirror_request_split(struct g_mirror_softc *sc, stru } static void -g_mirror_register_request(struct bio *bp) +g_mirror_register_request(struct g_mirror_softc *sc, struct bio *bp) { - struct g_mirror_softc *sc; + struct bio_queue queue; + struct bio *cbp; + struct g_consumer *cp; + struct g_mirror_disk *disk; - sc = bp->bio_to->private; + sx_assert(&sc->sc_lock, SA_XLOCKED); + + /* + * To avoid ordering issues, if a write is deferred because of a + * collision with a sync request, all I/O is deferred until that + * write is initiated. + */ + if (bp->bio_from->geom != sc->sc_sync.ds_geom && + !TAILQ_EMPTY(&sc->sc_regular_delayed)) { + g_mirror_regular_delay(sc, bp); + return; + } + switch (bp->bio_cmd) { case BIO_READ: switch (sc->sc_balance) { @@ -1707,13 +1684,6 @@ g_mirror_register_request(struct bio *bp) return; case BIO_WRITE: case BIO_DELETE: - { - struct bio_queue queue; - struct g_mirror_disk *disk; - struct g_mirror_disk_sync *sync; - struct g_consumer *cp; - struct bio *cbp; - /* * Delay the request if it is colliding with a synchronization * request. @@ -1742,12 +1712,11 @@ g_mirror_register_request(struct bio *bp) */ TAILQ_INIT(&queue); LIST_FOREACH(disk, &sc->sc_disks, d_next) { - sync = &disk->d_sync; switch (disk->d_state) { case G_MIRROR_DISK_STATE_ACTIVE: break; case G_MIRROR_DISK_STATE_SYNCHRONIZING: - if (bp->bio_offset >= sync->ds_offset) + if (bp->bio_offset >= disk->d_sync.ds_offset) continue; break; default: @@ -1777,6 +1746,8 @@ g_mirror_register_request(struct bio *bp) cp->provider->name, cp->acr, cp->acw, cp->ace)); } if (TAILQ_EMPTY(&queue)) { + KASSERT(bp->bio_cmd == BIO_DELETE, + ("No consumers for regular request %p", bp)); g_io_deliver(bp, EOPNOTSUPP); return; } @@ -1795,7 +1766,42 @@ g_mirror_register_request(struct bio *bp) */ TAILQ_INSERT_TAIL(&sc->sc_inflight, bp, bio_queue); return; - } + case BIO_FLUSH: + TAILQ_INIT(&queue); + LIST_FOREACH(disk, &sc->sc_disks, d_next) { + if (disk->d_state != G_MIRROR_DISK_STATE_ACTIVE) + continue; + cbp = g_clone_bio(bp); + if (cbp == NULL) { + while ((cbp = TAILQ_FIRST(&queue)) != NULL) { + TAILQ_REMOVE(&queue, cbp, bio_queue); + g_destroy_bio(cbp); + } + if (bp->bio_error == 0) + bp->bio_error = ENOMEM; + g_io_deliver(bp, bp->bio_error); + return; + } + TAILQ_INSERT_TAIL(&queue, cbp, bio_queue); + cbp->bio_done = g_mirror_done; + cbp->bio_caller1 = disk; + cbp->bio_to = disk->d_consumer->provider; + } + KASSERT(!TAILQ_EMPTY(&queue), + ("No consumers for regular request %p", bp)); + while ((cbp = TAILQ_FIRST(&queue)) != NULL) { + G_MIRROR_LOGREQ(3, cbp, "Sending request."); + TAILQ_REMOVE(&queue, cbp, bio_queue); + disk = cbp->bio_caller1; + cbp->bio_caller1 = NULL; + cp = disk->d_consumer; + KASSERT(cp->acr >= 1 && cp->acw >= 1 && cp->ace >= 1, + ("Consumer %s not opened (r%dw%de%d).", cp->provider->name, + cp->acr, cp->acw, cp->ace)); + cp->index++; + g_io_request(cbp, cp); + } + break; default: KASSERT(1 == 0, ("Invalid command here: %u (device=%s)", bp->bio_cmd, sc->sc_name)); @@ -1926,15 +1932,16 @@ g_mirror_worker(void *arg) G_MIRROR_DEBUG(5, "%s: I'm here 1.", __func__); continue; } + /* * Check if we can mark array as CLEAN and if we can't take * how much seconds should we wait. */ timeout = g_mirror_idle(sc, -1); + /* - * Now I/O requests. + * Handle I/O requests. */ - /* Get first request from the queue. */ mtx_lock(&sc->sc_queue_mtx); bp = TAILQ_FIRST(&sc->sc_queue); if (bp != NULL) @@ -1969,19 +1976,33 @@ g_mirror_worker(void *arg) if (bp->bio_from->geom == sc->sc_sync.ds_geom && (bp->bio_cflags & G_MIRROR_BIO_FLAG_SYNC) != 0) { - g_mirror_sync_request(bp); /* READ */ + /* + * Handle completion of the first half (the read) of a + * block synchronization operation. + */ + g_mirror_sync_request(sc, bp); } else if (bp->bio_to != sc->sc_provider) { if ((bp->bio_cflags & G_MIRROR_BIO_FLAG_REGULAR) != 0) - g_mirror_regular_request(bp); + /* + * Handle completion of a regular I/O request. + */ + g_mirror_regular_request(sc, bp); else if ((bp->bio_cflags & G_MIRROR_BIO_FLAG_SYNC) != 0) - g_mirror_sync_request(bp); /* WRITE */ + /* + * Handle completion of the second half (the + * write) of a block synchronization operation. + */ + g_mirror_sync_request(sc, bp); else { KASSERT(0, ("Invalid request cflags=0x%hx to=%s.", bp->bio_cflags, bp->bio_to->name)); } } else { - g_mirror_register_request(bp); + /* + * Initiate an I/O request. + */ + g_mirror_register_request(sc, bp); } G_MIRROR_DEBUG(5, "%s: I'm here 9.", __func__); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201801241515.w0OFFI74083795>