Skip to content

Commit d56d0ab

Browse files
committed
Cleanup: 64-bit kernel module parameters should use fixed width types
Various module parameters such as `zfs_arc_max` were originally `uint64_t` on OpenSolaris/Illumos, but were changed to `unsigned long` for Linux compatibility because Linux's kernel default module parameter implementation did not support 64-bit types on 32-bit platforms. This caused problems when porting OpenZFS to Windows because its LLP64 memory model made `unsigned long` a 32-bit type on 64-bit, which created the undesireable situation that parameters that should accept 64-bit values could not on 64-bit Windows. Upon inspection, it turns out that the Linux kernel module parameter interface is extensible, such that we are allowed to define our own types. Rather than maintaining the original type change via hacks to to continue shrinking module parameters on 32-bit Linux, we implement support for 64-bit module parameters on Linux. After doing a review of all 64-bit kernel parameters (found via the man page and also proposed changes by Andrew Innes), the kernel module parameters fell into a few groups: Parameters that were originally 64-bit on Illumos: * dbuf_cache_max_bytes * dbuf_metadata_cache_max_bytes * l2arc_feed_min_ms * l2arc_feed_secs * l2arc_headroom * l2arc_headroom_boost * l2arc_write_boost * l2arc_write_max * metaslab_aliquot * metaslab_force_ganging * zfetch_array_rd_sz * zfs_arc_max * zfs_arc_meta_limit * zfs_arc_meta_min * zfs_arc_min * zfs_async_block_max_blocks * zfs_condense_max_obsolete_bytes * zfs_condense_min_mapping_bytes * zfs_deadman_checktime_ms * zfs_deadman_synctime_ms * zfs_initialize_chunk_size * zfs_initialize_value * zfs_lua_max_instrlimit * zfs_lua_max_memlimit * zil_slog_bulk Parameters that were originally 32-bit on Illumos: * zfs_per_txg_dirty_frees_percent - made 32-bit again Parameters that were originally `ssize_t` on Illumos: * zfs_immediate_write_sz Note that `ssize_t` is `int32_t` on 32-bit and `int64_t` on 64-bit. It has been upgraded to 64-bit. Parameters that were `long`/`unsigned long` because of Linux/FreeBSD influence: * l2arc_rebuild_blocks_min_l2size * zfs_key_max_salt_uses * zfs_max_log_walking * zfs_max_logsm_summary_length * zfs_metaslab_max_size_cache_sec * zfs_min_metaslabs_to_flush * zfs_multihost_interval * zfs_unflushed_log_block_max * zfs_unflushed_log_block_min * zfs_unflushed_log_block_pct * zfs_unflushed_max_mem_amt * zfs_unflushed_max_mem_ppm New parameters that do not exist in Illumos: * l2arc_trim_ahead * vdev_file_logical_ashift * vdev_file_physical_ashift * zfs_arc_dnode_limit * zfs_arc_dnode_limit_percent * zfs_arc_dnode_reduce_percent * zfs_arc_meta_limit_percent * zfs_arc_sys_free * zfs_deadman_ziotime_ms * zfs_delete_blocks * zfs_history_output_max * zfs_livelist_max_entries * zfs_max_async_dedup_frees * zfs_max_nvlist_src_size * zfs_rebuild_max_segment * zfs_rebuild_vdev_limit * zfs_unflushed_log_txg_max * zfs_vdev_max_auto_ashift * zfs_vdev_min_auto_ashift * zfs_vnops_read_chunk_size * zvol_max_discard_blocks Rather than clutter the lists with commentary, the module parameters that need comments are repeated below. A few parameters were defined in Linux/FreeBSD specific code, where the use of ulong/long is not an issue for portability, so we leave them alone: * zfs_delete_blocks * zfs_key_max_salt_uses * zvol_max_discard_blocks The documentation for a few parameters was found to be incorrect: * zfs_deadman_checktime_ms - incorrectly documented as int * zfs_delete_blocks - not documented as Linux only * zfs_history_output_max - incorrectly documented as int * zfs_vnops_read_chunk_size - incorrectly documented as long * zvol_max_discard_blocks - incorrectly documented as ulong The documentation for these has been fixed, alongside the changes to document the switch to fixed width types. In addition, several kernel module parameters were percentages or held ashift values, so being 64-bit never made sense for them. They have been downgraded to 32-bit: * vdev_file_logical_ashift * vdev_file_physical_ashift * zfs_arc_dnode_limit_percent * zfs_arc_dnode_reduce_percent * zfs_arc_meta_limit_percent * zfs_unflushed_log_block_pct * zfs_vdev_max_auto_ashift * zfs_vdev_min_auto_ashift Of special note are `zfs_vdev_max_auto_ashift` and `zfs_vdev_min_auto_ashift`, which were already defined as `uint64_t`, and passed to the kernel as `ulong`. This is inherently buggy on big endian 32-bit Linux/FreeBSD, since the values would not be written to the correct locations. Lastly, a code comment suggests that `zfs_arc_sys_free` is Linux-specific, but there is nothing to indicate to me that it is Linux-specific. Nothing was done about that. Original-patch-by: Andrew Innes <andrew.c12@gmail.com> Original-patch-by: Jorgen Lundman <lundman@lundman.net> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
1 parent 72c99dc commit d56d0ab

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+349
-264
lines changed

