argSUSed: lib (so also the bit of module that is built for lib), include

[nabijaczleweli]

Motivation and Context

There are 2 unused arguments among us.

Description

Draft because on top of #12835 and #12067, otherwise should be g2g except as indicated below.

The main revelation over my previous attempt at this was making ASSERTs et al do (void)sizeof(args)...: this doesn't ODR-use the arguments, so they get elided as usual, but it marks them as used (and actually verifies that they compile, which is a nice bonus), bringing the total diffstat to just

113 files changed, 446 insertions(+), 331 deletions(-)

across 97 commits, which is more than fine, imo
These are sequestered to the first two commits.

Commits that start with a ! are more than purely editorial. In zfs sendrecv, I think this was a bug that I fixed. In arc.c I removed the mutex argument, which the comment says is used but isn't, and is only used by the caller?

In rdwr_efi.c, do we want efi_type() anymore? All it is right now is some #if 0ed Solaris code that returns ENOSYS, and is only applicable for the Solaris blockdev layer. In the Illumos gate, there's a single user: rmformat(1); I recommend a read of the manual as a blast from the past, but, well.

How Has This Been Tested?

make -C lib/ builds clean on clang 14 when configured with -Wall -Wextra -Wno-sign-compare -Wno-missing-field-initializers in CFLAGS

git -C lib grep -i argsused and the equivalent invocations for include and module/{avl,icp,lua,nvpair,spl,unicode,zcommon,zstd} are empty. The keen-eyed observer will note that this leaves only module/zfs and module/os (f/s, f/z, and l/z). (Well, and cstyle.pl.)

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.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes. – none should 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. – needs mass replacing on ; when ready

[nabijaczleweli]

Rebased with full commit messages, should be g2g, but see notes in OP

[behlendorf]

Generally looks good. In order to move this forward what I'd like to do is move the commits which include functional changes, or I commented on specifically, in to their own PRs. We'll want to round up additional reviewers for those.

For the rest it would be nice to squash the patch stack a bit. One option would be merging similar commits together by library/module or header.

[behlendorf]

This is bug. err should be assigned to zio->io_error after the conditional, see the Linux version of vdev_file_io_strategy. Can you move the updated fix for this to its own PR for review.

[nabijaczleweli]

#12904

[behlendorf]

There's also a good chance this is bug, the Linux counterpart to this function zfs_vget does make use of setgen for an error case. This would also be a good candidate to move to it's own PR so we can get the FreeBSD folks to take a closer look.

[nabijaczleweli]

#12905

[behlendorf]

Just for context the hash_lock still actually may be checked here through the HDR_EMPTY_OR_LOCKED() check. It doesn't need so this looks fine.

[behlendorf]

Why __unused here and not (void) config?

[nabijaczleweli]

Because there's __unused above, too, but fair point, I've changed them both to void casts.

[nabijaczleweli]

Updated. Moved the remaining one (libzfs/sendrecv) to #12906, others as noted.

As it stands, here's all the sus commits (sorted by amount of commits per section for demonstration purposes):

