check,common/rc: load module in _have_driver() and unload after test

The commit 06a0ba866d90 ("common/rc: avoid module load in
_have_driver()") removed module load from _have_driver(). However, it
was pointed out no module load in _have_driver() is confusing and adds
complexity [1]. It requires explicit module loads and unloads in number
of test cases. The module unloads must be checked if unload is safe or
not. Also module load error must be handled. To avoid these complexity,
a new helper function would be required, but it will be look like the
_have_driver() with module load.

Then revert back the feature to load module in _have_driver(). To
address the issue that the commit 06a0ba866d90 tried to fix, record the
modules loaded by _have_driver() in MODULES_TO_UNLOAD array. Unload
the recorded modules after each test case processing completed. This
avoids the side-effect by the modules loaded by _have_driver().

[1] https://lore.kernel.org/linux-block/89aedf1d-ae08-adef-db29-17e5bf85d054@grimberg.me/

Fixes: 06a0ba866d90 ("common/rc: avoid module load in _have_driver()")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

diff --git a/check b/check
index 5f57386c95d96113fee7a070b5d4b99ac238cf5a..34e96c487f3923604a29c9a92ca93a4a237314f0 100755 (executable)
--- a/check
+++ b/check
@@ -452,6 +452,16 @@ _unload_module() {
        return 1
 }
 
+_unload_modules() {
+       local i
+
+       for ((i=${#MODULES_TO_UNLOAD[@]}; i > 0; i--)); do
+               _unload_module "${MODULES_TO_UNLOAD[i-1]}" 10
+       done
+
+       unset MODULES_TO_UNLOAD
+}
+
 _run_test() {
        TEST_NAME="$1"
        CAN_BE_ZONED=0
@@ -459,6 +469,7 @@ _run_test() {
        DMESG_FILTER="cat"
        RUN_FOR_ZONED=0
        FALLBACK_DEVICE=0
+       MODULES_TO_UNLOAD=()
 
        local ret=0
 
@@ -538,6 +549,8 @@ _run_test() {
                fi
        fi
 
+       _unload_modules
+
        return $ret
 }
 

diff --git a/common/rc b/common/rc
index 9bc0dbc4224024602fd9fdcb444f6c8f0db393e9..a764b57d7deb86d76cfb23db78977a47da632ded 100644 (file)
--- a/common/rc
+++ b/common/rc
@@ -43,17 +43,24 @@ _module_file_exists()
 }
 
 # Check that the specified module or driver is available, regardless of whether
-# it is built-in or built separately as a module.
+# it is built-in or built separately as a module. Load the module if it is
+# loadable and not yet loaded. In this case, the loaded module is unloaded at
+# test case end regardless of whether the test case is skipped or executed.
 _have_driver()
 {
        local modname="${1//-/_}"
 
-       if [ ! -d "/sys/module/${modname}" ] &&
-                  ! modprobe -qn "${modname}"; then
+       if [ -d "/sys/module/${modname}" ]; then
+               return 0
+       fi
+
+       if ! modprobe -q "${modname}"; then
                SKIP_REASONS+=("driver ${modname} is not available")
                return 1
        fi
 
+       MODULES_TO_UNLOAD+=("${modname}")
+
        return 0
 }