cmd/ztest.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ static const ztest_shared_opts_t ztest_opts_defaults = {
252252

253253
extern uint64_t metaslab_force_ganging;
254254
extern uint64_t metaslab_df_alloc_threshold;
255-
extern unsigned long zfs_deadman_synctime_ms;
255+
extern uint64_t zfs_deadman_synctime_ms;
256256
extern uint_t metaslab_preload_limit;
257257
extern int zfs_compressed_arc_enabled;
258258
extern int zfs_abd_scatter_enabled;

config/kernel-mod-param.m4

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_MODULE_PARAM_CALL_CONST], [
1717
return (0);
1818
}
1919
20-
module_param_call(p, param_set, param_get, NULL, 0644);
20+
const struct kernel_param_ops spl_param_ops = {
21+
.set = param_set,
22+
.get = param_get,
23+
};
2124
],[])
2225
])
2326

include/os/freebsd/spl/sys/mod_os.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,17 @@
5252

5353
#define ZFS_MODULE_VIRTUAL_PARAM_CALL ZFS_MODULE_PARAM_CALL
5454

55-
#define param_set_arc_long_args(var) \
56-
CTLTYPE_ULONG, &var, 0, param_set_arc_long, "LU"
55+
#define param_set_arc_u64_args(var) \
56+
CTLTYPE_U64, &var, 0, param_set_arc_u64, "QU"
5757

5858
#define param_set_arc_int_args(var) \
5959
CTLTYPE_INT, &var, 0, param_set_arc_int, "I"
6060

6161
#define param_set_arc_min_args(var) \
62-
CTLTYPE_ULONG, NULL, 0, param_set_arc_min, "LU"
62+
CTLTYPE_U64, NULL, 0, param_set_arc_min, "QU"
6363

6464
#define param_set_arc_max_args(var) \
65-
CTLTYPE_ULONG, NULL, 0, param_set_arc_max, "LU"
65+
CTLTYPE_U64, NULL, 0, param_set_arc_max, "QU"
6666

6767
#define param_set_arc_free_target_args(var) \
6868
CTLTYPE_UINT, NULL, 0, param_set_arc_free_target, "IU"
@@ -74,22 +74,22 @@
7474
CTLTYPE_STRING, NULL, 0, param_set_deadman_failmode, "A"
7575

7676
#define param_set_deadman_synctime_args(var) \
77-
CTLTYPE_ULONG, NULL, 0, param_set_deadman_synctime, "LU"
77+
CTLTYPE_U64, NULL, 0, param_set_deadman_synctime, "QU"
7878

7979
#define param_set_deadman_ziotime_args(var) \
80-
CTLTYPE_ULONG, NULL, 0, param_set_deadman_ziotime, "LU"
80+
CTLTYPE_U64, NULL, 0, param_set_deadman_ziotime, "QU"
8181

8282
#define param_set_multihost_interval_args(var) \
83-
CTLTYPE_ULONG, NULL, 0, param_set_multihost_interval, "LU"
83+
CTLTYPE_U64, NULL, 0, param_set_multihost_interval, "QU"
8484

8585
#define param_set_slop_shift_args(var) \
8686
CTLTYPE_INT, NULL, 0, param_set_slop_shift, "I"
8787

8888
#define param_set_min_auto_ashift_args(var) \
89-
CTLTYPE_U64, NULL, 0, param_set_min_auto_ashift, "QU"
89+
CTLTYPE_UINT, NULL, 0, param_set_min_auto_ashift, "IU"
9090

