Avoid memory copies during mirror scrub

Issuing several scrub reads for a block we may use the parent ZIO
buffer for one of child ZIOs.  If that read complete successfully,
then we won't need to copy the data explicitly.  If block has only
one copy (typical for root vdev, which is also a mirror inside),
then we never need to copy -- succeed or fail as-is.  Previous
code also copied data from buffer of every successfully completed
child ZIO, but that just does not make any sense.

On healthy N-wide mirror this saves all N+1 (or even more in case
of ditto blocks) memory copies for each scrubbed block, allowing
CPU to focus mostly on check-summing.  For other vdev types it
should save one memory copy per block copy at root vdev.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Mark Maybee <mark.maybee@delphix.com>
Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Sponsored-By: iXsystems, Inc.
Closes #13606
This commit is contained in:
Alexander Motin 2022-07-05 19:26:20 -04:00 committed by GitHub
parent 6fca6195cd
commit 1ac7d194e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 61 additions and 70 deletions

View File

@ -436,36 +436,6 @@ vdev_mirror_child_done(zio_t *zio)
{ {
mirror_child_t *mc = zio->io_private; mirror_child_t *mc = zio->io_private;
/* See scrubbing read comment in vdev_mirror_io_start() */
if (zio->io_flags & ZIO_FLAG_SCRUB && zio->io_bp == NULL)
mc->mc_abd = zio->io_abd;
mc->mc_error = zio->io_error;
mc->mc_tried = 1;
mc->mc_skipped = 0;
}
static void
vdev_mirror_scrub_done(zio_t *zio)
{
mirror_child_t *mc = zio->io_private;
if (zio->io_error == 0) {
zio_t *pio;
zio_link_t *zl = NULL;
mutex_enter(&zio->io_lock);
while ((pio = zio_walk_parents(zio, &zl)) != NULL) {
mutex_enter(&pio->io_lock);
ASSERT3U(zio->io_size, >=, pio->io_size);
abd_copy(pio->io_abd, zio->io_abd, pio->io_size);
mutex_exit(&pio->io_lock);
}
mutex_exit(&zio->io_lock);
}
abd_free(zio->io_abd);
mc->mc_error = zio->io_error; mc->mc_error = zio->io_error;
mc->mc_tried = 1; mc->mc_tried = 1;
mc->mc_skipped = 0; mc->mc_skipped = 0;
@ -645,15 +615,13 @@ vdev_mirror_io_start(zio_t *zio)
if (zio->io_type == ZIO_TYPE_READ) { if (zio->io_type == ZIO_TYPE_READ) {
if ((zio->io_flags & ZIO_FLAG_SCRUB) && !mm->mm_resilvering) { if ((zio->io_flags & ZIO_FLAG_SCRUB) && !mm->mm_resilvering) {
/* /*
* For scrubbing reads we need to allocate a buffer * For scrubbing reads we need to issue reads to all
* for each child and issue reads to all children. * children. One child can reuse parent buffer, but
* If we can verify the checksum here, as indicated * for others we have to allocate separate ones to
* by io_bp being non-NULL, the data will be copied * verify checksums if io_bp is non-NULL, or compare
* into zio->io_data in vdev_mirror_scrub_done(). * them in vdev_mirror_io_done() otherwise.
* If not, then vdev_mirror_io_done() will compare
* all of the read buffers and return a checksum
* error if they aren't all identical.
*/ */
boolean_t first = B_TRUE;
for (c = 0; c < mm->mm_children; c++) { for (c = 0; c < mm->mm_children; c++) {
mc = &mm->mm_child[c]; mc = &mm->mm_child[c];
@ -665,13 +633,15 @@ vdev_mirror_io_start(zio_t *zio)
continue; continue;
} }
zio_nowait(zio_vdev_child_io(zio, zio->io_bp, mc->mc_abd = first ? zio->io_abd :
mc->mc_vd, mc->mc_offset,
abd_alloc_sametype(zio->io_abd, abd_alloc_sametype(zio->io_abd,
zio->io_size), zio->io_size, zio->io_size);
zio->io_type, zio->io_priority, 0, zio_nowait(zio_vdev_child_io(zio, zio->io_bp,
zio->io_bp ? vdev_mirror_scrub_done : mc->mc_vd, mc->mc_offset, mc->mc_abd,
zio->io_size, zio->io_type,
zio->io_priority, 0,
vdev_mirror_child_done, mc)); vdev_mirror_child_done, mc));
first = B_FALSE;
} }
zio_execute(zio); zio_execute(zio);
return; return;
@ -802,30 +772,30 @@ vdev_mirror_io_done(zio_t *zio)
return; return;
} }
/* if (zio->io_flags & ZIO_FLAG_SCRUB && !mm->mm_resilvering) {
* If we're scrubbing but don't have a BP available (because this abd_t *best_abd = NULL;
* vdev is under a raidz or draid vdev) then the best we can do is if (last_good_copy >= 0)
* compare all of the copies read. If they're not identical then best_abd = mm->mm_child[last_good_copy].mc_abd;
* return a checksum error and the most likely correct data. The
* raidz code will issue a repair I/O if possible.
*/
if (zio->io_flags & ZIO_FLAG_SCRUB && zio->io_bp == NULL) {
abd_t *last_good_abd;
ASSERT(zio->io_vd->vdev_ops == &vdev_replacing_ops || /*
zio->io_vd->vdev_ops == &vdev_spare_ops); * If we're scrubbing but don't have a BP available (because
* this vdev is under a raidz or draid vdev) then the best we
if (good_copies > 1) { * can do is compare all of the copies read. If they're not
last_good_abd = mm->mm_child[last_good_copy].mc_abd; * identical then return a checksum error and the most likely
abd_t *best_abd = NULL; * correct data. The raidz code will issue a repair I/O if
* possible.
*/
if (zio->io_bp == NULL) {
ASSERT(zio->io_vd->vdev_ops == &vdev_replacing_ops ||
zio->io_vd->vdev_ops == &vdev_spare_ops);
abd_t *pref_abd = NULL;
for (c = 0; c < last_good_copy; c++) { for (c = 0; c < last_good_copy; c++) {
mc = &mm->mm_child[c]; mc = &mm->mm_child[c];
if (mc->mc_error || !mc->mc_tried) if (mc->mc_error || !mc->mc_tried)
continue; continue;
if (abd_cmp(mc->mc_abd, last_good_abd) != 0) if (abd_cmp(mc->mc_abd, best_abd) != 0)
zio->io_error = SET_ERROR(ECKSUM); zio->io_error = SET_ERROR(ECKSUM);
/* /*
@ -833,24 +803,45 @@ vdev_mirror_io_done(zio_t *zio)
* by vdev_mirror_child_select() so it's * by vdev_mirror_child_select() so it's
* considered to be the best candidate. * considered to be the best candidate.
*/ */
if (best_abd == NULL && if (pref_abd == NULL &&
mc->mc_vd->vdev_ops == mc->mc_vd->vdev_ops ==
&vdev_draid_spare_ops) { &vdev_draid_spare_ops)
pref_abd = mc->mc_abd;
/*
* In the absence of a preferred copy, use
* the parent pointer to avoid a memory copy.
*/
if (mc->mc_abd == zio->io_abd)
best_abd = mc->mc_abd; best_abd = mc->mc_abd;
}
if (pref_abd)
best_abd = pref_abd;
} else {
/*
* If we have a BP available, then checksums are
* already verified and we just need a buffer
* with valid data, preferring parent one to
* avoid a memory copy.
*/
for (c = 0; c < last_good_copy; c++) {
mc = &mm->mm_child[c];
if (mc->mc_error || !mc->mc_tried)
continue;
if (mc->mc_abd == zio->io_abd) {
best_abd = mc->mc_abd;
break;
} }
} }
abd_copy(zio->io_abd, best_abd ? best_abd :
last_good_abd, zio->io_size);
} else if (good_copies == 1) {
last_good_abd = mm->mm_child[last_good_copy].mc_abd;
abd_copy(zio->io_abd, last_good_abd, zio->io_size);
} }
if (best_abd && best_abd != zio->io_abd)
abd_copy(zio->io_abd, best_abd, zio->io_size);
for (c = 0; c < mm->mm_children; c++) { for (c = 0; c < mm->mm_children; c++) {
mc = &mm->mm_child[c]; mc = &mm->mm_child[c];
abd_free(mc->mc_abd); if (mc->mc_abd != zio->io_abd)
abd_free(mc->mc_abd);
mc->mc_abd = NULL; mc->mc_abd = NULL;
} }
} }