.rodata globals and .data pruning: modules

[nabijaczleweli]

Motivation and Context

I haven't moved for the over seven hours since I started, but I've evaluated every variable that lives in .data (and globals in .rodata) in the kernel modules, and constified/eliminated/localised them appropriately. This means that all read-only data is now actual read-only data (except for if i missed something in my 12k of policies, doubt it though), and, if possible, at file scope. A lot of previously-global-symbols became inlinable (and inlined!) constants. Probably not in a big Wowee Performance Moment, but hey.

This is in many ways a follow-up to #12836 except that that took fifty seven seconds instead.

Also this does minor pre-#12895-clean-up. Just the obvious stuff near the diffhunks I was touching already to make it, uh, have a const in the file, just one.

Description

On top of #12844. One big commit (the one at the very top). Needs cstyle. Whatever. zfs.ko can go [REDACTED]

147 files changed, 659 insertions(+), 774 deletions(-)

How Has This Been Tested?

Builds on Linux. CI will show if the lua lightuserdatas actually need to be writable (i.e. the kernel will crash horribly if they do).

The methodology here is quite simple:

find module/ -name '*.ko' -exec objdump -tj .data {} \;

and git grep -> const -> rebuild -> rerun until you're satisfied for your module of choice, then

find module/ -name '*.ko' -exec objdump -tj .rodata {} \; | grep -v ' l '

and git grep -> static -> rebuild -> rerun until you're satisfied with your module of choice.

The Big Problem here is zfs.ko – well, the fact that it has a lot of platform-conditional code and it's bloody huge, compared to even the sum of all the others. Here's the final filter that removes all the variables that are Known Good, if you run it on FreeBSD you'll catch just the superfluous FreeBSD symbols:

