From 4ac37f8b2e6bdbfb3a0fd2ca56aedf05114719e8 Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Wed, 28 Apr 2021 01:48:11 +0100 Subject: [PATCH] vdev_mirror: don't scrub/resilver devices that can't be read This ensures that we don't accumulate checksum errors against offline or unavailable devices but, more importantly, means that we don't needlessly create DTL entries for offline devices that are already up-to-date. Consider a 3-way mirror, with disk A always online (and so always with an empty DTL) and B and C only occasionally online. When A & B resilver with C offline, B's DTL will effectively be appended to C's due to these spurious ZIOs even as the resilver empties B's DTL: * These ZIOs land in vdev_mirror_scrub_done() and flag an error * That flagged error causes vdev_mirror_io_done() to see unexpected_errors, so it issues a ZIO_TYPE_WRITE repair ZIO, which inherits ZIO_FLAG_SCAN_THREAD because zio_vdev_child_io() includes that flag in ZIO_VDEV_CHILD_FLAGS. * That ZIO fails, too, and eventually zio_done() gets its hands on it and calls vdev_stat_update(). * vdev_stat_update() sees the error and this zio... * is not speculative, * is not due to EIO (but rather ENXIO, since the device is closed) * has an ->io_vd != NULL (specifically, the offline leaf device) * is a write * is for a txg != 0 (but rather the read block's physical birth txg) * has ZIO_FLAG_SCAN_THREAD asserted * So: vdev_stat_update() calls vdev_dtl_dirty() on the offline vdev. Then, when A & C resilver with B offline, that story gets replayed and C's DTL will be appended to B's. In fact, one does not need this permanently-broken-mirror scenario to induce badness: breaking a mirror with no DTLs and then scrubbing will create DTLs for all offline devices. These DTLs will persist until the entire mirror is reassembled for the duration of the *resilver*, which, incidentally, will not consider the devices with good data to be sources of good data in the case of a read failure. Reviewed-by: Mark Maybee Reviewed-by: Brian Behlendorf Signed-off-by: Nathaniel Wesley Filardo Closes #11930 --- module/zfs/vdev_mirror.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/module/zfs/vdev_mirror.c b/module/zfs/vdev_mirror.c index f360a18c00..106678a870 100644 --- a/module/zfs/vdev_mirror.c +++ b/module/zfs/vdev_mirror.c @@ -649,6 +649,15 @@ vdev_mirror_io_start(zio_t *zio) */ for (c = 0; c < mm->mm_children; c++) { mc = &mm->mm_child[c]; + + /* Don't issue ZIOs to offline children */ + if (!vdev_mirror_child_readable(mc)) { + mc->mc_error = SET_ERROR(ENXIO); + mc->mc_tried = 1; + mc->mc_skipped = 1; + continue; + } + zio_nowait(zio_vdev_child_io(zio, zio->io_bp, mc->mc_vd, mc->mc_offset, abd_alloc_sametype(zio->io_abd,