https://github.com/openzfs/zfs/commit/c220771a47e4206fb43e6849957657c9504b1b14 https://github.com/openzfs/zfs/issues/13329 From c220771a47e4206fb43e6849957657c9504b1b14 Mon Sep 17 00:00:00 2001 From: Rich Ercolani <214141+rincebrain@users.noreply.github.com> Date: Wed, 20 Apr 2022 19:07:03 -0400 Subject: [PATCH] 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 Signed-off-by: Rich Ercolani Closes #13329 Closes #13338 --- 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; --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -94,7 +94,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] --- a/tests/zfs-tests/cmd/file_write/file_write.c +++ b/tests/zfs-tests/cmd/file_write/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); --- a/tests/zfs-tests/include/libtest.shlib +++ b/tests/zfs-tests/include/libtest.shlib @@ -4236,6 +4236,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 --- 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 --- a/tests/zfs-tests/tests/functional/fallocate/fallocate_punch-hole.ksh +++ b/tests/zfs-tests/tests/functional/fallocate/fallocate_punch-hole.ksh @@ -60,13 +60,17 @@ 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 } @@ -74,9 +78,9 @@ 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" --- /dev/null +++ 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" --- 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 +