public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH zfsonlinux] fix #4052 cherry-pick ZERO_RANGE correction
Date: Fri, 13 May 2022 11:00:00 +0200	[thread overview]
Message-ID: <20220513090000.556039-1-s.ivanov@proxmox.com> (raw)

the commit is already queued up for 2.1.5 upstream.
Since the 2.1.5 release is not in the makings yet AFAICS and pulling
in all the other changes has some potential for regressions - I went
for this single cherry-pick

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
quickly tested by compiling a kernel and booting it on 2 VMs and an HP
dl380 g8.

 ...ted-oversight-in-ZERO_RANGE-behavior.patch | 331 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 332 insertions(+)
 create mode 100644 debian/patches/0012-Corrected-oversight-in-ZERO_RANGE-behavior.patch

diff --git a/debian/patches/0012-Corrected-oversight-in-ZERO_RANGE-behavior.patch b/debian/patches/0012-Corrected-oversight-in-ZERO_RANGE-behavior.patch
new file mode 100644
index 00000000..9e185687
--- /dev/null
+++ b/debian/patches/0012-Corrected-oversight-in-ZERO_RANGE-behavior.patch
@@ -0,0 +1,331 @@
+From 0000000000000000000000000000000000000000 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 <behlendorf1@llnl.gov>
+Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
+Closes #13329
+Closes #13338
+(cherry picked from commit c220771a47e4206fb43e6849957657c9504b1b14)
+Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
+---
+ module/os/linux/zfs/zpl_file.c                |  10 +-
+ tests/runfiles/linux.run                      |   2 +-
+ tests/zfs-tests/cmd/file_write/file_write.c   |   2 +-
+ tests/zfs-tests/include/libtest.shlib         |  16 +++
+ .../tests/functional/fallocate/Makefile.am    |   3 +-
+ .../fallocate/fallocate_punch-hole.ksh        |  35 ++++--
+ .../fallocate/fallocate_zero-range.ksh        | 119 ++++++++++++++++++
+ .../tests/functional/fallocate/setup.ksh      |   5 +-
+ 8 files changed, 171 insertions(+), 21 deletions(-)
+ create mode 100755 tests/zfs-tests/tests/functional/fallocate/fallocate_zero-range.ksh
+
+diff --git a/module/os/linux/zfs/zpl_file.c b/module/os/linux/zfs/zpl_file.c
+index f1241c443..3a95a7414 100644
+--- a/module/os/linux/zfs/zpl_file.c
++++ b/module/os/linux/zfs/zpl_file.c
+@@ -764,11 +764,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
+index c01e1e3c4..94c1cbbc3 100644
+--- 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]
+diff --git a/tests/zfs-tests/cmd/file_write/file_write.c b/tests/zfs-tests/cmd/file_write/file_write.c
+index 60893c34f..9d2e71b67 100644
+--- 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);
+diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
+index dd43b02a6..94ab7ffd2 100644
+--- 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
+diff --git a/tests/zfs-tests/tests/functional/fallocate/Makefile.am b/tests/zfs-tests/tests/functional/fallocate/Makefile.am
+index 5ff366d24..86364d789 100644
+--- 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
+index ed83561bd..92f4552f5 100755
+--- 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"
+diff --git a/tests/zfs-tests/tests/functional/fallocate/fallocate_zero-range.ksh b/tests/zfs-tests/tests/functional/fallocate/fallocate_zero-range.ksh
+new file mode 100755
+index 000000000..e907b0f5d
+--- /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"
+diff --git a/tests/zfs-tests/tests/functional/fallocate/setup.ksh b/tests/zfs-tests/tests/functional/fallocate/setup.ksh
+index 32334d396..586ac026a 100755
+--- 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
++
diff --git a/debian/patches/series b/debian/patches/series
index d2770d39..285abdaa 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,3 +9,4 @@
 0009-Patch-move-manpage-arcstat-1-to-arcstat-8.patch
 0010-arcstat-Fix-integer-division-with-python3.patch
 0011-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
+0012-Corrected-oversight-in-ZERO_RANGE-behavior.patch
-- 
2.30.2





                 reply	other threads:[~2022-05-13  9:00 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220513090000.556039-1-s.ivanov@proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal