From 13b506c39225d71c773b450a2186ddcefe1efe48 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 22 Jan 2025 16:50:04 -0700 Subject: [PATCH 01/10] Make the vfs.zfs.vdev.raidz_impl sysctl cross-platform Signed-off-by: Alan Somers Sponsored by: ConnectWise --- include/os/freebsd/spl/sys/mod_os.h | 3 +++ include/sys/vdev_impl.h | 2 ++ include/sys/vdev_raidz.h | 1 + module/os/freebsd/zfs/sysctl_os.c | 20 +++++++++++++++++++ module/os/linux/zfs/vdev_disk.c | 12 +++++++++++ module/zfs/vdev.c | 4 ++++ module/zfs/vdev_raidz_math.c | 31 ++++++++++------------------- 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/include/os/freebsd/spl/sys/mod_os.h b/include/os/freebsd/spl/sys/mod_os.h index df7be6fc13f6..1479242de53b 100644 --- a/include/os/freebsd/spl/sys/mod_os.h +++ b/include/os/freebsd/spl/sys/mod_os.h @@ -94,6 +94,9 @@ #define param_set_max_auto_ashift_args(var) \ CTLTYPE_UINT, NULL, 0, param_set_max_auto_ashift, "IU" +#define param_set_raidz_impl_args(var) \ + CTLTYPE_STRING, NULL, 0, param_set_raidz_impl, "A" + #define spa_taskq_read_param_set_args(var) \ CTLTYPE_STRING, NULL, 0, spa_taskq_read_param, "A" diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index abd66b8abc96..46ca2859f991 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -62,6 +62,7 @@ struct abd; extern uint_t zfs_vdev_queue_depth_pct; extern uint_t zfs_vdev_def_queue_depth; extern uint_t zfs_vdev_async_write_max_active; +extern const char *zfs_vdev_raidz_impl; /* * Virtual device operations @@ -645,6 +646,7 @@ extern int vdev_obsolete_counts_are_precise(vdev_t *vd, boolean_t *are_precise); int vdev_checkpoint_sm_object(vdev_t *vd, uint64_t *sm_obj); void vdev_metaslab_group_create(vdev_t *vd); uint64_t vdev_best_ashift(uint64_t logical, uint64_t a, uint64_t b); +int param_set_raidz_impl(ZFS_MODULE_PARAM_ARGS); /* * Vdev ashift optimization tunables diff --git a/include/sys/vdev_raidz.h b/include/sys/vdev_raidz.h index 64f484e9aa13..eec5b17aa7c4 100644 --- a/include/sys/vdev_raidz.h +++ b/include/sys/vdev_raidz.h @@ -73,6 +73,7 @@ int vdev_raidz_math_generate(struct raidz_map *, struct raidz_row *); int vdev_raidz_math_reconstruct(struct raidz_map *, struct raidz_row *, const int *, const int *, const int); int vdev_raidz_impl_set(const char *); +int vdev_raidz_impl_get(char *buffer, size_t size); typedef struct vdev_raidz_expand { uint64_t vre_vdev_id; diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c index 7350b8a6d49f..0be71ffce022 100644 --- a/module/os/freebsd/zfs/sysctl_os.c +++ b/module/os/freebsd/zfs/sysctl_os.c @@ -679,6 +679,26 @@ param_set_deadman_failmode(SYSCTL_HANDLER_ARGS) return (-param_set_deadman_failmode_common(buf)); } +int +param_set_raidz_impl(SYSCTL_HANDLER_ARGS) +{ + char *buf; + int rc; + + buf = malloc(64, M_SOLARIS, M_NOWAIT | M_ZERO); + if (req->newptr == NULL) + vdev_raidz_impl_get(buf, 64); + + rc = sysctl_handle_string(oidp, buf, 64, req); + if (rc || req->newptr == NULL) { + free(buf, M_SOLARIS); + return (rc); + } + rc = vdev_raidz_impl_set(buf); + free(buf, M_SOLARIS); + return (rc); +} + int param_set_slop_shift(SYSCTL_HANDLER_ARGS) { diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index e8bd513e6909..13c3013ee626 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1637,6 +1637,18 @@ param_set_max_auto_ashift(const char *buf, zfs_kernel_param_t *kp) return (0); } +const char *zfs_vdev_raidz_impl = "TODO"; + +int +param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) +{ + int error; + + error = vdev_raidz_impl_set(val); + + return (error); +} + ZFS_MODULE_PARAM(zfs_vdev, zfs_vdev_, open_timeout_ms, UINT, ZMOD_RW, "Timeout before determining that a device is missing"); diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 5df2f77e5780..438b849ff704 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -6580,3 +6580,7 @@ ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, max_auto_ashift, param_set_max_auto_ashift, param_get_uint, ZMOD_RW, "Maximum ashift used when optimizing for logical -> physical sector " "size on new top-level vdevs"); + +ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, raidz_impl, + param_set_raidz_impl, param_get_charp, ZMOD_RW, + "RAIDZ implementation"); diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index e12b96170f55..34a08f5c076f 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -81,7 +81,7 @@ static boolean_t raidz_math_initialized = B_FALSE; #define RAIDZ_IMPL_READ(i) (*(volatile uint32_t *) &(i)) -static uint32_t zfs_vdev_raidz_impl = IMPL_SCALAR; +static uint32_t zfs_vdev_raidz_impl_setting = IMPL_SCALAR; static uint32_t user_sel_impl = IMPL_FASTEST; /* Hold all supported implementations */ @@ -111,7 +111,7 @@ vdev_raidz_math_get_ops(void) return (&vdev_raidz_scalar_impl); raidz_impl_ops_t *ops = NULL; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); switch (impl) { case IMPL_FASTEST: @@ -540,7 +540,7 @@ vdev_raidz_math_init(void) #endif /* Finish initialization */ - atomic_swap_32(&zfs_vdev_raidz_impl, user_sel_impl); + atomic_swap_32(&zfs_vdev_raidz_impl_setting, user_sel_impl); raidz_math_initialized = B_TRUE; } @@ -579,7 +579,7 @@ static const struct { * If we are called before init(), user preference will be saved in * user_sel_impl, and applied in later init() call. This occurs when module * parameter is specified on module load. Otherwise, directly update - * zfs_vdev_raidz_impl. + * zfs_vdev_raidz_impl_setting. * * @val Name of raidz implementation to use * @param Unused. @@ -625,7 +625,7 @@ vdev_raidz_impl_set(const char *val) if (err == 0) { if (raidz_math_initialized) - atomic_swap_32(&zfs_vdev_raidz_impl, impl); + atomic_swap_32(&zfs_vdev_raidz_impl_setting, impl); else atomic_swap_32(&user_sel_impl, impl); } @@ -633,41 +633,32 @@ vdev_raidz_impl_set(const char *val) return (err); } -#if defined(_KERNEL) && defined(__linux__) - -static int -zfs_vdev_raidz_impl_set(const char *val, zfs_kernel_param_t *kp) -{ - return (vdev_raidz_impl_set(val)); -} +#if defined(_KERNEL) -static int -zfs_vdev_raidz_impl_get(char *buffer, zfs_kernel_param_t *kp) +int +vdev_raidz_impl_get(char *buffer, size_t size) { int i, cnt = 0; char *fmt; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); ASSERT(raidz_math_initialized); /* list mandatory options */ for (i = 0; i < ARRAY_SIZE(math_impl_opts) - 2; i++) { fmt = (impl == math_impl_opts[i].sel) ? "[%s] " : "%s "; - cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + cnt += kmem_scnprintf(buffer + cnt, size - cnt, fmt, math_impl_opts[i].name); } /* list all supported implementations */ for (i = 0; i < raidz_supp_impl_cnt; i++) { fmt = (i == impl) ? "[%s] " : "%s "; - cnt += kmem_scnprintf(buffer + cnt, PAGE_SIZE - cnt, fmt, + cnt += kmem_scnprintf(buffer + cnt, size - cnt, fmt, raidz_supp_impl[i]->name); } return (cnt); } -module_param_call(zfs_vdev_raidz_impl, zfs_vdev_raidz_impl_set, - zfs_vdev_raidz_impl_get, NULL, 0644); -MODULE_PARM_DESC(zfs_vdev_raidz_impl, "Select raidz implementation."); #endif From 6a214e68b443818b245e9b16284ec05e533f205d Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 23 Jan 2025 11:18:32 -0700 Subject: [PATCH 02/10] fixup: Implement the "get" path of raidz_impl for Linux Signed-off-by: Alan Somers --- include/sys/vdev_raidz.h | 3 +++ module/os/linux/zfs/vdev_disk.c | 3 --- module/zfs/vdev_raidz_math.c | 6 ++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/sys/vdev_raidz.h b/include/sys/vdev_raidz.h index eec5b17aa7c4..e8af9f3715f1 100644 --- a/include/sys/vdev_raidz.h +++ b/include/sys/vdev_raidz.h @@ -66,6 +66,9 @@ extern const zio_vsd_ops_t vdev_raidz_vsd_ops; /* * vdev_raidz_math interface */ +#if defined(__linux__) +extern const char *zfs_vdev_raidz_impl; +#endif void vdev_raidz_math_init(void); void vdev_raidz_math_fini(void); const struct raidz_impl_ops *vdev_raidz_math_get_ops(void); diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index 13c3013ee626..8f464bb9de9e 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1637,15 +1637,12 @@ param_set_max_auto_ashift(const char *buf, zfs_kernel_param_t *kp) return (0); } -const char *zfs_vdev_raidz_impl = "TODO"; - int param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) { int error; error = vdev_raidz_impl_set(val); - return (error); } diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index 34a08f5c076f..6eb017c29c25 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -83,6 +83,9 @@ static boolean_t raidz_math_initialized = B_FALSE; static uint32_t zfs_vdev_raidz_impl_setting = IMPL_SCALAR; static uint32_t user_sel_impl = IMPL_FASTEST; +#if defined(__linux__) +const char *zfs_vdev_raidz_impl = "fastest"; +#endif /* Hold all supported implementations */ static size_t raidz_supp_impl_cnt = 0; @@ -628,6 +631,9 @@ vdev_raidz_impl_set(const char *val) atomic_swap_32(&zfs_vdev_raidz_impl_setting, impl); else atomic_swap_32(&user_sel_impl, impl); +#if defined(__linux__) + zfs_vdev_raidz_impl = raidz_supp_impl[i]->name; +#endif } return (err); From ba180bdbe09dab24b44a8945320225991a2c194d Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 23 Jan 2025 16:14:02 -0700 Subject: [PATCH 03/10] Don't attempt to use the portable sysctl macros Instead, define the FreeBSD and Linux sysctls separately. Signed-off-by: Alan Somers --- include/os/freebsd/spl/sys/mod_os.h | 3 --- include/sys/vdev_impl.h | 2 -- include/sys/vdev_raidz.h | 5 +--- module/os/freebsd/zfs/sysctl_os.c | 8 ++++++- module/os/linux/zfs/vdev_disk.c | 9 -------- module/zfs/vdev.c | 4 ---- module/zfs/vdev_raidz_math.c | 36 +++++++++++++++++++---------- 7 files changed, 32 insertions(+), 35 deletions(-) diff --git a/include/os/freebsd/spl/sys/mod_os.h b/include/os/freebsd/spl/sys/mod_os.h index 1479242de53b..df7be6fc13f6 100644 --- a/include/os/freebsd/spl/sys/mod_os.h +++ b/include/os/freebsd/spl/sys/mod_os.h @@ -94,9 +94,6 @@ #define param_set_max_auto_ashift_args(var) \ CTLTYPE_UINT, NULL, 0, param_set_max_auto_ashift, "IU" -#define param_set_raidz_impl_args(var) \ - CTLTYPE_STRING, NULL, 0, param_set_raidz_impl, "A" - #define spa_taskq_read_param_set_args(var) \ CTLTYPE_STRING, NULL, 0, spa_taskq_read_param, "A" diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index 46ca2859f991..abd66b8abc96 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -62,7 +62,6 @@ struct abd; extern uint_t zfs_vdev_queue_depth_pct; extern uint_t zfs_vdev_def_queue_depth; extern uint_t zfs_vdev_async_write_max_active; -extern const char *zfs_vdev_raidz_impl; /* * Virtual device operations @@ -646,7 +645,6 @@ extern int vdev_obsolete_counts_are_precise(vdev_t *vd, boolean_t *are_precise); int vdev_checkpoint_sm_object(vdev_t *vd, uint64_t *sm_obj); void vdev_metaslab_group_create(vdev_t *vd); uint64_t vdev_best_ashift(uint64_t logical, uint64_t a, uint64_t b); -int param_set_raidz_impl(ZFS_MODULE_PARAM_ARGS); /* * Vdev ashift optimization tunables diff --git a/include/sys/vdev_raidz.h b/include/sys/vdev_raidz.h index e8af9f3715f1..b50bee946158 100644 --- a/include/sys/vdev_raidz.h +++ b/include/sys/vdev_raidz.h @@ -66,17 +66,14 @@ extern const zio_vsd_ops_t vdev_raidz_vsd_ops; /* * vdev_raidz_math interface */ -#if defined(__linux__) -extern const char *zfs_vdev_raidz_impl; -#endif void vdev_raidz_math_init(void); void vdev_raidz_math_fini(void); const struct raidz_impl_ops *vdev_raidz_math_get_ops(void); int vdev_raidz_math_generate(struct raidz_map *, struct raidz_row *); int vdev_raidz_math_reconstruct(struct raidz_map *, struct raidz_row *, const int *, const int *, const int); -int vdev_raidz_impl_set(const char *); int vdev_raidz_impl_get(char *buffer, size_t size); +int vdev_raidz_impl_set(const char *); typedef struct vdev_raidz_expand { uint64_t vre_vdev_id; diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c index 0be71ffce022..109af1d6f6e8 100644 --- a/module/os/freebsd/zfs/sysctl_os.c +++ b/module/os/freebsd/zfs/sysctl_os.c @@ -679,7 +679,7 @@ param_set_deadman_failmode(SYSCTL_HANDLER_ARGS) return (-param_set_deadman_failmode_common(buf)); } -int +static int param_set_raidz_impl(SYSCTL_HANDLER_ARGS) { char *buf; @@ -808,6 +808,12 @@ SYSCTL_INT(_vfs_zfs, OID_AUTO, validate_skip, /* vdev_mirror.c */ +/* vdev_raidz_math.c */ + +SYSCTL_PROC(_vfs_zfs_vdev, OID_AUTO, raidz_impl, + CTLTYPE_STRING | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, + NULL, 0, param_set_raidz_impl, "IU", "select RAIDZ implementation"); + /* vdev_queue.c */ extern uint_t zfs_vdev_max_active; diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index 8f464bb9de9e..e8bd513e6909 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1637,15 +1637,6 @@ param_set_max_auto_ashift(const char *buf, zfs_kernel_param_t *kp) return (0); } -int -param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) -{ - int error; - - error = vdev_raidz_impl_set(val); - return (error); -} - ZFS_MODULE_PARAM(zfs_vdev, zfs_vdev_, open_timeout_ms, UINT, ZMOD_RW, "Timeout before determining that a device is missing"); diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 438b849ff704..5df2f77e5780 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -6580,7 +6580,3 @@ ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, max_auto_ashift, param_set_max_auto_ashift, param_get_uint, ZMOD_RW, "Maximum ashift used when optimizing for logical -> physical sector " "size on new top-level vdevs"); - -ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, raidz_impl, - param_set_raidz_impl, param_get_charp, ZMOD_RW, - "RAIDZ implementation"); diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index 6eb017c29c25..b2eea4c083be 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -81,11 +81,8 @@ static boolean_t raidz_math_initialized = B_FALSE; #define RAIDZ_IMPL_READ(i) (*(volatile uint32_t *) &(i)) -static uint32_t zfs_vdev_raidz_impl_setting = IMPL_SCALAR; +static uint32_t zfs_vdev_raidz_impl = IMPL_SCALAR; static uint32_t user_sel_impl = IMPL_FASTEST; -#if defined(__linux__) -const char *zfs_vdev_raidz_impl = "fastest"; -#endif /* Hold all supported implementations */ static size_t raidz_supp_impl_cnt = 0; @@ -114,7 +111,7 @@ vdev_raidz_math_get_ops(void) return (&vdev_raidz_scalar_impl); raidz_impl_ops_t *ops = NULL; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); switch (impl) { case IMPL_FASTEST: @@ -543,7 +540,7 @@ vdev_raidz_math_init(void) #endif /* Finish initialization */ - atomic_swap_32(&zfs_vdev_raidz_impl_setting, user_sel_impl); + atomic_swap_32(&zfs_vdev_raidz_impl, user_sel_impl); raidz_math_initialized = B_TRUE; } @@ -582,7 +579,7 @@ static const struct { * If we are called before init(), user preference will be saved in * user_sel_impl, and applied in later init() call. This occurs when module * parameter is specified on module load. Otherwise, directly update - * zfs_vdev_raidz_impl_setting. + * zfs_vdev_raidz_impl. * * @val Name of raidz implementation to use * @param Unused. @@ -628,12 +625,9 @@ vdev_raidz_impl_set(const char *val) if (err == 0) { if (raidz_math_initialized) - atomic_swap_32(&zfs_vdev_raidz_impl_setting, impl); + atomic_swap_32(&zfs_vdev_raidz_impl, impl); else atomic_swap_32(&user_sel_impl, impl); -#if defined(__linux__) - zfs_vdev_raidz_impl = raidz_supp_impl[i]->name; -#endif } return (err); @@ -646,7 +640,7 @@ vdev_raidz_impl_get(char *buffer, size_t size) { int i, cnt = 0; char *fmt; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); ASSERT(raidz_math_initialized); @@ -668,3 +662,21 @@ vdev_raidz_impl_get(char *buffer, size_t size) } #endif + +#if defined(_KERNEL) && defined(__linux__) +static int +zfs_vdev_raidz_impl_set(const char *val, zfs_kernel_param_t *kp) +{ + return (vdev_raidz_impl_set(val)); +} + +static int +zfs_vdev_raidz_impl_get(char *buffer, zfs_kernel_param_t *kp) +{ + return (vdev_raidz_impl_get(buffer, PAGE_SIZE)); +} + +module_param_call(zfs_vdev_raidz_impl, zfs_vdev_raidz_impl_set, + zfs_vdev_raidz_impl_get, NULL, 0644); +MODULE_PARM_DESC(zfs_vdev_raidz_impl, "Select raidz implementation."); +#endif From b7275ef4d6df6745b36dc892a4c6d5f2507ce713 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 24 Jan 2025 07:14:05 -0700 Subject: [PATCH 04/10] Revert "Don't attempt to use the portable sysctl macros" This reverts commit ba180bdbe09dab24b44a8945320225991a2c194d. --- include/os/freebsd/spl/sys/mod_os.h | 3 +++ include/sys/vdev_impl.h | 2 ++ include/sys/vdev_raidz.h | 5 +++- module/os/freebsd/zfs/sysctl_os.c | 8 +------ module/os/linux/zfs/vdev_disk.c | 9 ++++++++ module/zfs/vdev.c | 4 ++++ module/zfs/vdev_raidz_math.c | 36 ++++++++++------------------- 7 files changed, 35 insertions(+), 32 deletions(-) diff --git a/include/os/freebsd/spl/sys/mod_os.h b/include/os/freebsd/spl/sys/mod_os.h index df7be6fc13f6..1479242de53b 100644 --- a/include/os/freebsd/spl/sys/mod_os.h +++ b/include/os/freebsd/spl/sys/mod_os.h @@ -94,6 +94,9 @@ #define param_set_max_auto_ashift_args(var) \ CTLTYPE_UINT, NULL, 0, param_set_max_auto_ashift, "IU" +#define param_set_raidz_impl_args(var) \ + CTLTYPE_STRING, NULL, 0, param_set_raidz_impl, "A" + #define spa_taskq_read_param_set_args(var) \ CTLTYPE_STRING, NULL, 0, spa_taskq_read_param, "A" diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index abd66b8abc96..46ca2859f991 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -62,6 +62,7 @@ struct abd; extern uint_t zfs_vdev_queue_depth_pct; extern uint_t zfs_vdev_def_queue_depth; extern uint_t zfs_vdev_async_write_max_active; +extern const char *zfs_vdev_raidz_impl; /* * Virtual device operations @@ -645,6 +646,7 @@ extern int vdev_obsolete_counts_are_precise(vdev_t *vd, boolean_t *are_precise); int vdev_checkpoint_sm_object(vdev_t *vd, uint64_t *sm_obj); void vdev_metaslab_group_create(vdev_t *vd); uint64_t vdev_best_ashift(uint64_t logical, uint64_t a, uint64_t b); +int param_set_raidz_impl(ZFS_MODULE_PARAM_ARGS); /* * Vdev ashift optimization tunables diff --git a/include/sys/vdev_raidz.h b/include/sys/vdev_raidz.h index b50bee946158..e8af9f3715f1 100644 --- a/include/sys/vdev_raidz.h +++ b/include/sys/vdev_raidz.h @@ -66,14 +66,17 @@ extern const zio_vsd_ops_t vdev_raidz_vsd_ops; /* * vdev_raidz_math interface */ +#if defined(__linux__) +extern const char *zfs_vdev_raidz_impl; +#endif void vdev_raidz_math_init(void); void vdev_raidz_math_fini(void); const struct raidz_impl_ops *vdev_raidz_math_get_ops(void); int vdev_raidz_math_generate(struct raidz_map *, struct raidz_row *); int vdev_raidz_math_reconstruct(struct raidz_map *, struct raidz_row *, const int *, const int *, const int); -int vdev_raidz_impl_get(char *buffer, size_t size); int vdev_raidz_impl_set(const char *); +int vdev_raidz_impl_get(char *buffer, size_t size); typedef struct vdev_raidz_expand { uint64_t vre_vdev_id; diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c index 109af1d6f6e8..0be71ffce022 100644 --- a/module/os/freebsd/zfs/sysctl_os.c +++ b/module/os/freebsd/zfs/sysctl_os.c @@ -679,7 +679,7 @@ param_set_deadman_failmode(SYSCTL_HANDLER_ARGS) return (-param_set_deadman_failmode_common(buf)); } -static int +int param_set_raidz_impl(SYSCTL_HANDLER_ARGS) { char *buf; @@ -808,12 +808,6 @@ SYSCTL_INT(_vfs_zfs, OID_AUTO, validate_skip, /* vdev_mirror.c */ -/* vdev_raidz_math.c */ - -SYSCTL_PROC(_vfs_zfs_vdev, OID_AUTO, raidz_impl, - CTLTYPE_STRING | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, - NULL, 0, param_set_raidz_impl, "IU", "select RAIDZ implementation"); - /* vdev_queue.c */ extern uint_t zfs_vdev_max_active; diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index e8bd513e6909..8f464bb9de9e 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1637,6 +1637,15 @@ param_set_max_auto_ashift(const char *buf, zfs_kernel_param_t *kp) return (0); } +int +param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) +{ + int error; + + error = vdev_raidz_impl_set(val); + return (error); +} + ZFS_MODULE_PARAM(zfs_vdev, zfs_vdev_, open_timeout_ms, UINT, ZMOD_RW, "Timeout before determining that a device is missing"); diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 5df2f77e5780..438b849ff704 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -6580,3 +6580,7 @@ ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, max_auto_ashift, param_set_max_auto_ashift, param_get_uint, ZMOD_RW, "Maximum ashift used when optimizing for logical -> physical sector " "size on new top-level vdevs"); + +ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, raidz_impl, + param_set_raidz_impl, param_get_charp, ZMOD_RW, + "RAIDZ implementation"); diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index b2eea4c083be..6eb017c29c25 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -81,8 +81,11 @@ static boolean_t raidz_math_initialized = B_FALSE; #define RAIDZ_IMPL_READ(i) (*(volatile uint32_t *) &(i)) -static uint32_t zfs_vdev_raidz_impl = IMPL_SCALAR; +static uint32_t zfs_vdev_raidz_impl_setting = IMPL_SCALAR; static uint32_t user_sel_impl = IMPL_FASTEST; +#if defined(__linux__) +const char *zfs_vdev_raidz_impl = "fastest"; +#endif /* Hold all supported implementations */ static size_t raidz_supp_impl_cnt = 0; @@ -111,7 +114,7 @@ vdev_raidz_math_get_ops(void) return (&vdev_raidz_scalar_impl); raidz_impl_ops_t *ops = NULL; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); switch (impl) { case IMPL_FASTEST: @@ -540,7 +543,7 @@ vdev_raidz_math_init(void) #endif /* Finish initialization */ - atomic_swap_32(&zfs_vdev_raidz_impl, user_sel_impl); + atomic_swap_32(&zfs_vdev_raidz_impl_setting, user_sel_impl); raidz_math_initialized = B_TRUE; } @@ -579,7 +582,7 @@ static const struct { * If we are called before init(), user preference will be saved in * user_sel_impl, and applied in later init() call. This occurs when module * parameter is specified on module load. Otherwise, directly update - * zfs_vdev_raidz_impl. + * zfs_vdev_raidz_impl_setting. * * @val Name of raidz implementation to use * @param Unused. @@ -625,9 +628,12 @@ vdev_raidz_impl_set(const char *val) if (err == 0) { if (raidz_math_initialized) - atomic_swap_32(&zfs_vdev_raidz_impl, impl); + atomic_swap_32(&zfs_vdev_raidz_impl_setting, impl); else atomic_swap_32(&user_sel_impl, impl); +#if defined(__linux__) + zfs_vdev_raidz_impl = raidz_supp_impl[i]->name; +#endif } return (err); @@ -640,7 +646,7 @@ vdev_raidz_impl_get(char *buffer, size_t size) { int i, cnt = 0; char *fmt; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); ASSERT(raidz_math_initialized); @@ -662,21 +668,3 @@ vdev_raidz_impl_get(char *buffer, size_t size) } #endif - -#if defined(_KERNEL) && defined(__linux__) -static int -zfs_vdev_raidz_impl_set(const char *val, zfs_kernel_param_t *kp) -{ - return (vdev_raidz_impl_set(val)); -} - -static int -zfs_vdev_raidz_impl_get(char *buffer, zfs_kernel_param_t *kp) -{ - return (vdev_raidz_impl_get(buffer, PAGE_SIZE)); -} - -module_param_call(zfs_vdev_raidz_impl, zfs_vdev_raidz_impl_set, - zfs_vdev_raidz_impl_get, NULL, 0644); -MODULE_PARM_DESC(zfs_vdev_raidz_impl, "Select raidz implementation."); -#endif From 6295c7e5532fa318f130474f46431d501d661a7f Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 24 Jan 2025 07:51:49 -0700 Subject: [PATCH 05/10] Replace param_get_charp with custom param_get_raidz_impl Signed-off-by: Alan Somers --- include/sys/vdev_impl.h | 3 +++ module/os/linux/zfs/vdev_disk.c | 6 ++++++ module/zfs/vdev.c | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index 46ca2859f991..93715a2811cd 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -646,6 +646,9 @@ extern int vdev_obsolete_counts_are_precise(vdev_t *vd, boolean_t *are_precise); int vdev_checkpoint_sm_object(vdev_t *vd, uint64_t *sm_obj); void vdev_metaslab_group_create(vdev_t *vd); uint64_t vdev_best_ashift(uint64_t logical, uint64_t a, uint64_t b); +#if defined(__linux__) +int param_get_raidz_impl(char *buf, zfs_kernel_param_t *kp); +#endif int param_set_raidz_impl(ZFS_MODULE_PARAM_ARGS); /* diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index 8f464bb9de9e..ef00f0282ef9 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1637,6 +1637,12 @@ param_set_max_auto_ashift(const char *buf, zfs_kernel_param_t *kp) return (0); } +int +param_get_raidz_impl(char *buf, zfs_kernel_param_t *kp) +{ + return vdev_raidz_impl_get(buf, PAGE_SIZE); +} + int param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) { diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 438b849ff704..96621b2bd657 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -6582,5 +6582,5 @@ ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, max_auto_ashift, "size on new top-level vdevs"); ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, raidz_impl, - param_set_raidz_impl, param_get_charp, ZMOD_RW, + param_set_raidz_impl, param_get_raidz_impl, ZMOD_RW, "RAIDZ implementation"); From f957fd46791130799716a75f6093d290c28ea6aa Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 24 Jan 2025 10:12:30 -0700 Subject: [PATCH 06/10] Move the Linux-specific vdev_raidz stuff to a more appropriate file Signed-off-by: Alan Somers --- include/sys/vdev_impl.h | 1 - module/Kbuild.in | 1 + module/os/linux/zfs/vdev_disk.c | 15 ------------ module/os/linux/zfs/vdev_raidz.c | 42 ++++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 module/os/linux/zfs/vdev_raidz.c diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index 93715a2811cd..d45a5913dc0f 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -62,7 +62,6 @@ struct abd; extern uint_t zfs_vdev_queue_depth_pct; extern uint_t zfs_vdev_def_queue_depth; extern uint_t zfs_vdev_async_write_max_active; -extern const char *zfs_vdev_raidz_impl; /* * Virtual device operations diff --git a/module/Kbuild.in b/module/Kbuild.in index fc14d5cb535e..5190afc506f9 100644 --- a/module/Kbuild.in +++ b/module/Kbuild.in @@ -447,6 +447,7 @@ ZFS_OBJS_OS := \ trace.o \ vdev_disk.o \ vdev_file.o \ + vdev_raidz.o \ vdev_label_os.o \ zfs_acl.o \ zfs_ctldir.o \ diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index ef00f0282ef9..e8bd513e6909 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -1637,21 +1637,6 @@ param_set_max_auto_ashift(const char *buf, zfs_kernel_param_t *kp) return (0); } -int -param_get_raidz_impl(char *buf, zfs_kernel_param_t *kp) -{ - return vdev_raidz_impl_get(buf, PAGE_SIZE); -} - -int -param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) -{ - int error; - - error = vdev_raidz_impl_set(val); - return (error); -} - ZFS_MODULE_PARAM(zfs_vdev, zfs_vdev_, open_timeout_ms, UINT, ZMOD_RW, "Timeout before determining that a device is missing"); diff --git a/module/os/linux/zfs/vdev_raidz.c b/module/os/linux/zfs/vdev_raidz.c new file mode 100644 index 000000000000..00846b4f8e1c --- /dev/null +++ b/module/os/linux/zfs/vdev_raidz.c @@ -0,0 +1,42 @@ +/* + * 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 https://opensource.org/licenses/CDDL-1.0. + * 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) 2025 ConnectWise */ + +#include +#include +#include +#include +#include + +int +param_get_raidz_impl(char *buf, zfs_kernel_param_t *kp) +{ + return vdev_raidz_impl_get(buf, PAGE_SIZE); +} + +int +param_set_raidz_impl(const char *val, zfs_kernel_param_t *kp) +{ + int error; + + error = vdev_raidz_impl_set(val); + return (error); +} From d5ec5fc2015724173887b95e5cb963093c726f7c Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 24 Jan 2025 10:30:00 -0700 Subject: [PATCH 07/10] Don't assign the zfs_vdev_raidz_impl variable Signed-off-by: Alan Somers --- module/zfs/vdev_raidz_math.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index 6eb017c29c25..0a5d5e46a9ee 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -84,7 +84,8 @@ static boolean_t raidz_math_initialized = B_FALSE; static uint32_t zfs_vdev_raidz_impl_setting = IMPL_SCALAR; static uint32_t user_sel_impl = IMPL_FASTEST; #if defined(__linux__) -const char *zfs_vdev_raidz_impl = "fastest"; +/* Required, but not used, by ZFS_MODULE_PARAM_CALL */ +const char *zfs_vdev_raidz_impl = NULL; #endif /* Hold all supported implementations */ @@ -631,9 +632,6 @@ vdev_raidz_impl_set(const char *val) atomic_swap_32(&zfs_vdev_raidz_impl_setting, impl); else atomic_swap_32(&user_sel_impl, impl); -#if defined(__linux__) - zfs_vdev_raidz_impl = raidz_supp_impl[i]->name; -#endif } return (err); From 7416ac36dc1bc35cfbe95df99451080fabc5f2bd Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 24 Jan 2025 11:17:59 -0700 Subject: [PATCH 08/10] Unrename zfs_vdev_raidz_impl The ZFS_MODULE_PARAM_CALL macro still requires a symbol by this name, but apparently doesn't care about its type. Signed-off-by: Alan Somers --- include/sys/vdev_raidz.h | 5 ++--- module/zfs/vdev_raidz_math.c | 16 ++++++---------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/include/sys/vdev_raidz.h b/include/sys/vdev_raidz.h index e8af9f3715f1..ed042aedbdbc 100644 --- a/include/sys/vdev_raidz.h +++ b/include/sys/vdev_raidz.h @@ -66,9 +66,8 @@ extern const zio_vsd_ops_t vdev_raidz_vsd_ops; /* * vdev_raidz_math interface */ -#if defined(__linux__) -extern const char *zfs_vdev_raidz_impl; -#endif +/* Required, but not used, by ZFS_MODULE_PARAM_CALL */ +extern uint32_t zfs_vdev_raidz_impl; void vdev_raidz_math_init(void); void vdev_raidz_math_fini(void); const struct raidz_impl_ops *vdev_raidz_math_get_ops(void); diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c index 0a5d5e46a9ee..340d32b61bf8 100644 --- a/module/zfs/vdev_raidz_math.c +++ b/module/zfs/vdev_raidz_math.c @@ -81,12 +81,8 @@ static boolean_t raidz_math_initialized = B_FALSE; #define RAIDZ_IMPL_READ(i) (*(volatile uint32_t *) &(i)) -static uint32_t zfs_vdev_raidz_impl_setting = IMPL_SCALAR; +uint32_t zfs_vdev_raidz_impl = IMPL_SCALAR; static uint32_t user_sel_impl = IMPL_FASTEST; -#if defined(__linux__) -/* Required, but not used, by ZFS_MODULE_PARAM_CALL */ -const char *zfs_vdev_raidz_impl = NULL; -#endif /* Hold all supported implementations */ static size_t raidz_supp_impl_cnt = 0; @@ -115,7 +111,7 @@ vdev_raidz_math_get_ops(void) return (&vdev_raidz_scalar_impl); raidz_impl_ops_t *ops = NULL; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); switch (impl) { case IMPL_FASTEST: @@ -544,7 +540,7 @@ vdev_raidz_math_init(void) #endif /* Finish initialization */ - atomic_swap_32(&zfs_vdev_raidz_impl_setting, user_sel_impl); + atomic_swap_32(&zfs_vdev_raidz_impl, user_sel_impl); raidz_math_initialized = B_TRUE; } @@ -583,7 +579,7 @@ static const struct { * If we are called before init(), user preference will be saved in * user_sel_impl, and applied in later init() call. This occurs when module * parameter is specified on module load. Otherwise, directly update - * zfs_vdev_raidz_impl_setting. + * zfs_vdev_raidz_impl. * * @val Name of raidz implementation to use * @param Unused. @@ -629,7 +625,7 @@ vdev_raidz_impl_set(const char *val) if (err == 0) { if (raidz_math_initialized) - atomic_swap_32(&zfs_vdev_raidz_impl_setting, impl); + atomic_swap_32(&zfs_vdev_raidz_impl, impl); else atomic_swap_32(&user_sel_impl, impl); } @@ -644,7 +640,7 @@ vdev_raidz_impl_get(char *buffer, size_t size) { int i, cnt = 0; char *fmt; - const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl_setting); + const uint32_t impl = RAIDZ_IMPL_READ(zfs_vdev_raidz_impl); ASSERT(raidz_math_initialized); From 3274093f700eb4a04aa25236a0a3d4c3b2771967 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 24 Jan 2025 12:03:13 -0700 Subject: [PATCH 09/10] Style, and increase buffer size Signed-off-by: Alan Somers --- module/os/freebsd/zfs/sysctl_os.c | 7 ++++--- module/os/linux/zfs/vdev_raidz.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c index 0be71ffce022..8e3d1ceba0bd 100644 --- a/module/os/freebsd/zfs/sysctl_os.c +++ b/module/os/freebsd/zfs/sysctl_os.c @@ -682,14 +682,15 @@ param_set_deadman_failmode(SYSCTL_HANDLER_ARGS) int param_set_raidz_impl(SYSCTL_HANDLER_ARGS) { + const size_t bufsize = 128; char *buf; int rc; - buf = malloc(64, M_SOLARIS, M_NOWAIT | M_ZERO); + buf = malloc(bufsize, M_SOLARIS, M_NOWAIT | M_ZERO); if (req->newptr == NULL) - vdev_raidz_impl_get(buf, 64); + vdev_raidz_impl_get(buf, bufsize); - rc = sysctl_handle_string(oidp, buf, 64, req); + rc = sysctl_handle_string(oidp, buf, bufsize, req); if (rc || req->newptr == NULL) { free(buf, M_SOLARIS); return (rc); diff --git a/module/os/linux/zfs/vdev_raidz.c b/module/os/linux/zfs/vdev_raidz.c index 00846b4f8e1c..0b34ca52fb90 100644 --- a/module/os/linux/zfs/vdev_raidz.c +++ b/module/os/linux/zfs/vdev_raidz.c @@ -29,7 +29,7 @@ int param_get_raidz_impl(char *buf, zfs_kernel_param_t *kp) { - return vdev_raidz_impl_get(buf, PAGE_SIZE); + return (vdev_raidz_impl_get(buf, PAGE_SIZE)); } int From 8a90cf253d2c1ec63a601e0ab7702db1136bd4fc Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Mon, 27 Jan 2025 09:53:40 -0700 Subject: [PATCH 10/10] Use M_WAITOK Signed-off-by: Alan Somers --- module/os/freebsd/zfs/sysctl_os.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c index 8e3d1ceba0bd..bddb25a07204 100644 --- a/module/os/freebsd/zfs/sysctl_os.c +++ b/module/os/freebsd/zfs/sysctl_os.c @@ -686,7 +686,7 @@ param_set_raidz_impl(SYSCTL_HANDLER_ARGS) char *buf; int rc; - buf = malloc(bufsize, M_SOLARIS, M_NOWAIT | M_ZERO); + buf = malloc(bufsize, M_SOLARIS, M_WAITOK | M_ZERO); if (req->newptr == NULL) vdev_raidz_impl_get(buf, bufsize);