9191
#define param_set_max_auto_ashift_args(var) \
92-
CTLTYPE_U64, NULL, 0, param_set_max_auto_ashift, "QU"
92+
CTLTYPE_UINT, NULL, 0, param_set_max_auto_ashift, "IU"
9393

9494
#define fletcher_4_param_set_args(var) \
9595
CTLTYPE_STRING, NULL, 0, fletcher_4_param, "A"

include/os/linux/kernel/linux/mod_compat.h

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,15 @@
3030
#include <linux/module.h>
3131
#include <linux/moduleparam.h>
3232

33+
/*
34+
* Despite constifying struct kernel_param_ops, some older kernels define a
35+
* `__check_old_set_param()` function in their headers that checks for a
36+
* non-constified `->set()`. This has long been fixed in Linux mainline, but
37+
* since we support older kernels, we workaround it by using a preprocessor
38+
* definition to disable it.
39+
*/
40+
#define __check_old_set_param(set) (set)
41+
3342
/* Grsecurity kernel API change */
3443
#ifdef MODULE_PARAM_CALL_CONST
3544
typedef const struct kernel_param zfs_kernel_param_t;
@@ -40,14 +49,6 @@ typedef struct kernel_param zfs_kernel_param_t;
4049
#define ZMOD_RW 0644
4150
#define ZMOD_RD 0444
4251

