From 4e92aee23395a23efc3726b7377af0ce9b95c829 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Wed, 2 Jul 2025 14:11:37 -0400 Subject: [PATCH] Relax special_small_blocks restrictions special_small_blocks is applied to blocks after compression, so it makes no sense to demand its values to be power of 2. At most they could be multiple of 512, but that would still buy us nothing, so lets allow them be any within SPA_MAXBLOCKSIZE. Also special_small_blocks does not really need to depend on the set recordsize, enabled pool features or presence of special vdev. At worst in any of those cases it will just do nothing, so we should not complicate users lives by artificial limitations. While there, polish comments for recordsize and volblocksize. Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #17497 --- include/sys/dmu_objset.h | 2 +- lib/libzfs/libzfs_dataset.c | 37 ++------------- lib/libzfs/libzfs_pool.c | 34 -------------- man/man7/zfsprops.7 | 29 ++++++------ module/zcommon/zfs_prop.c | 11 ++--- module/zfs/dmu_objset.c | 6 --- module/zfs/zfs_ioctl.c | 9 ---- tests/runfiles/common.run | 3 +- tests/zfs-tests/tests/Makefile.am | 2 - .../alloc_class/alloc_class_010_pos.ksh | 2 +- .../alloc_class/alloc_class_011_neg.ksh | 4 +- .../alloc_class/alloc_class_014_neg.ksh | 39 ---------------- .../alloc_class/alloc_class_015_pos.ksh | 46 ------------------- 13 files changed, 31 insertions(+), 193 deletions(-) delete mode 100755 tests/zfs-tests/tests/functional/alloc_class/alloc_class_014_neg.ksh delete mode 100755 tests/zfs-tests/tests/functional/alloc_class/alloc_class_015_pos.ksh diff --git a/include/sys/dmu_objset.h b/include/sys/dmu_objset.h index 288ad3016..492be2920 100644 --- a/include/sys/dmu_objset.h +++ b/include/sys/dmu_objset.h @@ -152,7 +152,7 @@ struct objset { * The largest zpl file block allowed in special class. * cached here instead of zfsvfs for easier access. */ - int os_zpl_special_smallblock; + uint64_t os_zpl_special_smallblock; /* * Pointer is constant; the blkptr it points to is protected by diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 91560b40b..e1b91fc47 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -1039,7 +1039,6 @@ zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl, nvlist_t *ret; int chosen_normal = -1; int chosen_utf = -1; - int set_maxbs = 0; if (nvlist_alloc(&ret, NV_UNIQUE_NAME, 0) != 0) { (void) no_memory(hdl); @@ -1258,46 +1257,20 @@ zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl, (void) zfs_error(hdl, EZFS_BADPROP, errbuf); goto error; } - /* save the ZFS_PROP_RECORDSIZE during create op */ - if (zpool_hdl == NULL && prop == ZFS_PROP_RECORDSIZE) { - set_maxbs = intval; - } break; } case ZFS_PROP_SPECIAL_SMALL_BLOCKS: { - int maxbs = - set_maxbs == 0 ? SPA_OLD_MAXBLOCKSIZE : set_maxbs; + int maxbs = SPA_MAXBLOCKSIZE; char buf[64]; - if (zpool_hdl != NULL) { - char state[64] = ""; - - maxbs = zpool_get_prop_int(zpool_hdl, - ZPOOL_PROP_MAXBLOCKSIZE, NULL); - - /* - * Issue a warning but do not fail so that - * tests for settable properties succeed. - */ - if (zpool_prop_get_feature(zpool_hdl, - "feature@allocation_classes", state, - sizeof (state)) != 0 || - strcmp(state, ZFS_FEATURE_ACTIVE) != 0) { - (void) fprintf(stderr, gettext( - "%s: property requires a special " - "device in the pool\n"), propname); - } - } - if (intval != 0 && - (intval < SPA_MINBLOCKSIZE || - intval > maxbs || !ISP2(intval))) { + if (intval > SPA_MAXBLOCKSIZE) { zfs_nicebytes(maxbs, buf, sizeof (buf)); zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "invalid '%s=%llu' property: must be zero " - "or a power of 2 from 512B to %s"), - propname, (unsigned long long)intval, buf); + "invalid '%s' property: must be between " + "zero and %s"), + propname, buf); (void) zfs_error(hdl, EZFS_BADPROP, errbuf); goto error; } diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index 6f8fb994f..fb18c4309 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -1421,30 +1421,6 @@ zpool_get_state(zpool_handle_t *zhp) return (zhp->zpool_state); } -/* - * Check if vdev list contains a special vdev - */ -static boolean_t -zpool_has_special_vdev(nvlist_t *nvroot) -{ - nvlist_t **child; - uint_t children; - - if (nvlist_lookup_nvlist_array(nvroot, ZPOOL_CONFIG_CHILDREN, &child, - &children) == 0) { - for (uint_t c = 0; c < children; c++) { - const char *bias; - - if (nvlist_lookup_string(child[c], - ZPOOL_CONFIG_ALLOCATION_BIAS, &bias) == 0 && - strcmp(bias, VDEV_ALLOC_BIAS_SPECIAL) == 0) { - return (B_TRUE); - } - } - } - return (B_FALSE); -} - /* * Check if vdev list contains a dRAID vdev */ @@ -1548,16 +1524,6 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot, goto create_failed; } - if (nvlist_exists(zc_fsprops, - zfs_prop_to_name(ZFS_PROP_SPECIAL_SMALL_BLOCKS)) && - !zpool_has_special_vdev(nvroot)) { - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "%s property requires a special vdev"), - zfs_prop_to_name(ZFS_PROP_SPECIAL_SMALL_BLOCKS)); - (void) zfs_error(hdl, EZFS_BADPROP, errbuf); - goto create_failed; - } - if (!zc_props && (nvlist_alloc(&zc_props, NV_UNIQUE_NAME, 0) != 0)) { goto create_failed; diff --git a/man/man7/zfsprops.7 b/man/man7/zfsprops.7 index ac58203f0..829eb4205 100644 --- a/man/man7/zfsprops.7 +++ b/man/man7/zfsprops.7 @@ -541,10 +541,16 @@ The .Sy blocksize cannot be changed once the volume has been written, so it should be set at volume creation time. -The default -.Sy blocksize -for volumes is 16 KiB. -Any power of 2 from 512 bytes to 128 KiB is valid. +The size specified must be a power of two greater than or equal to +.Ar 512 B +and less than or equal to +.Ar 128 KiB . +If the +.Sy large_blocks +feature is enabled on the pool, the size may be up to +.Ar 16 MiB . +The default size is +.Ar 16 KiB . .Pp This property can also be referred to by its shortened column name, .Sy volblock . @@ -1282,10 +1288,12 @@ This feature must be enabled to be used .It Sy special_small_blocks Ns = Ns Ar size This value represents the threshold block size for including small file or zvol blocks into the special allocation class. -Blocks smaller than or equal to this -value will be assigned to the special allocation class while greater blocks -will be assigned to the regular class. -Valid values are zero or a power of two from 512 up to 1048576 (1 MiB). +Blocks smaller than or equal to this value after compression and encryption +will be assigned to the special allocation class, while greater blocks will +be assigned to the regular class. +Valid values are from 0 to maximum block size ( +.Ar 16 MiB +). The default size is 0 which means no small file or zvol blocks will be allocated in the special class. .Pp @@ -1569,11 +1577,6 @@ See .Xr zpool-features 7 for details on ZFS feature flags. .Pp -However, blocks larger than -.Ar 1 MiB -can have an impact on i/o latency (e.g. tying up a spinning disk for -~300ms), and also potentially on the memory allocator. -.Pp Note that maximum size is still limited by default to .Ar 1 MiB on x86_32, see diff --git a/module/zcommon/zfs_prop.c b/module/zcommon/zfs_prop.c index 8b4c42517..864e3898b 100644 --- a/module/zcommon/zfs_prop.c +++ b/module/zcommon/zfs_prop.c @@ -640,7 +640,7 @@ zfs_prop_init(void) "<1.00x or higher if compressed>", "REFRATIO", B_FALSE, sfeatures); zprop_register_number(ZFS_PROP_VOLBLOCKSIZE, "volblocksize", ZVOL_DEFAULT_BLOCKSIZE, PROP_ONETIME, - ZFS_TYPE_VOLUME, "512 to 128k, power of 2", "VOLBLOCK", B_FALSE, + ZFS_TYPE_VOLUME, "512 to 16M, power of 2", "VOLBLOCK", B_FALSE, sfeatures); zprop_register_index(ZFS_PROP_VOLTHREADING, "volthreading", 1, PROP_DEFAULT, ZFS_TYPE_VOLUME, "on | off", "zvol threading", @@ -734,13 +734,12 @@ zfs_prop_init(void) /* inherit number properties */ zprop_register_number(ZFS_PROP_RECORDSIZE, "recordsize", SPA_OLD_MAXBLOCKSIZE, PROP_INHERIT, - ZFS_TYPE_FILESYSTEM, "512 to 1M, power of 2", "RECSIZE", B_FALSE, - sfeatures); + ZFS_TYPE_FILESYSTEM, "512 to 16M, power of 2", + "RECSIZE", B_FALSE, sfeatures); zprop_register_number(ZFS_PROP_SPECIAL_SMALL_BLOCKS, "special_small_blocks", 0, PROP_INHERIT, - ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME, - "zero or 512 to 1M, power of 2", "SPECIAL_SMALL_BLOCKS", B_FALSE, - sfeatures); + ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME, "0 to 16M", + "SPECIAL_SMALL_BLOCKS", B_FALSE, sfeatures); /* hidden properties */ zprop_register_hidden(ZFS_PROP_NUMCLONES, "numclones", PROP_TYPE_NUMBER, diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index b3f792e4a..c1101088b 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -345,12 +345,6 @@ smallblk_changed_cb(void *arg, uint64_t newval) { objset_t *os = arg; - /* - * Inheritance and range checking should have been done by now. - */ - ASSERT(newval <= SPA_MAXBLOCKSIZE); - ASSERT(ISP2(newval)); - os->os_zpl_special_smallblock = newval; } diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index ebb1cfd07..3a413f4a7 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -5000,15 +5000,6 @@ zfs_check_settable(const char *dsname, nvpair_t *pair, cred_t *cr) } break; - case ZFS_PROP_SPECIAL_SMALL_BLOCKS: - /* - * This property could require the allocation classes - * feature to be active for setting, however we allow - * it so that tests of settable properties succeed. - * The CLI will issue a warning in this case. - */ - break; - case ZFS_PROP_SHARESMB: if (zpl_earlier_version(dsname, ZPL_VERSION_FUID)) return (SET_ERROR(ENOTSUP)); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 7969945a4..376518e9f 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -37,8 +37,7 @@ tests = ['alloc_class_001_pos', 'alloc_class_002_neg', 'alloc_class_003_pos', 'alloc_class_004_pos', 'alloc_class_005_pos', 'alloc_class_006_pos', 'alloc_class_007_pos', 'alloc_class_008_pos', 'alloc_class_009_pos', 'alloc_class_010_pos', 'alloc_class_011_neg', 'alloc_class_012_pos', - 'alloc_class_013_pos', 'alloc_class_014_neg', 'alloc_class_015_pos', - 'alloc_class_016_pos'] + 'alloc_class_013_pos', 'alloc_class_016_pos'] tags = ['functional', 'alloc_class'] [tests/functional/append] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index f854070ab..20a17a531 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -429,8 +429,6 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/alloc_class/alloc_class_011_neg.ksh \ functional/alloc_class/alloc_class_012_pos.ksh \ functional/alloc_class/alloc_class_013_pos.ksh \ - functional/alloc_class/alloc_class_014_neg.ksh \ - functional/alloc_class/alloc_class_015_pos.ksh \ functional/alloc_class/alloc_class_016_pos.ksh \ functional/alloc_class/cleanup.ksh \ functional/alloc_class/setup.ksh \ diff --git a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_010_pos.ksh b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_010_pos.ksh index 7f9d108ed..f7dfd42b0 100755 --- a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_010_pos.ksh +++ b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_010_pos.ksh @@ -36,7 +36,7 @@ log_must disk_setup log_must zpool create $TESTPOOL raidz $ZPOOL_DISKS special mirror \ $CLASS_DISK0 $CLASS_DISK1 -for value in 0 512 1024 2048 4096 8192 16384 32768 65536 131072 +for value in 0 200 512 1300 4096 12345 131072 1572864 16777216 do log_must zfs set special_small_blocks=$value $TESTPOOL ACTUAL=$(zfs get -p special_small_blocks $TESTPOOL | \ diff --git a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_011_neg.ksh b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_011_neg.ksh index a04e9ca43..0f9011754 100755 --- a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_011_neg.ksh +++ b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_011_neg.ksh @@ -22,7 +22,7 @@ # # DESCRIPTION: # Setting the special_small_blocks property to invalid values fails. -# Powers of two from 512 to 1M are allowed. +# Only values between 0 and 16M including are allowed. # verify_runnable "global" @@ -36,7 +36,7 @@ log_must disk_setup log_must zpool create $TESTPOOL raidz $ZPOOL_DISKS special mirror \ $CLASS_DISK0 $CLASS_DISK1 -for value in 256 1025 33554432 +for value in 16777217 33554432 4294967296 do log_mustnot zfs set special_small_blocks=$value $TESTPOOL done diff --git a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_014_neg.ksh b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_014_neg.ksh deleted file mode 100755 index e16b64a96..000000000 --- a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_014_neg.ksh +++ /dev/null @@ -1,39 +0,0 @@ -#!/bin/ksh -p -# SPDX-License-Identifier: CDDL-1.0 - -# -# This file and its contents are supplied under the terms of the -# Common Development and Distribution License ("CDDL"), version 1.0. -# You may only use this file in accordance with the terms of version -# 1.0 of the CDDL. -# -# A full copy of the text of the CDDL should have accompanied this -# source. A copy of the CDDL is also available via the Internet at -# http://www.illumos.org/license/CDDL. -# - -. $STF_SUITE/tests/functional/alloc_class/alloc_class.kshlib - -# -# DESCRIPTION: -# Setting the special_small_blocks property greater than recordsize fails. -# - -verify_runnable "global" - -claim="Setting the special_small_blocks property greater than recordsize fails" - -log_assert $claim -log_onexit cleanup -log_must disk_setup - -for size in 512 4096 32768 131072 524288 1048576 -do - let bigger=$size*2 - log_mustnot zpool create -O recordsize=$size \ - -O special_small_blocks=$bigger \ - $TESTPOOL raidz $ZPOOL_DISKS special mirror \ - $CLASS_DISK0 $CLASS_DISK1 -done - -log_pass $claim diff --git a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_015_pos.ksh b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_015_pos.ksh deleted file mode 100755 index 9d34375b7..000000000 --- a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_015_pos.ksh +++ /dev/null @@ -1,46 +0,0 @@ -#!/bin/ksh -p -# SPDX-License-Identifier: CDDL-1.0 - -# -# This file and its contents are supplied under the terms of the -# Common Development and Distribution License ("CDDL"), version 1.0. -# You may only use this file in accordance with the terms of version -# 1.0 of the CDDL. -# -# A full copy of the text of the CDDL should have accompanied this -# source. A copy of the CDDL is also available via the Internet at -# http://www.illumos.org/license/CDDL. -# - -. $STF_SUITE/tests/functional/alloc_class/alloc_class.kshlib - -# -# DESCRIPTION: -# Can set special_small_blocks property less than or equal to recordsize. -# - -verify_runnable "global" - -claim="Can set special_small_blocks property less than or equal to recordsize" - -log_assert $claim -log_onexit cleanup -log_must disk_setup - -for size in 8192 32768 131072 524288 1048576 -do - let smaller=$size/2 - log_must zpool create -O recordsize=$size \ - -O special_small_blocks=$smaller \ - $TESTPOOL raidz $ZPOOL_DISKS special mirror \ - $CLASS_DISK0 $CLASS_DISK1 - log_must zpool destroy -f "$TESTPOOL" - - log_must zpool create -O recordsize=$size \ - -O special_small_blocks=$size \ - $TESTPOOL raidz $ZPOOL_DISKS special mirror \ - $CLASS_DISK0 $CLASS_DISK1 - log_must zpool destroy -f "$TESTPOOL" -done - -log_pass $claim