From e8adbe4499bad417dc9f414665a3a70ed9761ad8 Mon Sep 17 00:00:00 2001 From: Pawel Jakub Dawidek Date: Sun, 26 Sep 2004 20:41:07 +0000 Subject: [PATCH] Avoid race while synchronizing components. It is very hard to bump into, but it is possible: 1. Read data from good component for synchronization. 2. Write data to the same area. 3. Write synchronization data, which are now stale. Found by: tegge --- sys/geom/mirror/g_mirror.c | 39 ++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/sys/geom/mirror/g_mirror.c b/sys/geom/mirror/g_mirror.c index 76f93e0e29e7..a9db20c5eb13 100644 --- a/sys/geom/mirror/g_mirror.c +++ b/sys/geom/mirror/g_mirror.c @@ -392,6 +392,7 @@ g_mirror_init_disk(struct g_mirror_softc *sc, struct g_provider *pp, disk->d_sync.ds_consumer = NULL; disk->d_sync.ds_offset = md->md_sync_offset; disk->d_sync.ds_offset_done = md->md_sync_offset; + disk->d_sync.ds_resync = -1; disk->d_sync.ds_syncid = md->md_syncid; if (errorp != NULL) *errorp = 0; @@ -952,6 +953,9 @@ g_mirror_sync_request(struct bio *bp) return; } case BIO_WRITE: + { + struct g_mirror_disk_sync *sync; + if (bp->bio_error != 0) { G_MIRROR_LOGREQ(0, bp, "Synchronization request failed (error=%d).", @@ -964,17 +968,20 @@ g_mirror_sync_request(struct bio *bp) return; } G_MIRROR_LOGREQ(3, bp, "Synchronization request finished."); - disk->d_sync.ds_offset_done = bp->bio_offset + bp->bio_length; + sync = &disk->d_sync; + sync->ds_offset_done = bp->bio_offset + bp->bio_length; g_destroy_bio(bp); - if (disk->d_sync.ds_offset_done == sc->sc_provider->mediasize) { + if (sync->ds_resync != -1) + break; + if (sync->ds_offset_done == sc->sc_provider->mediasize) { /* * Disk up-to-date, activate it. */ g_mirror_event_send(disk, G_MIRROR_DISK_STATE_ACTIVE, G_MIRROR_EVENT_DONTWAIT); return; - } else if ((disk->d_sync.ds_offset_done % - (G_MIRROR_SYNC_BLOCK_SIZE * 100)) == 0) { + } else if (sync->ds_offset_done % + (G_MIRROR_SYNC_BLOCK_SIZE * 100) == 0) { /* * Update offset_done on every 100 blocks. * XXX: This should be configurable. @@ -984,6 +991,7 @@ g_mirror_sync_request(struct bio *bp) g_topology_unlock(); } return; + } default: KASSERT(1 == 0, ("Invalid command here: %u (device=%s)", bp->bio_cmd, sc->sc_name)); @@ -1205,6 +1213,7 @@ g_mirror_register_request(struct bio *bp) case BIO_DELETE: { struct g_mirror_disk *disk; + struct g_mirror_disk_sync *sync; struct bio_queue_head queue; struct g_consumer *cp; struct bio *cbp; @@ -1215,12 +1224,21 @@ g_mirror_register_request(struct bio *bp) */ bioq_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 >= disk->d_sync.ds_offset) + if (bp->bio_offset >= sync->ds_offset) continue; + else if (bp->bio_offset + bp->bio_length > + sync->ds_offset_done && + (bp->bio_offset < sync->ds_resync || + sync->ds_resync != -1)) { + sync->ds_resync = bp->bio_offset - + (bp->bio_offset % + G_MIRROR_SYNC_BLOCK_SIZE); + } break; default: continue; @@ -1330,6 +1348,7 @@ g_mirror_worker(void *arg) { struct g_mirror_softc *sc; struct g_mirror_disk *disk; + struct g_mirror_disk_sync *sync; struct g_mirror_event *ep; struct bio *bp; u_int nreqs; @@ -1411,13 +1430,17 @@ g_mirror_worker(void *arg) G_MIRROR_DISK_STATE_SYNCHRONIZING) { continue; } - if (disk->d_sync.ds_offset >= + sync = &disk->d_sync; + if (sync->ds_offset >= sc->sc_provider->mediasize) { continue; } - if (disk->d_sync.ds_offset > - disk->d_sync.ds_offset_done) { + if (sync->ds_offset > sync->ds_offset_done) continue; + if (sync->ds_resync != -1) { + sync->ds_offset = sync->ds_resync; + sync->ds_offset_done = sync->ds_resync; + sync->ds_resync = -1; } g_mirror_sync_one(disk); }