nabijaczleweli@tarta:~/store/code/zfs$ find module/ -name '*.ko' -and ! -name znvpair.ko -and ! -name zzstd.ko -and ! -name zunicode.ko ! -name spl.ko ! -name zavl.ko ! -name zlua.ko -and ! -name icp.ko -and ! -name zcommon.ko -exec  objdump -tj .data {} \; | grep -vEe '_stats' -e '_kstat' -e zfs_vdev_mirror_rotating_inc -e zfs_vdev_mirror_rotating_seek_inc -e zfs_vdev_mirror_rotating_seek_offset -e zfs_vdev_mirror_non_rotating_inc -e zfs_vdev_mirror_non_rotating_seek_inc -e spa_config_generation -e zfs_vdev_raidz_impl -e vdev_raidz_fastest_impl -e sel_impl -e zfs_vnops_read_chunk_size -e inject_next_id -e arc_shrinker -e zfs_misc$ -e 'acl_rel_(head|tail)$' -e 'metaslab_(aliquot|bias_enabled|debug_load|debug_unload|df_alloc_threshold|df_free_pct|df_max_search|df_use_largest_segment|force_ganging|fragmentation_factor_enabled|lba_weighting_enabled|load_pct|preload_enabled|preload_limit|unload_delay|unload_delay_ms)' -e 'zfs_metaslab_(find_max_tries|fragmentation_threshold|max_size_cache_sec|mem_limit|segment_weight_enabled|sm_blksz_no_log|sm_blksz_with_log|switch_threshold|try_hard_before_gang)' -e 'zfs_mg_(fragmentation_threshold|noalloc_threshold)' -e 'zvol_(major|request_sync|prefetch_bytes|max_discard_blocks|threads|taskq|ida)$' -e zfs_vdev_async_write_max_active -e 'zfs_initialize_(value|chunk_size)' -e zfs_zevent_len_max -e ratelimit_dropped -e 'zevent_(cv|eid|flags|len_cur|list|lock|waiters)' -e 'zfs_vdev_(read|write)_gap_limit' -e 'zfs_vdev_aggregate_trim' -e 'zfs_vdev_aggregation_limit(_non_rotating)?' -e 'zfs_vdev_async_read_m(ax|in)_active' -e 'zfs_vdev_async_write_active_m(ax|in)_dirty_percent' -e 'zfs_vdev_async_write_m(ax|in)_active' -e 'zfs_vdev_def_queue_depth' -e 'zfs_vdev_initializing_m(ax|in)_active' -e 'zfs_vdev_max_active' -e 'zfs_vdev_nia_(credit|delay)' -e 'zfs_vdev_queue_depth_pct' -e 'zfs_vdev_rebuild_m(ax|in)_active' -e 'zfs_vdev_removal_m(ax|in)_active' -e 'zfs_vdev_scrub_m(ax|in)_active' -e 'zfs_vdev_sync_read_m(ax|in)_active' -e 'zfs_vdev_sync_write_m(ax|in)_active' -e 'zfs_vdev_trim_m(ax|in)_active' -e zfs_dbgmsg_enable -e zfs_fallocate_reserve_percent -e 'zfs_trim_extent_bytes_m(ax|in)' -e zfs_trim_metaslab_skip -e zfs_trim_queue_limit -e zfs_trim_txg_batch -e zfs_override_estimate_recordsize -e zfs_send_corrupt_data -e 'zfs_send_no_prefetch_queue_(ff|length)' -e 'zfs_send_queue_(ff|length)' -e zfs_send_unmodified_spill_blocks -e ARC_anon -e 'arc_evict_(count|lock|needed|waiters|zthr)' -e arc_grow_retry -e arc_growtime -e arc_ksp -e ARC_l2c_only -e arc_lotsfree_percent -e ARC_mfu -e ARC_mfu_ghost -e arc_min_prefetch_ms -e arc_min_prescient_prefetch_ms -e 'ARC_mru(_ghost)?' -e arc_no_grow_shift -e arc_p_min_shift -e 'arc_prune_(list|mtx)' -e arc_prune_taskq -e arc_reap_zthr -e arc_shrink_shift -e arc_stats -e arc_sums -e arc_warm -e l2arc_exclude_special -e l2arc_mfuonly -e 'l2arc_rebuild_(blocks_min_l2size|enabled)' -e l2arc_trim_ahead -e zfs_arc_average_blocksize -e 'zfs_arc_dnode_limit(_percent)?' -e zfs_arc_dnode_reduce_percent -e zfs_arc_evict_batch_limit -e zfs_arc_eviction_pct -e zfs_arc_grow_retry -e zfs_arc_lotsfree_percent -e zfs_arc_max -e 'zfs_arc_meta_(adjust_restarts|limit|limit_percent|min|prune|strategy)' -e zfs_arc_min -e zfs_arc_min_prefetch_ms -e zfs_arc_min_prescient_prefetch_ms -e zfs_arc_overflow_shift -e zfs_arc_p_dampener_disable -e zfs_arc_p_min_shift -e zfs_arc_pc_percent -e zfs_arc_shrink_shift -e zfs_arc_sys_free -e zfs_compressed_arc_enabled -e vdev_validate_skip -e zfs_checksum_events_per_second -e zfs_embedded_slog_min_ms -e zfs_nocacheflush -e zfs_scan_ignore_errors -e zfs_slow_io_events_per_second -e 'zfs_vdev_default_ms_(count|shift)' -e zfs_vdev_dtl_sm_blksz -e 'zfs_vdev_m(ax|in)_auto_ashift' -e zfs_vdev_min_ms_count -e zfs_vdev_ms_count_limit -e zfs_vdev_standard_sm_blksz -e 'l2arc_feed_(again|min_ms|secs)' -e 'l2arc_headroom(_boost)?' -e l2arc_meta_percent -e l2arc_noprefetch -e l2arc_norw -e 'l2arc_write_(boost|max)?' -e zfs_keep_log_spacemaps_at_export -e zfs_max_log_walking -e zfs_max_logsm_summary_length -e zfs_min_metaslabs_to_flush -e 'zfs_unflushed_log_block_(max|min|pct)' -e 'zfs_unflushed_max_mem_(amt|ppm)' -e zfetch_array_rd_sz -e zfetch_ksp -e 'zfetch_max_(((i?)distance)|streams)' -e zfetch_min_sec_reap -e zfs_prefetch_disable -e 'spa_config_(generation|path)' -e zfs_autoimport_disable -e zfs_key_max_salt_uses -e fill_weight -e zfs_async_block_max_blocks -e zfs_free_bpobj_enabled -e zfs_free_min_time_ms -e zfs_max_async_dedup_frees -e 'zfs_no_scrub_(io|prefetch)' -e zfs_obsolete_min_time_ms -e zfs_resilver_disable_defer -e zfs_resilver_min_time_ms -e zfs_scan_checkpoint_intval -e zfs_scan_fill_weight -e zfs_scan_issue_strategy -e zfs_scan_legacy -e 'zfs_scan_max_(ext_gap|lim_fact|lim_soft_fact)' -e zfs_scan_strict_mem_lim -e zfs_scan_suspend_progress -e zfs_scan_vdev_limit -e zfs_scrub_min_time_ms -e zfs_vdev_async_write_active_min_dirty_percent -e fill_weight -e zfs_async_block_max_blocks -e zfs_free_bpobj_enabled -e zfs_max_async_dedup_frees -e zfs_no_scrub_io -e zfs_no_scrub_prefetch -e 'zfs_(free|obsolete|resilver|scrub)_min_time_ms' -e zfs_resilver_disable_defer -e zfs_scan_checkpoint_intval -e zfs_scan_fill_weight -e zfs_scan_issue_strategy -e zfs_scan_legacy -e zfs_scan_max_ext_gap -e 'zfs_scan_mem_lim_(fact|soft_fact)' -e zfs_scan_strict_mem_lim -e zfs_scan_suspend_progress -e zfs_scan_vdev_limit -e zfs_vdev_async_write_active_min_dirty_percent -e 'zfs_livelist_(max_entries|min_percent_shared)' -e zfs_delay_min_dirty_percent -e zfs_delay_scale -e 'zfs_dirty_data_max(max|percent|max_percent)?' -e zfs_dirty_data_sync_percent -e zfs_sync_taskq_batch_pct -e zfs_wrlog_data_max -e 'zfs_zil_clean_taskq_(maxalloc|minalloc|nthr_pct)?' -e zfs_txg_timeout -e zfs_abd_scatter_min_size -e zap_iterate_prefetch -e fzap_default_block_shift -e zfs_ccw_retry_interval -e vdc_ksp -e 'zfs_vdev_cache_(bshift|max|size)' -e spa_asize_inflation -e 'spa_l2cache_(avl|lock)' -e spa_mode_global -e 'spa_namespace_(avl|cv|lock)' -e spa_slop_shift -e spa_spare_avl -e spa_spare_lock -e 'zfs_deadman_(checktime_ms|enabled|synctime_ms|ziotime_ms)' -e zfs_flags -e zfs_free_leak_on_eio -e zfs_recover -e zfs_ddt_data_is_special -e zfs_user_indirect_is_special -e zfs_special_class_metadata_reserve_pct -e 'zfs_rebuild_(max_segment|vdev_limit|scrub_enabled)' -e send_holes_without_birth_time -e zfs_pd_bytes_max -e zfs_traverse_indirect_prefetch_limit -e zfs_vdev_scheduler -e 'zfs_condense_indirect_(commit_entry_delay_ms|obsolete_pct|vdevs_enable)' -e zfs_condense_max_obsolete_bytes -e zfs_condense_min_mapping_bytes -e 'zfs_reconstruct_indirect_(combinations_max|damage_fraction)' -e 'zvol_(inhibit_dev|volmode|htable|state_list|state_lock)' -e 'zfs_(read_history|read_history_hits|txg_history|multihost_history)' -e 'zfs_sync_pass_(deferred_free|dont_compress|rewrite)' -e 'zio_buf_cache(allocs|frees)?' -e zio_cache -e zio_data_buf_cache -e zio_deadman_log_all -e zio_dva_throttle_enabled -e zio_exclude_metadata -e zio_link_cache -e zio_requeue_io_start_cut_in_line -e zio_slow_io_ms -e dmu_prefetch_max -e zfs_dmu_offset_next_sync -e zfs_nopwrite_enabled -e zfs_per_txg_dirty_frees_percent -e dmu_object_alloc_chunk_shift -e 'metaslab_(aliquot|bias_enabled|debug_load|debug_unload|df_alloc_threshold|df_free_pct|df_max_search|df_use_largest_segment|force_ganging|fragmentation_factor_enabled|lba_weighting_enabled|load_pct)' -e 'metaslab_preload_(enabled|limit)' -e 'metaslab_unload_delay(_ms)?' -e zfs_condense_pct -e 'zfs_metaslab_(find_max_tries|fragmentation_threshold|max_size_cache_sec|mem_limit|segment_weight_enabled|sm_blksz_no_log|sm_blksz_with_log|switch_threshold|try_hard_before_gang)' -e zfs_mg_fragmentation_threshold -e zfs_mg_noalloc_threshold -e zvol_replay_vector -e redact_sync_bufsize -e redaction_list_update_interval_ns -e 'zfs_redact_queue_(ff|length)' -e vdev_removal_max_span -e 'zfs_removal_(ignore_errors|suspend_progress)' -e zfs_remove_max_segment -e zfs_object_mutex_size -e zfs_unlink_suspend_progress -e znode_cache -e znode_hold_cache -e 'spa_load_verify_(shift|metadata|data)' -e spa_load_print_vdev_tree -e spa_load_verify_dryrun -e spa_mode_readable_spacemaps -e 'zfs_livelist_condense_(new_alloc|sync_cancel|sync_pause|zthr_cancel|zthr_pause)' -e 'zfs_max_missing_tvds(_cachefile|_scan)?' -e 'zio_taskq_batch_(pct|tpq)' -e 'zfs_lua_max_(instrlimit|memlimit)' -e zfs_allow_log_key -e zfs_fsyncer_key -e zfs_history_output_max -e zfs_max_nvlist_src_size -e 'zfsdev_state_(list|lock)' -e zfs_force_some_double_word_sm_entries -e space_map_ibs -e sa_cache -e dbu_evict_taskq -e 'dbuf_cache_(evict_thread|hiwater_pct|lowater_pct|max_bytes|shift)' -e 'dbuf_evict_(cv|lock|thread_exit)' -e dbuf_hash_table -e dbuf_kmem_cache -e 'dbuf_metadata_cache_(max_bytes|shift)' -e zfs_max_recordsize -e zfs_allow_redacted_dataset_mount -e zero_zil -e zfs_spa_discard_memory_limit -e 'zfs_recv_queue_(length|ff)' -e zfs_recv_write_batch_size -e 'zfs_multihost_(interval|import_intervals|fail_intervals)' -e zil_maxblocksize -e zfs_admin_snapshot -e zfs_expire_snapshot -e zfs_snapshot_lock -e 'zfs_snapshots_by_(name|objsetid)'-e raidz_impl_kstats -e 'raidz_math_(initialized|kstat)' -e 'raidz_supp_impl(_cnt)?' -e user_sel_impl -e zfs_vdev_raidz_impl -e 'zfs_dbgmsg_(enable|kstat|maxsize|size)' -e 'zfs_dbgmsgs(_lock)?' -e zpl_fs_type -e abd_cache -e abd_ksp -e 'abd_zero_(page|scatter)' -e 'zfs_abd_scatter_(max_order|min_size)' -e zfs_deadman_failmode -e zil_maxblocksize -e zfs_delete_blocks -e vdev_file_taskq -e 'vdev_file_(log|phys)ical_ashift' -e zfs_commit_timeout_pct -e zil_ksp -e zil_lwb_cache -e zil_nocacheflush -e zil_replay_disable -e zil_slog_bulk -e zil_zcw_cache -e 'recent_events_(cleaner_tqid|list|lock|tree)' -e 'zfs_zevent_retain_(expire_secs|max)' -e 'zfs_default_(i?)bs' -e zpl_xattr_handlers -e zfs_immediate_write_sz -e zfs_abd_scatter_enabled


