From af04e9a942d31e6b3eb1242308a414d5decde638 Mon Sep 17 00:00:00 2001 From: MigeljanImeri Date: Thu, 25 Jan 2024 09:43:16 -0700 Subject: [PATCH] Add vdev property to toggle adding io to vdev queue Added vdev property to toggle queueing io to vdev queue when reading / writing from / to vdev. The intention behind this property is to improve performance when using o_direct. Signed-off-by: MigeljanImeri --- include/sys/fs/zfs.h | 1 + include/sys/vdev_impl.h | 1 + lib/libzfs/libzfs.abi | 3 +- man/man7/vdevprops.7 | 2 + module/zcommon/zpool_prop.c | 3 + module/zfs/vdev.c | 12 +++ module/zfs/vdev_queue.c | 13 ++- tests/runfiles/common.run | 2 +- tests/zfs-tests/tests/Makefile.am | 1 + .../cli_root/zpool_get/vdev_get.cfg | 1 + .../cli_root/zpool_set/vdev_set_queue_io.ksh | 87 +++++++++++++++++++ 11 files changed, 122 insertions(+), 4 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zpool_set/vdev_set_queue_io.ksh diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 1676020d04d3..89ee30014c6c 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -378,6 +378,7 @@ typedef enum { VDEV_PROP_TRIM_SUPPORT, VDEV_PROP_TRIM_ERRORS, VDEV_PROP_SLOW_IOS, + VDEV_PROP_QUEUE_IO, VDEV_NUM_PROPS } vdev_prop_t; diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index abd66b8abc96..dbb98680e1bb 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -467,6 +467,7 @@ struct vdev { uint64_t vdev_io_t; uint64_t vdev_slow_io_n; uint64_t vdev_slow_io_t; + uint64_t vdev_queue_io; }; #define VDEV_PAD_SIZE (8 << 10) diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 1a96460c2b84..090e3b9b18d8 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -5916,7 +5916,8 @@ - + + diff --git a/man/man7/vdevprops.7 b/man/man7/vdevprops.7 index 34d4026b1009..f12e27e71762 100644 --- a/man/man7/vdevprops.7 +++ b/man/man7/vdevprops.7 @@ -156,6 +156,8 @@ If this device should perform new allocations, used to disable a device when it is scheduled for later removal. See .Xr zpool-remove 8 . +.It Sy queue_io +Add io to the vdev queue when reading or writing to this vdev. .El .Ss User Properties In addition to the standard native properties, ZFS supports arbitrary user diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index d3355730ba3d..c8831753dff9 100644 --- a/module/zcommon/zpool_prop.c +++ b/module/zcommon/zpool_prop.c @@ -466,6 +466,9 @@ vdev_prop_init(void) zprop_register_index(VDEV_PROP_TRIM_SUPPORT, "trim_support", 0, PROP_READONLY, ZFS_TYPE_VDEV, "on | off", "TRIMSUP", boolean_table, sfeatures); + zprop_register_index(VDEV_PROP_QUEUE_IO, "queue_io", 1, + PROP_DEFAULT, ZFS_TYPE_VDEV, "on | off", "QUEUE_IO", + boolean_table, sfeatures); /* default index properties */ zprop_register_index(VDEV_PROP_FAILFAST, "failfast", B_TRUE, diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index bcab46c63bfa..05bffc9fd585 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -704,6 +704,8 @@ vdev_alloc_common(spa_t *spa, uint_t id, uint64_t guid, vdev_ops_t *ops) vd->vdev_slow_io_n = vdev_prop_default_numeric(VDEV_PROP_SLOW_IO_N); vd->vdev_slow_io_t = vdev_prop_default_numeric(VDEV_PROP_SLOW_IO_T); + vd->vdev_queue_io = vdev_prop_default_numeric(VDEV_PROP_QUEUE_IO); + list_link_init(&vd->vdev_config_dirty_node); list_link_init(&vd->vdev_state_dirty_node); list_link_init(&vd->vdev_initialize_node); @@ -6052,6 +6054,15 @@ vdev_prop_set(vdev_t *vd, nvlist_t *innvl, nvlist_t *outnvl) } vd->vdev_slow_io_t = intval; break; + case VDEV_PROP_QUEUE_IO: + if (nvpair_value_uint64(elem, &intval) != 0) { + error = EINVAL; + break; + } + if (vd->vdev_ops->vdev_op_leaf) { + vd->vdev_queue_io = intval & 1; + } + break; default: /* Most processing is done in vdev_props_set_sync */ break; @@ -6415,6 +6426,7 @@ vdev_prop_get(vdev_t *vd, nvlist_t *innvl, nvlist_t *outnvl) case VDEV_PROP_IO_T: case VDEV_PROP_SLOW_IO_N: case VDEV_PROP_SLOW_IO_T: + case VDEV_PROP_QUEUE_IO: err = vdev_prop_get_int(vd, prop, &intval); if (err && err != ENOENT) break; diff --git a/module/zfs/vdev_queue.c b/module/zfs/vdev_queue.c index 092b3f375be0..ea752260bbf9 100644 --- a/module/zfs/vdev_queue.c +++ b/module/zfs/vdev_queue.c @@ -946,6 +946,11 @@ vdev_queue_io(zio_t *zio) zio->io_flags |= ZIO_FLAG_DONT_QUEUE; zio->io_timestamp = gethrtime(); + if (!zio->io_vd->vdev_queue_io) { + zio->io_queue_state = ZIO_QS_NONE; + return (zio); + } + mutex_enter(&vq->vq_lock); vdev_queue_io_add(vq, zio); nio = vdev_queue_io_to_issue(vq); @@ -978,8 +983,12 @@ vdev_queue_io_done(zio_t *zio) vq->vq_io_complete_ts = now; vq->vq_io_delta_ts = zio->io_delta = now - zio->io_timestamp; - mutex_enter(&vq->vq_lock); - vdev_queue_pending_remove(vq, zio); + if (zio->io_queue_state == ZIO_QS_ACTIVE) { + mutex_enter(&vq->vq_lock); + vdev_queue_pending_remove(vq, zio); + } + else + return; while ((nio = vdev_queue_io_to_issue(vq)) != NULL) { mutex_exit(&vq->vq_lock); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index f89a4b3e0aae..189c3fc8af3d 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -544,7 +544,7 @@ tags = ['functional', 'cli_root', 'zpool_scrub'] [tests/functional/cli_root/zpool_set] tests = ['zpool_set_001_pos', 'zpool_set_002_neg', 'zpool_set_003_neg', 'zpool_set_ashift', 'zpool_set_features', 'vdev_set_001_pos', - 'user_property_001_pos', 'user_property_002_neg'] + 'user_property_001_pos', 'user_property_002_neg', 'vdev_set_queue_io'] tags = ['functional', 'cli_root', 'zpool_set'] [tests/functional/cli_root/zpool_split] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 206ee8ac1542..3d4098e87cc8 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -1232,6 +1232,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zpool_set/setup.ksh \ functional/cli_root/zpool/setup.ksh \ functional/cli_root/zpool_set/vdev_set_001_pos.ksh \ + functional/cli_root/zpool_set/vdev_set_queue_io.ksh \ functional/cli_root/zpool_set/zpool_set_common.kshlib \ functional/cli_root/zpool_set/zpool_set_001_pos.ksh \ functional/cli_root/zpool_set/zpool_set_002_neg.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_get/vdev_get.cfg b/tests/zfs-tests/tests/functional/cli_root/zpool_get/vdev_get.cfg index 6cfa7eaf7514..0650cebb6b4a 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_get/vdev_get.cfg +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_get/vdev_get.cfg @@ -75,4 +75,5 @@ typeset -a properties=( trim_support trim_errors slow_ios + queue_io ) diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_set/vdev_set_queue_io.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_set/vdev_set_queue_io.ksh new file mode 100755 index 000000000000..658d1044f918 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_set/vdev_set_queue_io.ksh @@ -0,0 +1,87 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or https://opensource.org/licenses/CDDL-1.0. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2024 by Triad National Security, LLC. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# Toggling vdev queue_io property while reading from vdev should not cause panic. +# +# STRATEGY: +# 1. Create a zpool +# 2. Write a file to the pool. +# 3. Start reading from file, while also toggling the queue_io property on / off. +# + +verify_runnable "global" + +command -v fio > /dev/null || log_unsupported "fio missing" + +function toggle_queue_io +{ + zpool set queue_io=off $TESTPOOL1 $FILEDEV + sleep 0.1 + zpool set queue_io=on $TESTPOOL1 $FILEDEV + sleep 0.1 +} + +function cleanup +{ + log_must destroy_pool $TESTPOOL1 + rm -f $FILEDEV +} + +log_assert "Toggling vdev queue_io property while reading from vdev should not cause panic" +log_onexit cleanup + +# 1. Create a pool + +FILEDEV="$TEST_BASE_DIR/filedev.$$" +log_must truncate -s $(($MINVDEVSIZE * 2)) $FILEDEV +log_must create_pool $TESTPOOL1 $FILEDEV + +mntpnt=$(get_prop mountpoint $TESTPOOL1) + +# 2. Write a file to the pool + +log_must eval "fio --filename=$mntpnt/foobar --name=write-file \ + --rw=write --size=$MINVDEVSIZE --bs=128k --numjobs=1 --direct=1 \ + --ioengine=sync --runtime=10" + +# 3. Starting reading from file, while also toggling the queue_io property on / off. + +log_must eval "fio --filename=$mntpnt/foobar --name=read-file \ + --rw=read --size=$MINVDEVSIZE --bs=128k --numjobs=1 --direct=1 \ + --ioengine=sync --time_based --runtime=10 &" + +ITERATIONS=30 + +for i in $(seq $ITERATIONS); do + log_must toggle_queue_io +done; +wait + +log_pass "Toggling vdev queue_io property while reading from vdev does not cause panic"