Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zfs: backport patches for block cloning tunable and default disable; zfsUnstable: 2.2.1-unstable-2023-10-21 -> 2.2.1 #269465

Closed
wants to merge 2 commits into from
Closed

zfs: backport patches for block cloning tunable and default disable; zfsUnstable: 2.2.1-unstable-2023-10-21 -> 2.2.1 #269465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f278b09
zfsUnstable: 2.2.1-unstable-2023-10-21 -> 2.2.1
Mic92 Nov 22, 2023
3ca5f2a
zfs: backport patches for block cloning tunable and default disable
amarshall Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 203 additions & 0 deletions pkgs/os-specific/linux/zfs/brt-tunable.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
From 04bb75ca7d95459ea14f6fde112c9ae0128c722d Mon Sep 17 00:00:00 2001
From: Rich Ercolani <[email protected]>
Date: Thu, 16 Nov 2023 14:35:22 -0500
Subject: [PATCH] Add a tunable to disable BRT support.

Copy the disable parameter that FreeBSD implemented, and extend it to
work on Linux as well, until we're sure this is stable.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes #15529
(cherry picked from commit 03e9caaec006134b3db9d02ac40fe9369ee78b03)
---
include/os/freebsd/zfs/sys/zfs_vfsops_os.h | 1 +
include/os/linux/zfs/sys/zfs_vfsops_os.h | 2 ++
man/man4/zfs.4 | 5 +++++
module/os/freebsd/zfs/zfs_vfsops.c | 4 ++++
module/os/freebsd/zfs/zfs_vnops_os.c | 5 +++++
module/os/linux/zfs/zfs_vnops_os.c | 4 ++++
module/os/linux/zfs/zpl_file_range.c | 5 +++++
tests/zfs-tests/include/libtest.shlib | 15 +++++++++++++++
tests/zfs-tests/include/tunables.cfg | 1 +
.../tests/functional/block_cloning/cleanup.ksh | 4 ++++
.../tests/functional/block_cloning/setup.ksh | 5 +++++
11 files changed, 51 insertions(+)

