Corrected oversight in ZERO_RANGE behavior

It turns out, no, in fact, ZERO_RANGE and PUNCH_HOLE do have differing semantics in some ways - in particular, one requires KEEP_SIZE, and the other does not. Also added a zero-range test to catch this, corrected a flaw that made the punch-hole test succeed vacuously, and a typo in file_write. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Rich Ercolani <[email protected]> Closes openzfs#13329 Closes openzfs#13338
andrewc12 · Sep 23, 2022 · 94eae19 · 94eae19
1 parent a56d611
commit 94eae19
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 21 deletions.
diff --git a/module/os/linux/zfs/zpl_file.c b/module/os/linux/zfs/zpl_file.c
@@ -781,11 +781,13 @@ zpl_fallocate_common(struct inode *ip, int mode, loff_t offset, loff_t len)
 	if (mode & (test_mode)) {
 		flock64_t bf;
 
-		if (offset > olen)
-			goto out_unmark;
+		if (mode & FALLOC_FL_KEEP_SIZE) {
+			if (offset > olen)
+				goto out_unmark;
 
-		if (offset + len > olen)
-			len = olen - offset;
+			if (offset + len > olen)
+				len = olen - offset;
+		}
 		bf.l_type = F_WRLCK;
 		bf.l_whence = SEEK_SET;
 		bf.l_start = offset;

diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
@@ -90,7 +90,7 @@ tests = ['events_001_pos', 'events_002_pos', 'zed_rc_filter', 'zed_fd_spill']
 tags = ['functional', 'events']
 
 [tests/functional/fallocate:Linux]
-tests = ['fallocate_prealloc']
+tests = ['fallocate_prealloc', 'fallocate_zero-range']
 tags = ['functional', 'fallocate']
 
 [tests/functional/fault:Linux]

diff --git a/tests/zfs-tests/cmd/file/file_write.c b/tests/zfs-tests/cmd/file/file_write.c
@@ -251,7 +251,7 @@ usage(char *prog)
 	    "\t[-s offset] [-c write_count] [-d data]\n\n"
 	    "Where [data] equal to zero causes chars "
 	    "0->%d to be repeated throughout, or [data]\n"
-	    "equal to 'R' for psudorandom data.\n",
+	    "equal to 'R' for pseudorandom data.\n",
 	    prog, DATA_RANGE);
 
 	exit(1);

diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
@@ -3602,6 +3602,22 @@ function punch_hole # offset length file
 	esac
 }
 
+function zero_range # offset length file
+{
+	typeset offset=$1
+	typeset length=$2
+	typeset file=$3
+
+	case "$UNAME" in
+	Linux)
+		fallocate --zero-range --offset $offset --length $length "$file"
+		;;
+	*)
+		false
+		;;
+	esac
+}
+
 #
 # Wait for the specified arcstat to reach non-zero quiescence.
 # If echo is 1 echo the value after reaching quiescence, otherwise

diff --git a/tests/zfs-tests/tests/functional/fallocate/Makefile.am b/tests/zfs-tests/tests/functional/fallocate/Makefile.am
@@ -3,4 +3,5 @@ dist_pkgdata_SCRIPTS = \
 	setup.ksh \
 	cleanup.ksh \
 	fallocate_prealloc.ksh \
-	fallocate_punch-hole.ksh
+	fallocate_punch-hole.ksh \
+	fallocate_zero-range.ksh
diff --git a/tests/zfs-tests/tests/functional/fallocate/fallocate_punch-hole.ksh b/tests/zfs-tests/tests/functional/fallocate/fallocate_punch-hole.ksh
@@ -60,23 +60,27 @@ function cleanup
 	[[ -e $TESTDIR ]] && log_must rm -f $FILE
 }
 
-function check_disk_size
+function check_reported_size
 {
 	typeset expected_size=$1
 
-	disk_size=$(du $TESTDIR/file | awk '{print $1}')
-	if [ $disk_size -ne $expected_size ]; then
-		log_fail "Incorrect size: $disk_size != $expected_size"
+	if ! [ -e "${FILE}" ]; then
+		log_fail "$FILE does not exist"
+	fi
+
+	reported_size=$(du "${FILE}" | awk '{print $1}')
+	if [ "$reported_size" != "$expected_size" ]; then
+		log_fail "Incorrect reported size: $reported_size != $expected_size"
 	fi
 }
 
 function check_apparent_size
 {
 	typeset expected_size=$1
 
-	apparent_size=$(stat_size)
-	if [ $apparent_size -ne $expected_size ]; then
-		log_fail "Incorrect size: $apparent_size != $expected_size"
+	apparent_size=$(stat_size "${FILE}")
+	if [ "$apparent_size" != "$expected_size" ]; then
+		log_fail "Incorrect apparent size: $apparent_size != $expected_size"
 	fi
 }
 
@@ -86,25 +90,30 @@ log_onexit cleanup
 
 # Create a dense file and check it is the correct size.
 log_must file_write -o create -f $FILE -b $BLKSZ -c 8
-log_must check_disk_size  $((131072 * 8))
+sync_pool $TESTPOOL
+log_must check_reported_size 1027
 
 # Punch a hole for the first full block.
 log_must punch_hole 0 $BLKSZ $FILE