include: dmu.h
libefi: efi_type
libzfs_core
libshare: nfs: is_shared_cb
libshare: platform: nfs: disable_share_impl
libzpool: util
libzpool: kernel
libzpool: taskq
libzutil: import: find_import_impl
libzutil: import: platform
libzutil: import: platform: is_mpath_whole_disk
libnvpair: alloc_fixed: free
libnvpair: main: print_list_*
libnvpair: alloc_system: free
libnvpair: fnvpair: free
libnvpair: nvpair
libzfs: util: linux
libzfs: changelist
libzfs: config: node_compare
libzfs: crypto
libzfs: dataset: userquota_propname_decode
libzfs: mountpoint
libzfs: status
libzfs: mount: platforms
libzfs: fletcher
libzfs: iter
module: avl
module: zcommon: zprop
module: zfs: linux: abd
module: zfs: linux: arc
module: zfs: debug: linux
module: zfs: freebsd: znode
module: zfs: freebsd: spa_os
module: zfs: bpobj
module: zfs: bptree
module: zfs: dmu
module: zfs: dmu: diff
module: zfs: dmu: objset
module: zfs: dmu: redact
module: zfs: dmu: send
module: zfs: dmu: traverse
module: zfs: dmu: zfetch
module: zfs: dnode
module: zfs: dsl: bookmark
module: zfs: dsl: dataset
module: zfs: dsl: destroy
module: zfs: dsl: dir
module: zfs: dsl: pool
module: zfs: dsl: prop
module: zfs: dsl: scan
module: zfs: dsl: synctask: null_checkfunc
module: zfs: fm
module: zfs: gzip
module: zfs: lz4
module: zfs: lzjb
module: zfs: metaslab
module: zfs: range_tree
module: zfs: sa
module: zfs: sha256
module: zfs: spa
module: zfs: spa: checkpoint
module: zfs: spa: history
module: zfs: spa: misc
module: zfs: spa: vdev
module: zfs: vdev: draid
module: zfs: vdev: file: platforms
module: zfs: vdev: indirect
module: zfs: vdev: initialize
module: zfs: vdev: mirror
module: zfs: vdev: missing
module: zfs: vdev: raidz
module: zfs: vdev: removal
module: zfs: vdev: trim
module: zfs: zcp
module: zfs: zcp: synctask
module: zfs: racct: platforms
module: zfs: zil
module: zfs: zio
module: zfs: zio: checksum
module: zfs: zio: compress
module: zfs: zio: crypt: platforms
module: zfs: abd
module: zfs: arc
module: zfs: dbuf
module: zfs: zfs_fm
module: icp: algs: gcm
module: icp: algs: ccm
module: icp: io: aes
module: icp: io: sha1_mod
module: icp: core: kcf: mech_tabs
module: icp: core: kcf: prov_lib
module: icp: io: sha2_mod
module: icp: os: modhash
module: icp: os: modconf
module: icp: core: kcf: sched
module: icp: io: skein_mod
module: icp: remaining

Here's variant lite (libraries grouped, modules grouped by the subsystem):

include: dmu.h
libefi: efi_type
libzfs_core
libshare: nfs: is_shared_cb
libzpool
libzutil: import
libnvpair
libzfs
module: avl
module: zcommon: zprop
module: zfs: linux
module: zfs: freebsd
module: zfs: bpobj
module: zfs: bptree
module: zfs: dmu
module: zfs: dnode
module: zfs: dsl
module: zfs: fm
module: zfs: gzip
module: zfs: lz4
module: zfs: lzjb
module: zfs: metaslab
module: zfs: range_tree
module: zfs: sa
module: zfs: sha256
module: zfs: spa
module: zfs: vdev
module: zfs: zcp
module: zfs: racct: platforms
module: zfs: zil
module: zfs: zio
module: zfs: abd
module: zfs: arc
module: zfs: dbuf
module: zfs: zfs_fm
module: icp: algs
module: icp: io
module: icp: core: kcf
module: icp: os: modhash
module: icp: os: modconf
module: icp: remaining

Or variant heavy (grouped by module, I kept the linux/freebsd bits separate because I usually do but in variant heavy+, let's say, these can also meld together):

include: dmu.h
libefi: efi_type
libzfs_core
libshare: nfs: is_shared_cb
libzpool
libzutil: import
libnvpair
libzfs
module: avl
module: zcommon: zprop
module: zfs: linux
module: zfs: freebsd
module: zfs
module: icp

Which one strikes you best?

[behlendorf]

Let's go with the last variant, that seems like a sufficient level of granularity for this kind of change. The module: zfs commit I imagine with be substantially larger than the others but that's not a problem.

include: dmu.h
libefi: efi_type
libzfs_core
libshare: nfs: is_shared_cb
libzpool
libzutil: import
libnvpair
libzfs
module: avl
module: zcommon: zprop
module: zfs: linux
module: zfs: freebsd
module: zfs
module: icp