Author: markj
Date: Wed Jan 24 15:16:17 2018
New Revision: 328334
URL: https://svnweb.freebsd.org/changeset/base/328334
Log:
MFC r327779, r327780:
Fix handling of read errors during synchronization.
Added:
stable/11/tests/sys/geom/class/mirror/sync_error.sh
- copied unchanged from r327780,
head/tests/sys/geom/class/mirror/sync_error.sh
Modified:
stable/11/sys/geom/mirror/g_mirror.c
stable/11/tests/sys/geom/class/mirror/Makefile
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 15:15:18 2018
(r328333)
+++ stable/11/sys/geom/mirror/g_mirror.c Wed Jan 24 15:16:17 2018
(r328334)
@@ -108,6 +108,8 @@ static int g_mirror_update_disk(struct g_mirror_disk *
static void g_mirror_update_device(struct g_mirror_softc *sc, bool force);
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_reinit(const struct g_mirror_disk *disk,
+ struct bio *bp, off_t offset);
static void g_mirror_sync_stop(struct g_mirror_disk *disk, int type);
static void g_mirror_register_request(struct g_mirror_softc *sc,
struct bio *bp);
@@ -1296,10 +1298,11 @@ g_mirror_sync_request_free(struct g_mirror_disk *disk,
/*
* Handle synchronization requests.
- * Every synchronization request is two-steps process: first, READ request is
- * send to active provider and then WRITE request (with read data) to the
provider
- * being synchronized. When WRITE is finished, new synchronization request is
- * send.
+ * Every synchronization request is a two-step process: first, a read request
is
+ * sent to the mirror provider via the sync consumer. If that request completes
+ * successfully, it is converted to a write and sent to the disk being
+ * synchronized. If the write also completes successfully, the synchronization
+ * offset is advanced and a new read request is submitted.
*/
static void
g_mirror_sync_request(struct g_mirror_softc *sc, struct bio *bp)
@@ -1324,13 +1327,16 @@ g_mirror_sync_request(struct g_mirror_softc *sc, struc
return;
}
+ sync = &disk->d_sync;
+
/*
* Synchronization request.
*/
switch (bp->bio_cmd) {
- case BIO_READ:
- {
+ case BIO_READ: {
+ struct g_mirror_disk *d;
struct g_consumer *cp;
+ int readable;
KFAIL_POINT_ERROR(DEBUG_FP, g_mirror_sync_request_read,
bp->bio_error);
@@ -1339,7 +1345,33 @@ g_mirror_sync_request(struct g_mirror_softc *sc, struc
G_MIRROR_LOGREQ(0, bp,
"Synchronization request failed (error=%d).",
bp->bio_error);
- g_mirror_sync_request_free(disk, bp);
+
+ /*
+ * If there's at least one other disk from which we can
+ * read the block, retry the request.
+ */
+ readable = 0;
+ LIST_FOREACH(d, &sc->sc_disks, d_next)
+ if (d->d_state == G_MIRROR_DISK_STATE_ACTIVE &&
+ !(d->d_flags & G_MIRROR_DISK_FLAG_BROKEN))
+ readable++;
+
+ /*
+ * The read error will trigger a syncid bump, so there's
+ * no need to do that here.
+ *
+ * If we can retry the read from another disk, do so.
+ * Otherwise, all we can do is kick out the new disk.
+ */
+ if (readable == 0) {
+ g_mirror_sync_request_free(disk, bp);
+ g_mirror_event_send(disk,
+ G_MIRROR_DISK_STATE_DISCONNECTED,
+ G_MIRROR_EVENT_DONTWAIT);
+ } else {
+ g_mirror_sync_reinit(disk, bp, bp->bio_offset);
+ goto retry_read;
+ }
return;
}
G_MIRROR_LOGREQ(3, bp,
@@ -1353,12 +1385,10 @@ g_mirror_sync_request(struct g_mirror_softc *sc, struc
cp->index++;
g_io_request(bp, cp);
return;
- }
- case BIO_WRITE:
- {
+ }
+ case BIO_WRITE: {
off_t offset;
- void *data;
- int i, idx;
+ int i;
KFAIL_POINT_ERROR(DEBUG_FP, g_mirror_sync_request_write,
bp->bio_error);
@@ -1375,7 +1405,6 @@ g_mirror_sync_request(struct g_mirror_softc *sc, struc
return;
}
G_MIRROR_LOGREQ(3, bp, "Synchronization request finished.");
- sync = &disk->d_sync;
if (sync->ds_offset >= sc->sc_mediasize ||
sync->ds_consumer == NULL ||
(sc->sc_flags & G_MIRROR_DEVICE_FLAG_DESTROY) != 0) {
@@ -1395,20 +1424,13 @@ g_mirror_sync_request(struct g_mirror_softc *sc, struc
}
/* Send next synchronization request. */
- data = bp->bio_data;
- idx = (int)(uintptr_t)bp->bio_caller1;
- g_reset_bio(bp);
- bp->bio_cmd = BIO_READ;
- bp->bio_offset = sync->ds_offset;
- bp->bio_length = MIN(MAXPHYS, sc->sc_mediasize -
bp->bio_offset);
+ g_mirror_sync_reinit(disk, bp, sync->ds_offset);
sync->ds_offset += bp->bio_length;
- bp->bio_done = g_mirror_sync_done;
- bp->bio_data = data;
- bp->bio_from = sync->ds_consumer;
- bp->bio_to = sc->sc_provider;
- bp->bio_caller1 = (void *)(uintptr_t)idx;
+
+retry_read:
G_MIRROR_LOGREQ(3, bp, "Sending synchronization request.");
sync->ds_consumer->index++;
+
/*
* Delay the request if it is colliding with a regular request.
*/
@@ -1434,11 +1456,9 @@ g_mirror_sync_request(struct g_mirror_softc *sc, struc
sync->ds_update_ts = time_uptime;
}
return;
- }
+ }
default:
- KASSERT(1 == 0, ("Invalid command here: %u (device=%s)",
- bp->bio_cmd, sc->sc_name));
- break;
+ panic("Invalid I/O request %p", bp);
}
}
@@ -2029,15 +2049,39 @@ g_mirror_update_idle(struct g_mirror_softc *sc, struct
}
static void
+g_mirror_sync_reinit(const struct g_mirror_disk *disk, struct bio *bp,
+ off_t offset)
+{
+ void *data;
+ int idx;
+
+ data = bp->bio_data;
+ idx = (int)(uintptr_t)bp->bio_caller1;
+ g_reset_bio(bp);
+
+ bp->bio_cmd = BIO_READ;
+ bp->bio_data = data;
+ bp->bio_done = g_mirror_sync_done;
+ bp->bio_from = disk->d_sync.ds_consumer;
+ bp->bio_to = disk->d_softc->sc_provider;
+ bp->bio_caller1 = (void *)(uintptr_t)idx;
+ bp->bio_offset = offset;
+ bp->bio_length = MIN(MAXPHYS,
+ disk->d_softc->sc_mediasize - bp->bio_offset);
+}
+
+static void
g_mirror_sync_start(struct g_mirror_disk *disk)
{
struct g_mirror_softc *sc;
+ struct g_mirror_disk_sync *sync;
struct g_consumer *cp;
struct bio *bp;
int error, i;
g_topology_assert_not();
sc = disk->d_softc;
+ sync = &disk->d_sync;
sx_assert(&sc->sc_lock, SX_LOCKED);
KASSERT(disk->d_state == G_MIRROR_DISK_STATE_SYNCHRONIZING,
@@ -2063,54 +2107,48 @@ g_mirror_sync_start(struct g_mirror_disk *disk)
g_mirror_get_diskname(disk));
if ((sc->sc_flags & G_MIRROR_DEVICE_FLAG_NOFAILSYNC) == 0)
disk->d_flags |= G_MIRROR_DISK_FLAG_DIRTY;
- KASSERT(disk->d_sync.ds_consumer == NULL,
+ KASSERT(sync->ds_consumer == NULL,
("Sync consumer already exists (device=%s, disk=%s).",
sc->sc_name, g_mirror_get_diskname(disk)));
- disk->d_sync.ds_consumer = cp;
- disk->d_sync.ds_consumer->private = disk;
- disk->d_sync.ds_consumer->index = 0;
+ sync->ds_consumer = cp;
+ sync->ds_consumer->private = disk;
+ sync->ds_consumer->index = 0;
/*
* Allocate memory for synchronization bios and initialize them.
*/
- disk->d_sync.ds_bios = malloc(sizeof(struct bio *) * g_mirror_syncreqs,
+ sync->ds_bios = malloc(sizeof(struct bio *) * g_mirror_syncreqs,
M_MIRROR, M_WAITOK);
for (i = 0; i < g_mirror_syncreqs; i++) {
bp = g_alloc_bio();
- disk->d_sync.ds_bios[i] = bp;
- bp->bio_parent = NULL;
- bp->bio_cmd = BIO_READ;
+ sync->ds_bios[i] = bp;
+
bp->bio_data = malloc(MAXPHYS, M_MIRROR, M_WAITOK);
- bp->bio_cflags = 0;
- bp->bio_offset = disk->d_sync.ds_offset;
- bp->bio_length = MIN(MAXPHYS, sc->sc_mediasize -
bp->bio_offset);
- disk->d_sync.ds_offset += bp->bio_length;
- bp->bio_done = g_mirror_sync_done;
- bp->bio_from = disk->d_sync.ds_consumer;
- bp->bio_to = sc->sc_provider;
bp->bio_caller1 = (void *)(uintptr_t)i;
+ g_mirror_sync_reinit(disk, bp, sync->ds_offset);
+ sync->ds_offset += bp->bio_length;
}
/* Increase the number of disks in SYNCHRONIZING state. */
sc->sc_sync.ds_ndisks++;
/* Set the number of in-flight synchronization requests. */
- disk->d_sync.ds_inflight = g_mirror_syncreqs;
+ sync->ds_inflight = g_mirror_syncreqs;
/*
* Fire off first synchronization requests.
*/
for (i = 0; i < g_mirror_syncreqs; i++) {
- bp = disk->d_sync.ds_bios[i];
+ bp = sync->ds_bios[i];
G_MIRROR_LOGREQ(3, bp, "Sending synchronization request.");
- disk->d_sync.ds_consumer->index++;
+ sync->ds_consumer->index++;
/*
* Delay the request if it is colliding with a regular request.
*/
if (g_mirror_regular_collision(sc, bp))
g_mirror_sync_delay(sc, bp);
else
- g_io_request(bp, disk->d_sync.ds_consumer);
+ g_io_request(bp, sync->ds_consumer);
}
}
Modified: stable/11/tests/sys/geom/class/mirror/Makefile
==============================================================================
--- stable/11/tests/sys/geom/class/mirror/Makefile Wed Jan 24 15:15:18
2018 (r328333)
+++ stable/11/tests/sys/geom/class/mirror/Makefile Wed Jan 24 15:16:17
2018 (r328334)
@@ -18,6 +18,8 @@ TAP_TESTS_SH+= 11_test
TAP_TESTS_SH+= 12_test
TAP_TESTS_SH+= 13_test
+ATF_TESTS_SH+= sync_error
+
${PACKAGE}FILES+= conf.sh
.for t in ${TAP_TESTS_SH}
Copied: stable/11/tests/sys/geom/class/mirror/sync_error.sh (from r327780,
head/tests/sys/geom/class/mirror/sync_error.sh)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ stable/11/tests/sys/geom/class/mirror/sync_error.sh Wed Jan 24 15:16:17
2018 (r328334, copy of r327780,
head/tests/sys/geom/class/mirror/sync_error.sh)
@@ -0,0 +1,110 @@
+# $FreeBSD$
+
+REG_READ_FP=debug.fail_point.g_mirror_regular_request_read
+
+atf_test_case sync_read_error_2_disks cleanup
+sync_read_error_2_disks_head()
+{
+ atf_set "descr" \
+ "Ensure that we properly handle read errors during
synchronization."
+ atf_set "require.user" "root"
+}
+sync_read_error_2_disks_body()
+{
+ . $(atf_get_srcdir)/conf.sh
+
+ f1=$(mktemp ${base}.XXXXXX)
+ f2=$(mktemp ${base}.XXXXXX)
+
+ atf_check dd if=/dev/zero bs=1M count=32 of=$f1 status=none
+ atf_check truncate -s 32M $f2
+
+ md1=$(attach_md -t vnode -f ${f1})
+ md2=$(attach_md -t vnode -f ${f2})
+
+ atf_check gmirror label $name $md1
+ devwait
+
+ atf_check -s exit:0 -e empty -o not-empty sysctl
${REG_READ_FP}='1*return(5)'
+
+ # If a read error occurs while synchronizing and the mirror contains
+ # a single active disk, gmirror has no choice but to fail the
+ # synchronization and kick the new disk out of the mirror.
+ atf_check gmirror insert $name $md2
+ sleep 0.1
+ syncwait
+ atf_check [ $(gmirror status -s $name | wc -l) -eq 1 ]
+ atf_check -s exit:0 -o match:"DEGRADED $md1 \(ACTIVE\)" \
+ gmirror status -s $name
+}
+sync_read_error_2_disks_cleanup()
+{
+ . $(atf_get_srcdir)/conf.sh
+
+ atf_check -s exit:0 -e empty -o not-empty sysctl ${REG_READ_FP}='off'
+ gmirror_test_cleanup
+}
+
+atf_test_case sync_read_error_3_disks cleanup
+sync_read_error_3_disks_head()
+{
+ atf_set "descr" \
+ "Ensure that we properly handle read errors during
synchronization."
+ atf_set "require.user" "root"
+}
+sync_read_error_3_disks_body()
+{
+ . $(atf_get_srcdir)/conf.sh
+
+ f1=$(mktemp ${base}.XXXXXX)
+ f2=$(mktemp ${base}.XXXXXX)
+ f3=$(mktemp ${base}.XXXXXX)
+
+ atf_check dd if=/dev/random bs=1M count=32 of=$f1 status=none
+ atf_check truncate -s 32M $f2
+ atf_check truncate -s 32M $f3
+
+ md1=$(attach_md -t vnode -f ${f1})
+ md2=$(attach_md -t vnode -f ${f2})
+ md3=$(attach_md -t vnode -f ${f3})
+
+ atf_check gmirror label $name $md1
+ devwait
+
+ atf_check gmirror insert $name $md2
+ syncwait
+
+ atf_check -s exit:0 -e empty -o not-empty sysctl
${REG_READ_FP}='1*return(5)'
+
+ # If a read error occurs while synchronizing a new disk, and we have
+ # multiple active disks, we retry the read after an error. The disk
+ # which returned the read error is kicked out of the mirror.
+ atf_check gmirror insert $name $md3
+ syncwait
+ atf_check [ $(gmirror status -s $name | wc -l) -eq 2 ]
+ atf_check -s exit:0 -o match:"DEGRADED $md3 \(ACTIVE\)" \
+ gmirror status -s $name
+
+ # Make sure that the two active disks are identical. Destroy the
+ # mirror first so that the metadata sectors are wiped.
+ if $(gmirror status -s $name | grep -q $md1); then
+ active=$md1
+ else
+ active=$md2
+ fi
+ atf_check gmirror destroy $name
+ atf_check cmp /dev/$active /dev/$md3
+}
+sync_read_error_3_disks_cleanup()
+{
+ . $(atf_get_srcdir)/conf.sh
+
+ atf_check -s exit:0 -e empty -o not-empty sysctl ${REG_READ_FP}='off'
+ gmirror_test_cleanup
+}
+
+atf_init_test_cases()
+{
+ atf_add_test_case sync_read_error_2_disks
+ atf_add_test_case sync_read_error_3_disks
+}
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