-log_must check_disk_size  $((131072 * 7))
+sync_pool $TESTPOOL
+log_must check_reported_size 899
 
 # Partially punch a hole in the second block.
 log_must punch_hole $BLKSZ $((BLKSZ / 2)) $FILE
-log_must check_disk_size  $((131072 * 7))
+sync_pool $TESTPOOL
+log_must check_reported_size 899
 
-# Punch a hole which overlaps the third and forth block.
+# Punch a hole which overlaps the third and fourth block.
 log_must punch_hole $(((BLKSZ * 2) + (BLKSZ / 2))) $((BLKSZ)) $FILE
-log_must check_disk_size  $((131072 * 7))
+sync_pool $TESTPOOL
+log_must check_reported_size 899
 
 # Punch a hole from the fifth block past the end of file.  The apparent
 # file size should not change since --keep-size is implied.
 apparent_size=$(stat_size $FILE)
 log_must punch_hole $((BLKSZ * 4)) $((BLKSZ * 10)) $FILE
-log_must check_disk_size  $((131072 * 4))
+sync_pool $TESTPOOL
+log_must check_reported_size 387
 log_must check_apparent_size $apparent_size
 
 log_pass "Ensure holes can be punched in files making them sparse"
diff --git a/tests/zfs-tests/tests/functional/fallocate/fallocate_zero-range.ksh b/tests/zfs-tests/tests/functional/fallocate/fallocate_zero-range.ksh
@@ -0,0 +1,119 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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) 2020 by Lawrence Livermore National Security, LLC.
+# Copyright (c) 2021 by The FreeBSD Foundation.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+#
+# DESCRIPTION:
+# Test FALLOC_FL_ZERO_RANGE functionality
+#
+# STRATEGY:
+# 1. Create a dense file
+# 2. Zero various ranges in the file and verify the result.
+#
+
+verify_runnable "global"
+
+if is_freebsd; then
+	log_unsupported "FreeBSD does not implement an analogue to ZERO_RANGE."
+fi
+
+FILE=$TESTDIR/$TESTFILE0
+BLKSZ=$(get_prop recordsize $TESTPOOL)
+
+function cleanup
+{
+	[[ -e $TESTDIR ]] && log_must rm -f $FILE
+}
+
+# Helpfully, this function expects kilobytes, and check_apparent_size expects bytes.
+function check_reported_size
+{
+	typeset expected_size=$1
+
+	if ! [ -e "${FILE}" ]; then
+		log_fail "$FILE does not exist"
+	fi
+
+	reported_size=$(du "${FILE}" | awk '{print $1}')
+	if [ "$reported_size" != "$expected_size" ]; then
+		log_fail "Incorrect reported size: $reported_size != $expected_size"
+	fi
+}
+
+function check_apparent_size
+{
+	typeset expected_size=$1
+
+	apparent_size=$(stat_size "${FILE}")
+	if [ "$apparent_size" != "$expected_size" ]; then
+		log_fail "Incorrect apparent size: $apparent_size != $expected_size"
+	fi
+}
+
+log_assert "Ensure ranges can be zeroed in files"
+
+log_onexit cleanup
+
+# Create a dense file and check it is the correct size.
+log_must file_write -o create -f $FILE -b $BLKSZ -c 8
+sync_pool $TESTPOOL
+log_must check_reported_size 1027
+
+# Zero a range covering the first full block.
+log_must zero_range 0 $BLKSZ $FILE
+sync_pool $TESTPOOL
+log_must check_reported_size 899
+
+# Partially zero a range in the second block.
+log_must zero_range $BLKSZ $((BLKSZ / 2)) $FILE
+sync_pool $TESTPOOL
+log_must check_reported_size 899
+
+# Zero range which overlaps the third and fourth block.
+log_must zero_range $(((BLKSZ * 2) + (BLKSZ / 2))) $((BLKSZ)) $FILE
+sync_pool $TESTPOOL
+log_must check_reported_size 899
+
+# Zero range from the fifth block past the end of file, with --keep-size.
+# The apparent file size must not change, since we did specify --keep-size.
+apparent_size=$(stat_size $FILE)
+log_must fallocate --keep-size --zero-range --offset $((BLKSZ * 4)) --length $((BLKSZ * 10)) "$FILE"
+sync_pool $TESTPOOL
+log_must check_reported_size 387
+log_must check_apparent_size $apparent_size
+
+# Zero range from the fifth block past the end of file.  The apparent
+# file size should change since --keep-size is not implied, unlike
+# with PUNCH_HOLE.
+apparent_size=$(stat_size $FILE)
+log_must zero_range $((BLKSZ * 4)) $((BLKSZ * 10)) $FILE
+sync_pool $TESTPOOL
+log_must check_reported_size 387
+log_must check_apparent_size $((BLKSZ * 14))
+
+log_pass "Ensure ranges can be zeroed in files"
diff --git a/tests/zfs-tests/tests/functional/fallocate/setup.ksh b/tests/zfs-tests/tests/functional/fallocate/setup.ksh
@@ -26,4 +26,7 @@
 . $STF_SUITE/include/libtest.shlib
 
 DISK=${DISKS%% *}
-default_setup $DISK
+default_setup_noexit $DISK
+log_must zfs set compression=off $TESTPOOL
+log_pass
+