Pool allocation classes misplacing small file blocks

Due to an off-by-one condition in spa_preferred_class() we are picking
the "normal" allocation class instead of the "special" one for file
blocks with size equal to the special_small_blocks property value.

This change fix the small code issue, update the ZFS Test Suite and the
zfs(8) man page.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Don Brady <don.brady@delphix.com>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #8351
Closes #8361

diff --git a/man/man8/zfs.8 b/man/man8/zfs.8
index be28f2b62a86bdeae9e698b30ae2b2b07a339a49..4658c5b8b33826fe626ae919926ce34e104c8d57 100644 (file)
--- a/man/man8/zfs.8
+++ b/man/man8/zfs.8
@@ -1521,9 +1521,11 @@ This feature must be enabled to be used
 .Pc .
 .It Sy special_small_blocks Ns = Ns Em size
 This value represents the threshold block size for including small file
-blocks into the special allocation class. Valid values are zero or a
-power of two from 512B up to 128K. The default size is 0 which means no
-small file blocks will be allocated in the special class.
+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 512B up to 128K. The default size is 0 which means no small file blocks
+will be allocated in the special class.
 .Pp
 Before setting this property, a special class vdev must be added to the
 pool. See

diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c
index 877f312b1b3e7ebde4b3af03d5e40e34f2b9929c..0976cc49c305478b9f032ebc4f6141a278f97414 100644 (file)
--- a/module/zfs/spa_misc.c
+++ b/module/zfs/spa_misc.c
@@ -1851,7 +1851,7 @@ spa_preferred_class(spa_t *spa, uint64_t size, dmu_object_type_t objtype,
         * zfs_special_class_metadata_reserve_pct exclusively for metadata.
         */
        if (DMU_OT_IS_FILE(objtype) &&
-           has_special_class && size < special_smallblk) {
+           has_special_class && size <= special_smallblk) {
                metaslab_class_t *special = spa_special_class(spa);
                uint64_t alloc = metaslab_class_get_alloc(special);
                uint64_t space = metaslab_class_get_space(special);

diff --git a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_012_pos.ksh b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_012_pos.ksh
index 2371c5a2689327646b95543bf789c15e2ceae7cb..bd6c6631fa1de78f484f22552aa3bf67a1031706 100755 (executable)
--- a/tests/zfs-tests/tests/functional/alloc_class/alloc_class_012_pos.ksh
+++ b/tests/zfs-tests/tests/functional/alloc_class/alloc_class_012_pos.ksh
@@ -24,6 +24,39 @@
 
 verify_runnable "global"
 
+#
+# Verify the file identified by the input <inode> is written on a special vdev
+# According to the pool layout used in this test vdev_id 3 and 4 are special
+# XXX: move this function to libtest.shlib once we get "Vdev Properties"
+#
+function file_in_special_vdev # <dataset> <inode>
+{
+       typeset dataset="$1"
+       typeset inum="$2"
+
+       zdb -dddddd $dataset $inum | awk '{
+# find DVAs from string "offset level dva" only for L0 (data) blocks
+if (match($0,"L0 [0-9]+")) {
+   dvas[0]=$3
+   dvas[1]=$4
+   dvas[2]=$5
+   for (i = 0; i < 3; ++i) {
+      if (match(dvas[i],"([^:]+):.*")) {
+         dva = substr(dvas[i], RSTART, RLENGTH);
+         # parse DVA from string "vdev:offset:asize"
+         if (split(dva,arr,":") != 3) {
+            print "Error parsing DVA: <" dva ">";
+            exit 1;
+         }
+         # verify vdev is "special"
+         if (arr[1] < 3) {
+            exit 1;
+         }
+      }
+   }
+}}'
+}
+
 claim="Removing a special device from a pool succeeds."
 
 log_assert $claim
@@ -53,6 +86,13 @@ done
 log_must sync_pool $TESTPOOL
 log_must zpool list -v $TESTPOOL
 
+# Verify the files were written in the special class vdevs
+for i in 1 2 3 4; do
+       dataset="$TESTPOOL/$TESTFS"
+       inum="$(stat -c '%i' /$TESTPOOL/$TESTFS/testfile.$i)"
+       log_must file_in_special_vdev $dataset $inum
+done
+
 #
 # remove a special allocation vdev and force a remapping
 # N.B. The 'zfs remap' command has been disabled and may be removed.
@@ -67,6 +107,7 @@ log_must sync_pool $TESTPOOL
 sleep 1
 
 log_must zdb -bbcc $TESTPOOL
+log_must zpool list -v $TESTPOOL
 log_must zpool destroy -f "$TESTPOOL"
 
 log_pass $claim