From cb3b0ddb49fa3de61f44ff829f280553db22c0be Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Mon, 1 Jul 2024 11:19:16 +1000 Subject: [PATCH 1/2] flush: don't report flush error when disabling flush support The first time a device returns ENOTSUP in repsonse to a flush request, we set vdev_nowritecache so we don't issue flushes in the future and instead just pretend the succeeded. However, we still return an error for the initial flush, even though we just decided such errors are meaningless! So, when setting vdev_nowritecache in response to a flush error, also reset the error code to assume success. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris --- module/zfs/zio.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/module/zfs/zio.c b/module/zfs/zio.c index f4d7e57542a1..f13228051dec 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -4608,11 +4608,14 @@ zio_vdev_io_assess(zio_t *zio) /* * If a cache flush returns ENOTSUP or ENOTTY, we know that no future * attempts will ever succeed. In this case we set a persistent - * boolean flag so that we don't bother with it in the future. + * boolean flag so that we don't bother with it in the future, and + * then we act like the flush succeeded. */ if ((zio->io_error == ENOTSUP || zio->io_error == ENOTTY) && - zio->io_type == ZIO_TYPE_FLUSH && vd != NULL) + zio->io_type == ZIO_TYPE_FLUSH && vd != NULL) { vd->vdev_nowritecache = B_TRUE; + zio->io_error = 0; + } if (zio->io_error) zio->io_pipeline = ZIO_INTERLOCK_PIPELINE; From 96adb7e1606c538f183c6f1675746b49e656ec38 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Mon, 1 Jul 2024 11:19:16 +1000 Subject: [PATCH 2/2] flush: only detect lack of flush support in one place It seems there's no good reason for vdev_disk & vdev_geom to explicitly detect no support for flush and set vdev_nowritecache. Instead, just signal it by setting the error to ENOTSUP, and let zio_vdev_io_assess() take care of it in one place. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris --- module/os/freebsd/zfs/vdev_geom.c | 15 --------------- module/os/linux/zfs/vdev_disk.c | 5 ++--- module/zfs/zio.c | 6 +++--- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/module/os/freebsd/zfs/vdev_geom.c b/module/os/freebsd/zfs/vdev_geom.c index b7ff1063b089..7aaa42bfb1a8 100644 --- a/module/os/freebsd/zfs/vdev_geom.c +++ b/module/os/freebsd/zfs/vdev_geom.c @@ -1014,21 +1014,6 @@ vdev_geom_io_intr(struct bio *bp) zio->io_error = SET_ERROR(EIO); switch (zio->io_error) { - case ENOTSUP: - /* - * If we get ENOTSUP for BIO_FLUSH or BIO_DELETE we know - * that future attempts will never succeed. In this case - * we set a persistent flag so that we don't bother with - * requests in the future. - */ - switch (bp->bio_cmd) { - case BIO_FLUSH: - vd->vdev_nowritecache = B_TRUE; - break; - case BIO_DELETE: - break; - } - break; case ENXIO: if (!vd->vdev_remove_wanted) { /* diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index 6a66a72b91a9..e8bd513e6909 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1198,9 +1198,8 @@ vdev_disk_io_flush_completion(struct bio *bio) { zio_t *zio = bio->bi_private; zio->io_error = bi_status_to_errno(bio->bi_status); - - if (zio->io_error && (zio->io_error == EOPNOTSUPP)) - zio->io_vd->vdev_nowritecache = B_TRUE; + if (zio->io_error == EOPNOTSUPP || zio->io_error == ENOTTY) + zio->io_error = SET_ERROR(ENOTSUP); bio_put(bio); ASSERT3S(zio->io_error, >=, 0); diff --git a/module/zfs/zio.c b/module/zfs/zio.c index f13228051dec..bd6752f00ac5 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -4606,13 +4606,13 @@ zio_vdev_io_assess(zio_t *zio) } /* - * If a cache flush returns ENOTSUP or ENOTTY, we know that no future + * If a cache flush returns ENOTSUP we know that no future * attempts will ever succeed. In this case we set a persistent * boolean flag so that we don't bother with it in the future, and * then we act like the flush succeeded. */ - if ((zio->io_error == ENOTSUP || zio->io_error == ENOTTY) && - zio->io_type == ZIO_TYPE_FLUSH && vd != NULL) { + if (zio->io_error == ENOTSUP && zio->io_type == ZIO_TYPE_FLUSH && + vd != NULL) { vd->vdev_nowritecache = B_TRUE; zio->io_error = 0; }