nabijaczleweli@tarta:~/store/code/zfs$ find module/ -name '*.ko' -and ! -name znvpair.ko -and ! -name zzstd.ko -and ! -name zunicode.ko ! -name spl.ko ! -name zavl.ko ! -name zlua.ko -and ! -name icp.ko -and ! -name zcommon.ko -exec  objdump -tj .rodata {} \; | grep -v ' l ' | grep -vEe vdev_raidz_avx512f_impl -e zpl_ops_snapdir -e 'vdev_raidz_(pow|log)2' -e rt_btree_ops -e vdev_raidz_vsd_ops -e zpl_ops_shares -e zpl_dir_inode_operations -e vdev_raidz_ssse3_impl -e zio_compress_table -e dmu_ot_byteswap -e zpl_dir_file_operations -e zio_crypt_table -e zio_type_name -e vdev_raidz_.+_impl -e  zvol_replay_vector -e  zpl_symlink_inode_operations  -e vdev_hole_ops -e vdev_replacing_ops -e vdev_root_ops -e zpl_super_operations -e zpl_inode_operations -e zfs_metaslab_ops -e zpl_ops_root -e zpl_fops_root -e recv_clone_name -e zpl_dentry_operations -e vdev_disk_ops -e 'raidz_(gen|rec)_name' -e dmu_ot -e zpl_fops_shares -e zpl_fops_shares -e vdev_file_ops -e zfs_attr_table -e zpl_address_space_operations -e zpl_fops_snapdir -e
ddt_zap_ops -e zpl_file_operations -e zpl_special_inode_operations -e vdev_missing_ops -e vdev_draid_spare_ops -e zio_checksum_table -e gf_clmul_mod_lt -e zfs_replay_vector -e zpl_export_operations -e vdev_spare_ops -e vdev_indirect_ops -e vdev_raidz_ops -e vdev_draid_ops -e vdev_mirror_ops

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements. – needs cstyle probably
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes. – hopefully none apply
• I have run the ZFS Test Suite with this change applied. – CI take my hand
• All commit messages are properly formatted and contain Signed-off-by.

