aboutsummaryrefslogtreecommitdiffstats
path: root/module/os
diff options
context:
space:
mode:
authorRichard Yao <[email protected]>2022-09-27 19:42:41 -0400
committerGitHub <[email protected]>2022-09-27 16:42:41 -0700
commitfdc2d303710416868a05084e984fd8f231e948bd (patch)
tree058a29de2a9d3c998f008b626dd63951487f136e /module/os
parent7584fbe846b4127d5a16c5967a005e8f5c1e7b08 (diff)
Cleanup: Specify unsignedness on things that should not be signed
In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables than uint32_t because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t. Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Neal Gompa <[email protected]> Signed-off-by: Richard Yao <[email protected]> Closes #13875
Diffstat (limited to 'module/os')
-rw-r--r--module/os/freebsd/zfs/arc_os.c8
-rw-r--r--module/os/freebsd/zfs/sysctl_os.c22
-rw-r--r--module/os/freebsd/zfs/zfs_debug.c16
-rw-r--r--module/os/linux/spl/spl-taskq.c6
-rw-r--r--module/os/linux/zfs/arc_os.c2
-rw-r--r--module/os/linux/zfs/zfs_debug.c18
6 files changed, 38 insertions, 34 deletions
diff --git a/module/os/freebsd/zfs/arc_os.c b/module/os/freebsd/zfs/arc_os.c
index 30e96a889..dfe5c3d31 100644
--- a/module/os/freebsd/zfs/arc_os.c
+++ b/module/os/freebsd/zfs/arc_os.c
@@ -138,7 +138,7 @@ arc_default_max(uint64_t min, uint64_t allmem)
static void
arc_prune_task(void *arg)
{
- int64_t nr_scan = (intptr_t)arg;
+ uint64_t nr_scan = (uintptr_t)arg;
arc_reduce_target_size(ptob(nr_scan));
@@ -168,12 +168,12 @@ arc_prune_task(void *arg)
* for releasing it once the registered arc_prune_func_t has completed.
*/
void
-arc_prune_async(int64_t adjust)
+arc_prune_async(uint64_t adjust)
{
#ifndef __LP64__
- if (adjust > INTPTR_MAX)
- adjust = INTPTR_MAX;
+ if (adjust > UINTPTR_MAX)
+ adjust = UINTPTR_MAX;
#endif
taskq_dispatch(arc_prune_taskq, arc_prune_task,
(void *)(intptr_t)adjust, TQ_SLEEP);
diff --git a/module/os/freebsd/zfs/sysctl_os.c b/module/os/freebsd/zfs/sysctl_os.c
index 4d908381c..980bb1c0f 100644
--- a/module/os/freebsd/zfs/sysctl_os.c
+++ b/module/os/freebsd/zfs/sysctl_os.c
@@ -514,19 +514,19 @@ SYSCTL_INT(_vfs_zfs_metaslab, OID_AUTO, sm_blksz_with_log,
* space map representation must be before we compact it on-disk.
* Values should be greater than or equal to 100.
*/
-extern int zfs_condense_pct;
+extern uint_t zfs_condense_pct;
/* BEGIN CSTYLED */
-SYSCTL_INT(_vfs_zfs, OID_AUTO, condense_pct,
+SYSCTL_UINT(_vfs_zfs, OID_AUTO, condense_pct,
CTLFLAG_RWTUN, &zfs_condense_pct, 0,
"Condense on-disk spacemap when it is more than this many percents"
" of in-memory counterpart");
/* END CSTYLED */
-extern int zfs_remove_max_segment;
+extern uint_t zfs_remove_max_segment;
/* BEGIN CSTYLED */
-SYSCTL_INT(_vfs_zfs, OID_AUTO, remove_max_segment,
+SYSCTL_UINT(_vfs_zfs, OID_AUTO, remove_max_segment,
CTLFLAG_RWTUN, &zfs_remove_max_segment, 0,
"Largest contiguous segment ZFS will attempt to allocate when removing"
" a device");
@@ -561,10 +561,10 @@ SYSCTL_QUAD(_vfs_zfs_metaslab, OID_AUTO, df_alloc_threshold,
* Once the space map's free space drops below this level we dynamically
* switch to using best-fit allocations.
*/
-extern int metaslab_df_free_pct;
+extern uint_t metaslab_df_free_pct;
/* BEGIN CSTYLED */
-SYSCTL_INT(_vfs_zfs_metaslab, OID_AUTO, df_free_pct,
+SYSCTL_UINT(_vfs_zfs_metaslab, OID_AUTO, df_free_pct,
CTLFLAG_RWTUN, &metaslab_df_free_pct, 0,
"The minimum free space, in percent, which must be available in a"
" space map to continue allocations in a first-fit fashion");
@@ -584,10 +584,10 @@ SYSCTL_INT(_vfs_zfs_metaslab, OID_AUTO, load_pct,
/*
* Max number of metaslabs per group to preload.
*/
-extern int metaslab_preload_limit;
+extern uint_t metaslab_preload_limit;
/* BEGIN CSTYLED */
-SYSCTL_INT(_vfs_zfs_metaslab, OID_AUTO, preload_limit,
+SYSCTL_UINT(_vfs_zfs_metaslab, OID_AUTO, preload_limit,
CTLFLAG_RWTUN, &metaslab_preload_limit, 0,
"Max number of metaslabs per group to preload");
/* END CSTYLED */
@@ -852,7 +852,7 @@ SYSCTL_INT(_vfs_zfs, OID_AUTO, validate_skip,
/* vdev_queue.c */
-extern uint32_t zfs_vdev_max_active;
+extern uint_t zfs_vdev_max_active;
/* BEGIN CSTYLED */
SYSCTL_UINT(_vfs_zfs, OID_AUTO, top_maxinflight,
@@ -861,10 +861,10 @@ SYSCTL_UINT(_vfs_zfs, OID_AUTO, top_maxinflight,
" (LEGACY)");
/* END CSTYLED */
-extern int zfs_vdev_def_queue_depth;
+extern uint_t zfs_vdev_def_queue_depth;
/* BEGIN CSTYLED */
-SYSCTL_INT(_vfs_zfs_vdev, OID_AUTO, def_queue_depth,
+SYSCTL_UINT(_vfs_zfs_vdev, OID_AUTO, def_queue_depth,
CTLFLAG_RWTUN, &zfs_vdev_def_queue_depth, 0,
"Default queue depth for each allocator");
/* END CSTYLED */
diff --git a/module/os/freebsd/zfs/zfs_debug.c b/module/os/freebsd/zfs/zfs_debug.c
index 32fb5a872..abb3c0033 100644
--- a/module/os/freebsd/zfs/zfs_debug.c
+++ b/module/os/freebsd/zfs/zfs_debug.c
@@ -29,14 +29,14 @@
typedef struct zfs_dbgmsg {
list_node_t zdm_node;
time_t zdm_timestamp;
- int zdm_size;
+ uint_t zdm_size;
char zdm_msg[1]; /* variable length allocation */
} zfs_dbgmsg_t;
static list_t zfs_dbgmsgs;
-static int zfs_dbgmsg_size = 0;
+static uint_t zfs_dbgmsg_size = 0;
static kmutex_t zfs_dbgmsgs_lock;
-int zfs_dbgmsg_maxsize = 4<<20; /* 4MB */
+uint_t zfs_dbgmsg_maxsize = 4<<20; /* 4MB */
static kstat_t *zfs_dbgmsg_kstat;
/*
@@ -88,10 +88,10 @@ zfs_dbgmsg_addr(kstat_t *ksp, loff_t n)
}
static void
-zfs_dbgmsg_purge(int max_size)
+zfs_dbgmsg_purge(uint_t max_size)
{
zfs_dbgmsg_t *zdm;
- int size;
+ uint_t size;
ASSERT(MUTEX_HELD(&zfs_dbgmsgs_lock));
@@ -155,7 +155,7 @@ void
__zfs_dbgmsg(char *buf)
{
zfs_dbgmsg_t *zdm;
- int size;
+ uint_t size;
DTRACE_PROBE1(zfs__dbgmsg, char *, buf);
@@ -168,7 +168,7 @@ __zfs_dbgmsg(char *buf)
mutex_enter(&zfs_dbgmsgs_lock);
list_insert_tail(&zfs_dbgmsgs, zdm);
zfs_dbgmsg_size += size;
- zfs_dbgmsg_purge(MAX(zfs_dbgmsg_maxsize, 0));
+ zfs_dbgmsg_purge(zfs_dbgmsg_maxsize);
mutex_exit(&zfs_dbgmsgs_lock);
}
@@ -248,5 +248,5 @@ zfs_dbgmsg_print(const char *tag)
ZFS_MODULE_PARAM(zfs, zfs_, dbgmsg_enable, INT, ZMOD_RW,
"Enable ZFS debug message log");
-ZFS_MODULE_PARAM(zfs, zfs_, dbgmsg_maxsize, INT, ZMOD_RW,
+ZFS_MODULE_PARAM(zfs, zfs_, dbgmsg_maxsize, UINT, ZMOD_RW,
"Maximum ZFS debug log size");
diff --git a/module/os/linux/spl/spl-taskq.c b/module/os/linux/spl/spl-taskq.c
index 3b0c29606..abf4dca58 100644
--- a/module/os/linux/spl/spl-taskq.c
+++ b/module/os/linux/spl/spl-taskq.c
@@ -46,8 +46,10 @@ module_param(spl_taskq_thread_priority, int, 0644);
MODULE_PARM_DESC(spl_taskq_thread_priority,
"Allow non-default priority for taskq threads");
-static int spl_taskq_thread_sequential = 4;
-module_param(spl_taskq_thread_sequential, int, 0644);
+static uint_t spl_taskq_thread_sequential = 4;
+/* BEGIN CSTYLED */
+module_param(spl_taskq_thread_sequential, uint, 0644);
+/* END CSTYLED */
MODULE_PARM_DESC(spl_taskq_thread_sequential,
"Create new taskq threads after N sequential tasks");
diff --git a/module/os/linux/zfs/arc_os.c b/module/os/linux/zfs/arc_os.c
index d2a176d77..eaaf7d0bb 100644
--- a/module/os/linux/zfs/arc_os.c
+++ b/module/os/linux/zfs/arc_os.c
@@ -513,7 +513,7 @@ arc_prune_task(void *ptr)
* for releasing it once the registered arc_prune_func_t has completed.
*/
void
-arc_prune_async(int64_t adjust)
+arc_prune_async(uint64_t adjust)
{
arc_prune_t *ap;
diff --git a/module/os/linux/zfs/zfs_debug.c b/module/os/linux/zfs/zfs_debug.c
index 2a4e3f378..819416b68 100644
--- a/module/os/linux/zfs/zfs_debug.c
+++ b/module/os/linux/zfs/zfs_debug.c
@@ -29,13 +29,13 @@
typedef struct zfs_dbgmsg {
procfs_list_node_t zdm_node;
uint64_t zdm_timestamp;
- int zdm_size;
+ uint_t zdm_size;
char zdm_msg[1]; /* variable length allocation */
} zfs_dbgmsg_t;
static procfs_list_t zfs_dbgmsgs;
-static int zfs_dbgmsg_size = 0;
-int zfs_dbgmsg_maxsize = 4<<20; /* 4MB */
+static uint_t zfs_dbgmsg_size = 0;
+uint_t zfs_dbgmsg_maxsize = 4<<20; /* 4MB */
/*
* Internal ZFS debug messages are enabled by default.
@@ -68,14 +68,14 @@ zfs_dbgmsg_show(struct seq_file *f, void *p)
}
static void
-zfs_dbgmsg_purge(int max_size)
+zfs_dbgmsg_purge(uint_t max_size)
{
while (zfs_dbgmsg_size > max_size) {
zfs_dbgmsg_t *zdm = list_remove_head(&zfs_dbgmsgs.pl_list);
if (zdm == NULL)
return;
- int size = zdm->zdm_size;
+ uint_t size = zdm->zdm_size;
kmem_free(zdm, size);
zfs_dbgmsg_size -= size;
}
@@ -135,7 +135,7 @@ __set_error(const char *file, const char *func, int line, int err)
void
__zfs_dbgmsg(char *buf)
{
- int size = sizeof (zfs_dbgmsg_t) + strlen(buf);
+ uint_t size = sizeof (zfs_dbgmsg_t) + strlen(buf);
zfs_dbgmsg_t *zdm = kmem_zalloc(size, KM_SLEEP);
zdm->zdm_size = size;
zdm->zdm_timestamp = gethrestime_sec();
@@ -144,7 +144,7 @@ __zfs_dbgmsg(char *buf)
mutex_enter(&zfs_dbgmsgs.pl_lock);
procfs_list_add(&zfs_dbgmsgs, zdm);
zfs_dbgmsg_size += size;
- zfs_dbgmsg_purge(MAX(zfs_dbgmsg_maxsize, 0));
+ zfs_dbgmsg_purge(zfs_dbgmsg_maxsize);
mutex_exit(&zfs_dbgmsgs.pl_lock);
}
@@ -252,6 +252,8 @@ zfs_dbgmsg_print(const char *tag)
module_param(zfs_dbgmsg_enable, int, 0644);
MODULE_PARM_DESC(zfs_dbgmsg_enable, "Enable ZFS debug message log");
-module_param(zfs_dbgmsg_maxsize, int, 0644);
+/* BEGIN CSTYLED */
+module_param(zfs_dbgmsg_maxsize, uint, 0644);
+/* END CSTYLED */
MODULE_PARM_DESC(zfs_dbgmsg_maxsize, "Maximum ZFS debug log size");
#endif