Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scripts: zfs.sh: unload zfs with rmmod #13353

Closed
wants to merge 1 commit into from

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

The zfs.sh script can no longer be used to remove the kmods
when they are not installed on the system.

$ sudo ./scripts/zfs.sh -u
modprobe: FATAL: Module zfs not found.

$ sudo modprobe -r zfs
modprobe: FATAL: Module zfs not found.

cc: @nabijaczleweli

Description

Use rmmod instead of modprobe -r in the zfs.sh script to unload
the kmods. While modprobe -r is nicer, it can only be used when
the kmods are installed on the system, if they're not the following
error will always be returned.

sudo modprobe -r zfs
modprobe: FATAL: Module zfs not found.

This reverts the switch to modprobe from commit 469b848.

How Has This Been Tested?

Locally by building the source then loading and then unloading the
in-tree kmods.

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:

Use `rmmod` instead of `modprobe -r` in the zfs.sh script to unload
the kmods.  While `modprobe -r` is nicer, it can only be used when
the kmods are installed on the system, if they're not the following
error will always be returned.

    sudo modprobe -r zfs
    modprobe: FATAL: Module zfs not found.

This reverts the switch to modprobe from commit 469b848.

Signed-off-by: Brian Behlendorf <[email protected]>
Issue openzfs#13274
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Apr 20, 2022
@nabijaczleweli
Copy link
Contributor

nabijaczleweli commented Apr 21, 2022

(a) your description appears to only hold for.. no clue which system?

root@kasan-test:~# modprobe -r zfs
[  101.046424] ZFS: Unloaded module v2.1.99-1063_g62109f87b
root@kasan-test:~# modprobe -r zfs

(b) conversely, rmmod does break on this:

root@kasan-test:~# rmmod zfs
[  136.251948] ZFS: Unloaded module v2.1.99-1063_g62109f87b
root@kasan-test:~# rmmod zfs
rmmod: ERROR: Module zfs is not currently loaded

so I'm not sure how this is better? I assumed this would be the reverse on ancient RHEL but it isn't:
image

(c) but this is, rather trivially, broken across a pre-unification/post-unification upgrade (which is why we needed to use modprobe -r in the first place)

nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this pull request Apr 21, 2022
Some ancient modprobes error out when provided with -r unloaded-module

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Supersedes openzfs#13353
@bwatkinson
Copy link
Contributor

(a) your description appears to only hold for.. no clue which system?

root@kasan-test:~# modprobe -r zfs
[  101.046424] ZFS: Unloaded module v2.1.99-1063_g62109f87b
root@kasan-test:~# modprobe -r zfs

(b) conversely, rmmod does break on this:

root@kasan-test:~# rmmod zfs
[  136.251948] ZFS: Unloaded module v2.1.99-1063_g62109f87b
root@kasan-test:~# rmmod zfs
rmmod: ERROR: Module zfs is not currently loaded

so I'm not sure how this is better? I assumed this would be the reverse on ancient RHEL but it isn't: image

(c) but this is, rather trivially, broken across a pre-unification/post-unification upgrade (which is why we needed to use modprobe -r in the first place)

I just gave #13356 a go on CentOS 8 (CentOS Linux release 8.2.2004 (Core)) and I am still getting the error @behlendorf described in this PR when doing zfs.sh -uv. The only way I could remove the modules was by doing rmmod with your other PR.

@nabijaczleweli
Copy link
Contributor

CentOS 8 Stream (Stream 8)?, #13356:

[nab@centos8 zfs]$ sudo ./scripts/zfs.sh -vu
Successfully unloaded ZFS module stack
[nab@centos8 zfs]$ sudo ./scripts/zfs.sh -vu
Successfully unloaded ZFS module stack

Running #13356 with sh -x ends with

+ unload_modules_linux
+ for KMOD in "$KMOD_ZFS" "$KMOD_SPL"
+ NAME=zfs.ko
+ NAME=zfs
+ '[' -d /sys/module/zfs ']'
+ for KMOD in "$KMOD_ZFS" "$KMOD_SPL"
+ NAME=spl.ko
+ NAME=spl
+ '[' -d /sys/module/spl ']'
+ '[' yes = yes ']'
+ echo 'Successfully unloaded ZFS module stack'
Successfully unloaded ZFS module stack
+ '[' no = yes ']'
+ exit 0

as expected.

@behlendorf
Copy link
Contributor Author

Running #13356 with sh -x on RHEL 8.5 ends with:

+ unload_modules_linux
+ for KMOD in "$KMOD_ZFS" "$KMOD_SPL"
+ NAME=zfs.ko
+ NAME=zfs
+ '[' -d /sys/module/zfs ']'
+ /sbin/modprobe -r zfs
modprobe: FATAL: Module zfs not found.
+ return
+ '[' no = yes ']'
+ exit 0

This was with modprobe kmod version 25. It's entirely possible CentOS stream has moved to something newer.

]$ modprobe --version
kmod version 25
+XZ +ZLIB +OPENSSL -EXPERIMENTAL

I should have been more clear in my description, this modprobe isn't checking for /sys/module/<kmod>. That would all be very reasonable, should exist on all supported Linux platforms, and may be exactly what the latest version does (I haven't yet checked). However, according to strace it appears to be checking for the kmod name in the depmod generated files. Presumably, if it's not a known kmod from the depmod search paths it simply bails, unlike rmmod which does a bit less checking and simply invokes the syscall and hopes for the best.

openat(AT_FDCWD, "/lib/modules/4.18.0-348.20.1.el8_5.x86_64/modules.dep.bin", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0640, st_size=432870, ...}) = 0
mmap(NULL, 432870, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fb147043000
close(3)                                = 0
openat(AT_FDCWD, "/lib/modules/4.18.0-348.20.1.el8_5.x86_64/modules.alias.bin", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0640, st_size=857300, ...}) = 0
mmap(NULL, 857300, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fb146f71000
close(3)                                = 0
openat(AT_FDCWD, "/lib/modules/4.18.0-348.20.1.el8_5.x86_64/modules.symbols.bin", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0640, st_size=560668, ...}) = 0
mmap(NULL, 560668, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fb146ee8000
close(3)                                = 0
openat(AT_FDCWD, "/lib/modules/4.18.0-348.20.1.el8_5.x86_64/modules.builtin.bin", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0640, st_size=10109, ...}) = 0
mmap(NULL, 10109, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fb1470c0000
close(3)                                = 0
write(2, "modprobe: FATAL: Module zfs not "..., 39modprobe: FATAL: Module zfs not found.
) = 39
exit_group(1)                           = ?
+++ exited with 1 +++

@nabijaczleweli
Copy link
Contributor

Hm. Right. Updated #13356 to use rmmod and remove all modules (including legacy ones).

@behlendorf behlendorf closed this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants