Skip to content

Commit 6fd9fae

Browse files
committed
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 uint32_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 uint32_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. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
1 parent ede037c commit 6fd9fae

Some content is hidden

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

47 files changed

+443
-414
lines changed

cmd/zdb/zdb.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ zdb_ot_name(dmu_object_type_t type)
117117
extern int reference_tracking_enable;
118118
extern int zfs_recover;
119119
extern unsigned long zfs_arc_meta_min, zfs_arc_meta_limit;
120-
extern int zfs_vdev_async_read_max_active;
120+
extern uint32_t zfs_vdev_async_read_max_active;
121121
extern boolean_t spa_load_verify_dryrun;
122122
extern boolean_t spa_mode_readable_spacemaps;
123-
extern int zfs_reconstruct_indirect_combinations_max;
123+
extern uint32_t zfs_reconstruct_indirect_combinations_max;
124124
extern int zfs_btree_verify_intensity;
125125

126126
static const char cmdname[] = "zdb";

cmd/ztest.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,10 @@ static const ztest_shared_opts_t ztest_opts_defaults = {
253253
extern uint64_t metaslab_force_ganging;
254254
extern uint64_t metaslab_df_alloc_threshold;
255255
extern unsigned long zfs_deadman_synctime_ms;
256-
extern int metaslab_preload_limit;
256+
extern uint32_t metaslab_preload_limit;
257257
extern int zfs_compressed_arc_enabled;
258258
extern int zfs_abd_scatter_enabled;
259-
extern int dmu_object_alloc_chunk_shift;
259+
extern uint32_t dmu_object_alloc_chunk_shift;
260260
extern boolean_t zfs_force_some_double_word_sm_entries;
261261
extern unsigned long zio_decompress_fail_fraction;
262262
extern unsigned long zfs_reconstruct_indirect_damage_fraction;
@@ -4638,7 +4638,7 @@ ztest_dmu_object_next_chunk(ztest_ds_t *zd, uint64_t id)
46384638
{
46394639
(void) id;
46404640
objset_t *os = zd->zd_os;
4641-
int dnodes_per_chunk = 1 << dmu_object_alloc_chunk_shift;
4641+
uint32_t dnodes_per_chunk = 1 << dmu_object_alloc_chunk_shift;
46424642
uint64_t object;
46434643

46444644
/*

include/sys/arc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ typedef void arc_write_done_func_t(zio_t *zio, arc_buf_t *buf, void *priv);
8484
typedef void arc_prune_func_t(int64_t bytes, void *priv);
8585

8686
/* Shared module parameters */
87-
extern int zfs_arc_average_blocksize;
87+
extern uint32_t zfs_arc_average_blocksize;
8888
extern int l2arc_exclude_special;
8989

9090
/* generic arc_done_func_t's which you can use */

include/sys/arc_impl.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -976,15 +976,15 @@ extern arc_stats_t arc_stats;
976976
extern arc_sums_t arc_sums;
977977
extern hrtime_t arc_growtime;
978978
extern boolean_t arc_warm;
979-
extern int arc_grow_retry;
980-
extern int arc_no_grow_shift;
981-
extern int arc_shrink_shift;
979+
extern uint32_t arc_grow_retry;
980+
extern uint32_t arc_no_grow_shift;
981+
extern uint32_t arc_shrink_shift;
982982
extern kmutex_t arc_prune_mtx;
983983
extern list_t arc_prune_list;
984984
extern arc_state_t ARC_mfu;
985985
extern arc_state_t ARC_mru;
986986
extern uint_t zfs_arc_pc_percent;
987-
extern int arc_lotsfree_percent;
987+
extern uint32_t arc_lotsfree_percent;
988988
extern unsigned long zfs_arc_min;
989989
extern unsigned long zfs_arc_max;
990990

@@ -995,7 +995,7 @@ extern void arc_wait_for_eviction(uint64_t, boolean_t);
995995

996996
extern void arc_lowmem_init(void);
997997
extern void arc_lowmem_fini(void);
998-
extern void arc_prune_async(int64_t);
998+
extern void arc_prune_async(uint64_t);
999999
extern int arc_memory_throttle(spa_t *spa, uint64_t reserve, uint64_t txg);
10001000
extern uint64_t arc_free_memory(void);
10011001
extern int64_t arc_available_memory(void);

include/sys/dmu.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ int dmu_assign_arcbuf_by_dnode(dnode_t *dn, uint64_t offset,
865865
int dmu_assign_arcbuf_by_dbuf(dmu_buf_t *handle, uint64_t offset,
866866
struct arc_buf *buf, dmu_tx_t *tx);
867867
#define dmu_assign_arcbuf dmu_assign_arcbuf_by_dbuf
868-
extern int zfs_max_recordsize;
868+
extern uint32_t zfs_max_recordsize;
869869

870870
/*
871871
* Asynchronously try to read in the data.
@@ -1070,7 +1070,7 @@ int dmu_diff(const char *tosnap_name, const char *fromsnap_name,
10701070
#define ZFS_CRC64_POLY 0xC96C5795D7870F42ULL /* ECMA-182, reflected form */
10711071
extern uint64_t zfs_crc64_table[256];
10721072

1073-
extern int dmu_prefetch_max;
1073+
extern uint32_t dmu_prefetch_max;
10741074

10751075
#ifdef __cplusplus
10761076
}

include/sys/dsl_pool.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ struct dsl_deadlist;
6060
extern unsigned long zfs_dirty_data_max;
6161
extern unsigned long zfs_dirty_data_max_max;
6262
extern unsigned long zfs_wrlog_data_max;
63-
extern int zfs_dirty_data_max_percent;
64-
extern int zfs_dirty_data_max_max_percent;
65-
extern int zfs_delay_min_dirty_percent;
63+
extern uint32_t zfs_dirty_data_max_percent;
64+
extern uint32_t zfs_dirty_data_max_max_percent;
65+
extern uint32_t zfs_delay_min_dirty_percent;
6666
extern unsigned long zfs_delay_scale;
6767

6868
/* These macros are for indexing into the zfs_all_blkstats_t. */

include/sys/fm/util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ extern void fm_init(void);
9595
extern void fm_fini(void);
9696
extern void zfs_zevent_post_cb(nvlist_t *nvl, nvlist_t *detector);
9797
extern int zfs_zevent_post(nvlist_t *, nvlist_t *, zevent_cb_t *);
98-
extern void zfs_zevent_drain_all(int *);
98+
extern void zfs_zevent_drain_all(uint32_t *);
9999
extern zfs_file_t *zfs_zevent_fd_hold(int, minor_t *, zfs_zevent_t **);
100100
extern void zfs_zevent_fd_rele(zfs_file_t *);
101101
extern int zfs_zevent_next(zfs_zevent_t *, nvlist_t **, uint64_t *, uint64_t *);

include/sys/spa.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,7 @@ extern int spa_scrub_pause_resume(spa_t *spa, pool_scrub_cmd_t flag);
826826
extern void spa_sync(spa_t *spa, uint64_t txg); /* only for DMU use */
827827
extern void spa_sync_allpools(void);
828828

829-
extern int zfs_sync_pass_deferred_free;
829+
extern uint32_t zfs_sync_pass_deferred_free;
830830

831831
/* spa namespace global mutex */
832832
extern kmutex_t spa_namespace_lock;
@@ -1013,7 +1013,7 @@ extern boolean_t spa_indirect_vdevs_loaded(spa_t *spa);
10131013
extern blkptr_t *spa_get_rootblkptr(spa_t *spa);
10141014
extern void spa_set_rootblkptr(spa_t *spa, const blkptr_t *bp);
10151015
extern void spa_altroot(spa_t *, char *, size_t);
1016-
extern int spa_sync_pass(spa_t *spa);
1016+
extern uint32_t spa_sync_pass(spa_t *spa);
10171017
extern char *spa_name(spa_t *spa);
10181018
extern uint64_t spa_guid(spa_t *spa);
10191019
extern uint64_t spa_load_guid(spa_t *spa);

include/sys/spa_impl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ struct spa {
215215
nvlist_t *spa_config_splitting; /* config for splitting */
216216
nvlist_t *spa_load_info; /* info and errors from load */
217217
uint64_t spa_config_txg; /* txg of last config change */
218-
int spa_sync_pass; /* iterate-to-convergence */
218+
uint32_t spa_sync_pass; /* iterate-to-convergence */
219219
pool_state_t spa_state; /* pool state */
220220
int spa_inject_ref; /* injection references */
221221
uint8_t spa_sync_on; /* sync threads are running */
@@ -446,7 +446,7 @@ struct spa {
446446

447447
extern char *spa_config_path;
448448
extern const char *zfs_deadman_failmode;
449-
extern int spa_slop_shift;
449+
extern uint32_t spa_slop_shift;
450450
extern void spa_taskq_dispatch_ent(spa_t *spa, zio_type_t t, zio_taskq_type_t q,
451451
task_func_t *func, void *arg, uint_t flags, taskq_ent_t *ent);
452452
extern void spa_taskq_dispatch_sync(spa_t *, zio_type_t t, zio_taskq_type_t q,

include/sys/txg.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ extern void *txg_list_head(txg_list_t *tl, uint64_t txg);
138138
extern void *txg_list_next(txg_list_t *tl, void *p, uint64_t txg);
139139

140140
/* Global tuning */
141-
extern int zfs_txg_timeout;
141+
extern uint32_t zfs_txg_timeout;
142142

143143

144144
#ifdef ZFS_DEBUG

0 commit comments

Comments
 (0)