[nabijaczleweli]

@behlendorf Given that this touches the following with the same proverbial commit message (much like #12844):

include/libzfs.h
cmd/zhack/
cmd/ztest/
include/os/freebsd/zfs/sys/
include/os/linux/spl/rpc/
include/os/linux/spl/sys/
include/os/linux/zfs/sys/
include/sys/
include/sys/crypto/
include/sys/fs/
lib/libnvpair/
lib/libspl/include/
module/avl/
module/icp/algs/aes/
module/icp/algs/edonr/
module/icp/algs/modes/
module/icp/algs/skein/
module/icp/api/
module/icp/asm-x86_64/aes/
module/icp/asm-x86_64/modes/
module/icp/asm-x86_64/sha1/sha1-
module/icp/asm-x86_64/sha2/
module/icp/core/
module/icp/include/sys/
module/icp/include/sys/crypto/
module/icp/io/
module/icp/os/
module/icp/spi/
module/nvpair/
module/os/freebsd/zfs/
module/os/linux/spl/
module/os/linux/zfs/
module/unicode/
module/zcommon/
module/zfs/

Does a cleave line like this make sense?

include: libzfs
include: private
cmd: zhack
cmd: ztest
include: zfs: freebsd
include: spl: linux
include: zfs: linux
include: sys:
lib: libnvpair
lib: libspl: include
module: avl
module: icp
module: nvpair
module: zfs: freebsd 
module: zfs: linux 
module: spl: linux 
module: unicode
module: zcommon
module: zfs

[nabijaczleweli]

Rebased

[behlendorf]

I'm fine with a single commit for "prune .data, global .rodata" changes. Since it'll take me a while to work through that diff I'll go ahead and get those other 4 cleanup patches merged first. They all looked good to me.

[nabijaczleweli]

Would you I split them off into a separate PR or are you fine just picking them off this one?

[behlendorf]

No \(md Sy does not render as an * for me with man v2.7.6.1, but instead as an empty square. This was with RHEL 8.

[nabijaczleweli]

md is the multiplication dot, and present in every groff version as far back as the git repository goes (1991) – are you on a VT or some other generally-incapable terminal (or, uh, have broken locales maybe?)?

[behlendorf]

Let me grab the first three. The man page change seems to not render for me correctly. Can you see what the issue is there, then open up a new PR for that fix.

config: check for parallel(1), use it for cstyle
module: zfs: znode: freebsd: remove unused variable
module: Makefile: flatten subdir loop, use $PWD instead of pwd 

[nabijaczleweli]

Clean rebase

[behlendorf]

I've gone over this twice now, and it all looks pretty reasonable. Though it's really easy to gloss over-something in a change this large and repetitive, but we have the CI to help with that! One question, are there additional compiler warnings we should consider enabling by default after this change?

[nabijaczleweli]

I don't know? AFAICT this is all valid and just a very unfortunate side-effect of copying/following the everything-global-mutable default. Maybe there's a "static is mutable but doesn't need to be" warning? (A quick look through the Clang warning list says there isn't.) But even that's.. weak for most of this, since consumers mask this by over-requiring mutable data, and the default is non-static (hell, there's bits in the freebsd sysctl handling that do externs for the parameters in their own .cs without header prototypes). Alas :/