43-
#define INT int
44-
#define LONG long
45-
/* BEGIN CSTYLED */
46-
#define UINT uint
47-
#define ULONG ulong
48-
/* END CSTYLED */
49-
#define STRING charp
50-
5152
enum scope_prefix_types {
5253
zfs,
5354
zfs_arc,
@@ -80,6 +81,50 @@ enum scope_prefix_types {
8081
zfs_zil
8182
};
8283

84+
/*
85+
* While we define our own s64/u64 types, there is no reason to reimplement the
86+
* existing Linux kernel types, so we use the preprocessor to remap our
87+
* "custom" implementations to the kernel ones. This is done because the CPP
88+
* does not allow us to write conditional definitions. The fourth definition
89+
* exists because the CPP will not allow us to replace things like INT with int
90+
* before string concatenation.
91+
*/
92+
93+
#define spl_param_set_int param_set_int
94+
#define spl_param_get_int param_get_int
95+
#define spl_param_ops_int param_ops_int
96+
#define spl_param_ops_INT param_ops_int
97+
98+
#define spl_param_set_long param_set_long
99+
#define spl_param_get_long param_get_long
100+
#define spl_param_ops_long param_ops_long
101+
#define spl_param_ops_LONG param_ops_long
102+
103+
#define spl_param_set_uint param_set_uint
104+
#define spl_param_get_uint param_get_uint
105+
#define spl_param_ops_uint param_ops_uint
106+
#define spl_param_ops_UINT param_ops_uint
107+
108+
#define spl_param_set_ulong param_set_ulong
109+
#define spl_param_get_ulong param_get_ulong
110+
#define spl_param_ops_ulong param_ops_ulong
111+
#define spl_param_ops_ULONG param_ops_ulong
112+
113+
#define spl_param_set_charp param_set_charp
114+
#define spl_param_get_charp param_get_charp
115+
#define spl_param_ops_charp param_ops_charp
116+
#define spl_param_ops_STRING param_ops_charp
117+
118+
int spl_param_set_s64(const char *val, zfs_kernel_param_t *kp);
119+
extern int spl_param_get_s64(char *buffer, zfs_kernel_param_t *kp);
120+
extern const struct kernel_param_ops spl_param_ops_s64;
121+
#define spl_param_ops_S64 spl_param_ops_s64
122+
123+
extern int spl_param_set_u64(const char *val, zfs_kernel_param_t *kp);
124+
extern int spl_param_get_u64(char *buffer, zfs_kernel_param_t *kp);
125+
extern const struct kernel_param_ops spl_param_ops_u64;
126+
#define spl_param_ops_U64 spl_param_ops_u64
127+
83128
/*
84129
* Declare a module parameter / sysctl node
85130
*
@@ -112,7 +157,8 @@ enum scope_prefix_types {
112157
_Static_assert( \
113158
sizeof (scope_prefix) == sizeof (enum scope_prefix_types), \
114159
"" #scope_prefix " size mismatch with enum scope_prefix_types"); \
115-
module_param(name_prefix ## name, type, perm); \
160+
module_param_cb(name_prefix ## name, &spl_param_ops_ ## type, \
161+
&name_prefix ## name, perm); \
116162
MODULE_PARM_DESC(name_prefix ## name, desc)
117163

118164
/*

include/sys/arc_impl.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -985,8 +985,8 @@ extern arc_state_t ARC_mfu;
985985
extern arc_state_t ARC_mru;
986986
extern uint_t zfs_arc_pc_percent;
987987
extern uint_t arc_lotsfree_percent;
988-
extern unsigned long zfs_arc_min;
989-
extern unsigned long zfs_arc_max;
988+
extern uint64_t zfs_arc_min;
989+
extern uint64_t zfs_arc_max;
990990

991991
extern void arc_reduce_target_size(int64_t to_free);
992992
extern boolean_t arc_reclaim_needed(void);
@@ -1003,7 +1003,7 @@ extern void arc_tuning_update(boolean_t);
10031003
extern void arc_register_hotplug(void);
10041004
extern void arc_unregister_hotplug(void);
10051005

1006-
extern int param_set_arc_long(ZFS_MODULE_PARAM_ARGS);
1006+
extern int param_set_arc_u64(ZFS_MODULE_PARAM_ARGS);
10071007
extern int param_set_arc_int(ZFS_MODULE_PARAM_ARGS);
10081008
extern int param_set_arc_min(ZFS_MODULE_PARAM_ARGS);
10091009
extern int param_set_arc_max(ZFS_MODULE_PARAM_ARGS);

include/sys/dmu_zfetch.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
extern "C" {
3737
#endif
3838

39-
extern unsigned long zfetch_array_rd_sz;
39+
extern uint64_t zfetch_array_rd_sz;
4040

4141
struct dnode; /* so we can reference dnode */
4242

include/sys/dsl_deadlist.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ typedef struct livelist_condense_entry {
8484
boolean_t cancelled;
8585
} livelist_condense_entry_t;
8686

87-
extern unsigned long zfs_livelist_max_entries;
87+
extern uint64_t zfs_livelist_max_entries;
8888
extern int zfs_livelist_min_percent_shared;
8989

9090
typedef int deadlist_iter_t(void *args, dsl_deadlist_entry_t *dle);

include/sys/dsl_pool.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ struct dsl_scan;
5757
struct dsl_crypto_params;
5858
struct dsl_deadlist;
5959

60-
extern unsigned long zfs_dirty_data_max;
61-
extern unsigned long zfs_dirty_data_max_max;
62-
extern unsigned long zfs_wrlog_data_max;
60+
extern uint64_t zfs_dirty_data_max;
61+
extern uint64_t zfs_dirty_data_max_max;
62+
extern uint64_t zfs_wrlog_data_max;
6363
extern uint_t zfs_dirty_data_max_percent;
6464
extern uint_t zfs_dirty_data_max_max_percent;
6565
extern uint_t zfs_delay_min_dirty_percent;
66-
extern unsigned long zfs_delay_scale;
66+
extern uint64_t zfs_delay_scale;
6767

6868
/* These macros are for indexing into the zfs_all_blkstats_t. */
6969
#define DMU_OT_DEFERRED DMU_OT_NONE

include/sys/mmp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ extern void mmp_signal_all_threads(void);
6464

6565
/* Global tuning */
6666
extern int param_set_multihost_interval(ZFS_MODULE_PARAM_ARGS);
67-
extern ulong_t zfs_multihost_interval;
67+
extern uint64_t zfs_multihost_interval;
6868
extern uint_t zfs_multihost_fail_intervals;
6969
extern uint_t zfs_multihost_import_intervals;
7070

include/sys/spa.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,9 +1218,9 @@ int param_set_deadman_failmode(ZFS_MODULE_PARAM_ARGS);
12181218

12191219
extern spa_mode_t spa_mode_global;
12201220
extern int zfs_deadman_enabled;
1221-
extern unsigned long zfs_deadman_synctime_ms;
1222-
extern unsigned long zfs_deadman_ziotime_ms;
1223-
extern unsigned long zfs_deadman_checktime_ms;
1221+
extern uint64_t zfs_deadman_synctime_ms;
1222+
extern uint64_t zfs_deadman_ziotime_ms;
1223+
extern uint64_t zfs_deadman_checktime_ms;
12241224

12251225
extern kmem_cache_t *zio_buf_cache[];
12261226
extern kmem_cache_t *zio_data_buf_cache[];

0 commit comments

Comments
 (0)