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.

Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.

Author: amotin

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

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;
 			}

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;

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 .
@@ -1285,7 +1291,9 @@ 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).
+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

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,

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;
 }
 

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]

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 \

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 | \

tests/zfs-tests/tests/functional/alloc_class/alloc_class_011_neg.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 256 1025 33554432
+for value in 16777217 33554432 4294967296
 do
 	log_mustnot zfs set special_small_blocks=$value $TESTPOOL
 done