diff --git a/include/os/freebsd/zfs/sys/zfs_vfsops_os.h b/include/os/freebsd/zfs/sys/zfs_vfsops_os.h
index 24bb03575..56a0ac96a 100644
--- a/include/os/freebsd/zfs/sys/zfs_vfsops_os.h
+++ b/include/os/freebsd/zfs/sys/zfs_vfsops_os.h
@@ -286,6 +286,7 @@ typedef struct zfid_long {

extern uint_t zfs_fsyncer_key;
extern int zfs_super_owner;
+extern int zfs_bclone_enabled;

extern void zfs_init(void);
extern void zfs_fini(void);
diff --git a/include/os/linux/zfs/sys/zfs_vfsops_os.h b/include/os/linux/zfs/sys/zfs_vfsops_os.h
index b4d5db21f..220466550 100644
--- a/include/os/linux/zfs/sys/zfs_vfsops_os.h
+++ b/include/os/linux/zfs/sys/zfs_vfsops_os.h
@@ -45,6 +45,8 @@ extern "C" {
typedef struct zfsvfs zfsvfs_t;
struct znode;

+extern int zfs_bclone_enabled;
+
/*
* This structure emulates the vfs_t from other platforms. It's purpose
* is to facilitate the handling of mount options and minimize structural
diff --git a/man/man4/zfs.4 b/man/man4/zfs.4
index 71a3e67ee..e26d92526 100644
--- a/man/man4/zfs.4
+++ b/man/man4/zfs.4
@@ -1137,6 +1137,11 @@ Selecting any option other than
results in vector instructions
from the respective CPU instruction set being used.
.
+.It Sy zfs_bclone_enabled Ns = Ns Sy 1 Ns | Ns 0 Pq int
+Enable the experimental block cloning feature.
+If this setting is 0, then even if feature@block_cloning is enabled,
+attempts to clone blocks will act as though the feature is disabled.
+.
.It Sy zfs_blake3_impl Ns = Ns Sy fastest Pq string
Select a BLAKE3 implementation.
.Pp
diff --git a/module/os/freebsd/zfs/zfs_vfsops.c b/module/os/freebsd/zfs/zfs_vfsops.c
index e8b9ada13..09e18de81 100644
--- a/module/os/freebsd/zfs/zfs_vfsops.c
+++ b/module/os/freebsd/zfs/zfs_vfsops.c
@@ -89,6 +89,10 @@ int zfs_debug_level;
SYSCTL_INT(_vfs_zfs, OID_AUTO, debug, CTLFLAG_RWTUN, &zfs_debug_level, 0,
"Debug level");

+int zfs_bclone_enabled = 1;
+SYSCTL_INT(_vfs_zfs, OID_AUTO, bclone_enabled, CTLFLAG_RWTUN,
+ &zfs_bclone_enabled, 0, "Enable block cloning");
+
struct zfs_jailparam {
int mount_snapshot;
};
diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c
index c498a1328..f672deed3 100644
--- a/module/os/freebsd/zfs/zfs_vnops_os.c
+++ b/module/os/freebsd/zfs/zfs_vnops_os.c
@@ -6243,6 +6243,11 @@ zfs_freebsd_copy_file_range(struct vop_copy_file_range_args *ap)
int error;
uint64_t len = *ap->a_lenp;

+ if (!zfs_bclone_enabled) {
+ mp = NULL;
+ goto bad_write_fallback;
+ }
+
/*
* TODO: If offset/length is not aligned to recordsize, use
* vn_generic_copy_file_range() on this fragment.
diff --git a/module/os/linux/zfs/zfs_vnops_os.c b/module/os/linux/zfs/zfs_vnops_os.c
index 33baac9db..76fac3a02 100644
--- a/module/os/linux/zfs/zfs_vnops_os.c
+++ b/module/os/linux/zfs/zfs_vnops_os.c
@@ -4229,4 +4229,8 @@ EXPORT_SYMBOL(zfs_map);
module_param(zfs_delete_blocks, ulong, 0644);
MODULE_PARM_DESC(zfs_delete_blocks, "Delete files larger than N blocks async");

+/* CSTYLED */
+module_param(zfs_bclone_enabled, uint, 0644);
+MODULE_PARM_DESC(zfs_bclone_enabled, "Enable block cloning");
+
#endif
diff --git a/module/os/linux/zfs/zpl_file_range.c b/module/os/linux/zfs/zpl_file_range.c
index c47fe99da..73476ff40 100644
--- a/module/os/linux/zfs/zpl_file_range.c
+++ b/module/os/linux/zfs/zpl_file_range.c
@@ -31,6 +31,8 @@
#include <sys/zfs_vnops.h>
#include <sys/zfeature.h>

+int zfs_bclone_enabled = 1;
+
/*
* Clone part of a file via block cloning.
*
@@ -50,6 +52,9 @@ __zpl_clone_file_range(struct file *src_file, loff_t src_off,
fstrans_cookie_t cookie;
int err;

+ if (!zfs_bclone_enabled)
+ return (-EOPNOTSUPP);
+
if (!spa_feature_is_enabled(
dmu_objset_spa(ITOZSB(dst_i)->z_os), SPA_FEATURE_BLOCK_CLONING))
return (-EOPNOTSUPP);
diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
index 844caa17d..d5d7bb6c8 100644
--- a/tests/zfs-tests/include/libtest.shlib
+++ b/tests/zfs-tests/include/libtest.shlib
@@ -3334,6 +3334,21 @@ function set_tunable_impl
esac
}

+function save_tunable
+{
+ [[ ! -d $TEST_BASE_DIR ]] && return 1
+ [[ -e $TEST_BASE_DIR/tunable-$1 ]] && return 2
+ echo "$(get_tunable """$1""")" > "$TEST_BASE_DIR"/tunable-"$1"
+}
+
+function restore_tunable
+{
+ [[ ! -e $TEST_BASE_DIR/tunable-$1 ]] && return 1
+ val="$(cat $TEST_BASE_DIR/tunable-"""$1""")"
+ set_tunable64 "$1" "$val"
+ rm $TEST_BASE_DIR/tunable-$1
+}
+
#
# Get a global system tunable
#
diff --git a/tests/zfs-tests/include/tunables.cfg b/tests/zfs-tests/include/tunables.cfg
index 8010a9451..1e8c8fd71 100644
--- a/tests/zfs-tests/include/tunables.cfg
+++ b/tests/zfs-tests/include/tunables.cfg
@@ -90,6 +90,7 @@ VOL_INHIBIT_DEV UNSUPPORTED zvol_inhibit_dev
VOL_MODE vol.mode zvol_volmode
VOL_RECURSIVE vol.recursive UNSUPPORTED
VOL_USE_BLK_MQ UNSUPPORTED UNSUPPORTED
+BCLONE_ENABLED zfs_bclone_enabled zfs_bclone_enabled
XATTR_COMPAT xattr_compat zfs_xattr_compat
ZEVENT_LEN_MAX zevent.len_max zfs_zevent_len_max
ZEVENT_RETAIN_MAX zevent.retain_max zfs_zevent_retain_max
diff --git a/tests/zfs-tests/tests/functional/block_cloning/cleanup.ksh b/tests/zfs-tests/tests/functional/block_cloning/cleanup.ksh
index 7ac13adb6..b985445a5 100755
--- a/tests/zfs-tests/tests/functional/block_cloning/cleanup.ksh
+++ b/tests/zfs-tests/tests/functional/block_cloning/cleanup.ksh
@@ -31,4 +31,8 @@ verify_runnable "global"

default_cleanup_noexit

+if tunable_exists BCLONE_ENABLED ; then
+ log_must restore_tunable BCLONE_ENABLED
+fi
+
log_pass
diff --git a/tests/zfs-tests/tests/functional/block_cloning/setup.ksh b/tests/zfs-tests/tests/functional/block_cloning/setup.ksh
index 512f5a064..58441bf8f 100755
--- a/tests/zfs-tests/tests/functional/block_cloning/setup.ksh
+++ b/tests/zfs-tests/tests/functional/block_cloning/setup.ksh
@@ -33,4 +33,9 @@ fi

verify_runnable "global"

+if tunable_exists BCLONE_ENABLED ; then
+ log_must save_tunable BCLONE_ENABLED
+ log_must set_tunable32 BCLONE_ENABLED 1
+fi
+
log_pass
--
2.42.0

10 changes: 9 additions & 1 deletion pkgs/os-specific/linux/zfs/stable.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
, stdenv
, linuxKernel
, removeLinuxDRM ? false
, fetchpatch
, fetchpatch2
, ...
} @ args:

Expand All @@ -25,4 +25,12 @@ callPackage ./generic.nix args {
version = "2.2.0";

sha256 = "sha256-s1sdXSrLu6uSOmjprbUa4cFsE2Vj7JX5i75e4vRnlvg=";

extraPatches = [
./brt-tunable.patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should check this in our zfs nixos test?

% zpool get all | grep feature@block_cloning
zroot  feature@block_cloning          disabled                       local

after having the flag?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pool feature will still be enabled, instead /sys/module/zfs/parameters/zfs_bclone_enabled will be available and 0.

It’s a bit tricky since with #269452 we will have a variant where we expect it to not be present at all (so we should handle that), but also vanilla 2.2.0 won’t have it either—but then we want to fail if it’s not present. From initial glance, makeZfsTest makes it a bit tricky to switch on the version… I guess could switch on the version in the test itself?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can definitely encode version checking inside makeZfsTest if you want, you just have to use package and look at its version.

(fetchpatch2 {
url = "https://github.com/openzfs/zfs/commit/479dca51c66a731e637bd2d4f9bba01a05f9ac9f.patch";
hash = "sha256-Qil4sm2h+d2fjwuiQO1nJ63/M83K3XH2A3wEFNr2f5M=";
})
];
}
9 changes: 4 additions & 5 deletions pkgs/os-specific/linux/zfs/unstable.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,20 @@ in
callPackage ./generic.nix args {
# check the release notes for compatible kernels
kernelCompatible = if stdenv'.isx86_64 || removeLinuxDRM
then kernel.kernelOlder "6.6"
then kernel.kernelOlder "6.7"
else kernel.kernelOlder "6.2";

latestCompatibleLinuxPackages = if stdenv'.isx86_64 || removeLinuxDRM
then linuxKernel.packages.linux_6_5
then linuxKernel.packages.linux_6_6
else linuxKernel.packages.linux_6_1;

# this package should point to a version / git revision compatible with the latest kernel release
# IMPORTANT: Always use a tagged release candidate or commits from the
# zfs-<version>-staging branch, because this is tested by the OpenZFS
# maintainers.
version = "2.2.1-unstable-2023-10-21";
rev = "95785196f26e92d82cf4445654ba84e4a9671c57";
version = "2.2.1";

sha256 = "sha256-s1sdXSrLu6uSOmjprbUa4cFsE2Vj7JX5i75e4vRnlvg=";
sha256 = "sha256-2Q/Nhp3YKgMCLPNRNBq5r9U4GeuYlWMWAsjsQy3vFW4=";

isUnstable = true;
}