From 962971d24e31ba0a82094e1786a2bfce124cc29f Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Tue, 15 Sep 2020 12:10:18 -0400 Subject: [PATCH] Cleanup: use `modprobe --remove` rather than `rmmod` MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Background ========== According to the rmmod(8) man page: rmmod is a trivial program to remove a module (when module unloading support is provided) from the kernel. Most users will want to use modprobe(8) with the -r option instead. `rmmod` simply unloads the provided module and decrements the refcount of the modules it depended on but doesn't unload those dependencies if their refcount is zero. Issue ===== With the following scenario we can end up if modules with a zero refcount still loaded in the kernel: modprobe lttng-test lttng-sessiond ... (test case) ... ctrl+c sessiond rmmod lttng-test When we teardown the lttng-sessiond, some modules are kept in the kernel because the `lttng-test` module depends on them. So unloading `lttng-test` using `rmmod` keeps those dependencies in the kernel. Solution ======== Use `modprobe --remove` to unload modules and their now unused dependencies. From the modprobe(8) man page: -r, --remove This option causes modprobe to remove rather than insert a module. If the modules it depends on are also unused, modprobe will try to remove them too. Unlike insertion, more than one module can be specified on the command line Note ==== This commit also replaces existing uses of `modprobe -r` to `modprobe --remove` for consistency. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: I7be83a645097e1eddd478cfbb717906b971f04ea --- tests/destructive/metadata-regeneration | 2 +- tests/perf/test_perf_raw.in | 2 +- tests/regression/kernel/test_clock_override | 10 ++++------ tests/regression/kernel/test_rotation_destroy_flush | 4 ++-- tests/regression/tools/filtering/test_valid_filter | 2 +- tests/regression/tools/metadata/test_kernel | 2 +- .../tools/notification/test_notification_kernel | 2 +- .../tools/notification/test_notification_multi_app | 4 ++-- tests/regression/tools/regen-metadata/test_kernel | 2 +- tests/regression/tools/regen-statedump/test_kernel | 2 +- tests/regression/tools/rotation/test_kernel | 2 +- tests/regression/tools/wildcard/test_event_wildcard | 2 +- tests/utils/utils.sh | 2 +- 13 files changed, 18 insertions(+), 20 deletions(-) diff --git a/tests/destructive/metadata-regeneration b/tests/destructive/metadata-regeneration index 729634f87..ce96bcd1a 100755 --- a/tests/destructive/metadata-regeneration +++ b/tests/destructive/metadata-regeneration @@ -211,7 +211,7 @@ skip $isroot "Root access is needed. Skipping all tests." $NUM_TESTS || modprobe lttng-test test_kernel_local test_kernel_streaming - rmmod lttng-test + modprobe --remove lttng-test test_ust_local test_ust_streaming diff --git a/tests/perf/test_perf_raw.in b/tests/perf/test_perf_raw.in index 9fab2cd79..d2e8de5df 100644 --- a/tests/perf/test_perf_raw.in +++ b/tests/perf/test_perf_raw.in @@ -133,7 +133,7 @@ skip $isroot "Root access is needed for kernel testing, skipping." 9 || { modprobe lttng-test test_kernel_raw - rmmod lttng-test + modprobe --remove lttng-test } stop_lttng_sessiond diff --git a/tests/regression/kernel/test_clock_override b/tests/regression/kernel/test_clock_override index 1fbba7717..bf1a21f27 100755 --- a/tests/regression/kernel/test_clock_override +++ b/tests/regression/kernel/test_clock_override @@ -50,7 +50,7 @@ function signal_cleanup() { diag "*** Exiting ***" stop_lttng_sessiond - modprobe -r lttng-test lttng-clock-plugin-test lttng-clock + modprobe --remove lttng-test lttng-clock-plugin-test lttng-clock full_cleanup } @@ -92,7 +92,7 @@ function test_clock_override_metadata() destroy_lttng_session_ok $SESSION_NAME stop_lttng_sessiond - modprobe -r lttng-test lttng-clock-plugin-test lttng-clock + modprobe --remove lttng-test lttng-clock-plugin-test lttng-clock local TRACE_METADATA_FILE_PATH="$(find "$TRACE_PATH" -name metadata -type f)" local TRACE_METADATA_DIR="$(dirname "$TRACE_METADATA_FILE_PATH")" @@ -140,7 +140,7 @@ function test_clock_override_timestamp() cut -d, -f1 | uniq | wc -l) test $unique_timestamps_count -gt 1 ok $? "Unique event timestamps without clock override: $unique_timestamps_count expect >1" - rmmod lttng-test + modprobe --remove lttng-test stop_lttng_sessiond # Test with clock override plugin. @@ -158,10 +158,8 @@ function test_clock_override_timestamp() stop_lttng_tracing_ok $SESSION_NAME destroy_lttng_session_ok $SESSION_NAME - rmmod lttng-test stop_lttng_sessiond - rmmod lttng-clock-plugin-test - rmmod lttng-clock + modprobe --remove lttng-clock-plugin-test lttng-clock lttng-test # Use Babeltrace with "-n all" to give a comma separated list for # easy extraction of timestamps. diff --git a/tests/regression/kernel/test_rotation_destroy_flush b/tests/regression/kernel/test_rotation_destroy_flush index 03933a3af..7120864dc 100755 --- a/tests/regression/kernel/test_rotation_destroy_flush +++ b/tests/regression/kernel/test_rotation_destroy_flush @@ -39,7 +39,7 @@ source $TESTDIR/utils/utils.sh function signal_cleanup() { diag "*** Exiting ***" - modprobe -r lttng-test + modprobe --remove lttng-test full_cleanup } @@ -106,7 +106,7 @@ function test_rotation_destroy_flush_single() rm -rf $TRACE_PATH - modprobe -r lttng-test + modprobe --remove lttng-test stop_lttng_sessiond } diff --git a/tests/regression/tools/filtering/test_valid_filter b/tests/regression/tools/filtering/test_valid_filter index 2c146fe7b..cae41b23c 100755 --- a/tests/regression/tools/filtering/test_valid_filter +++ b/tests/regression/tools/filtering/test_valid_filter @@ -1475,7 +1475,7 @@ skip $isroot "Root access is needed. Skipping all kernel valid filter tests." $N i=$(( i + 2 )) done - rmmod lttng-test + modprobe --remove lttng-test } stop_lttng_sessiond diff --git a/tests/regression/tools/metadata/test_kernel b/tests/regression/tools/metadata/test_kernel index c0a039289..a7249298e 100755 --- a/tests/regression/tools/metadata/test_kernel +++ b/tests/regression/tools/metadata/test_kernel @@ -129,7 +129,7 @@ skip $isroot "Root access is needed. Skipping all kernel metadata tests." $NUM_T ${fct_test} done - rmmod lttng-test + modprobe --remove lttng-test stop_lttng_sessiond unset LTTNG_HOME diff --git a/tests/regression/tools/notification/test_notification_kernel b/tests/regression/tools/notification/test_notification_kernel index a1795169c..cece15f26 100755 --- a/tests/regression/tools/notification/test_notification_kernel +++ b/tests/regression/tools/notification/test_notification_kernel @@ -102,7 +102,7 @@ function kernel_test wait $APP_PID 2> /dev/null - rmmod lttng-test + modprobe --remove lttng-test rm -rf ${consumerd_pipe[@]} 2> /dev/null } diff --git a/tests/regression/tools/notification/test_notification_multi_app b/tests/regression/tools/notification/test_notification_multi_app index 0c578d223..693471feb 100755 --- a/tests/regression/tools/notification/test_notification_multi_app +++ b/tests/regression/tools/notification/test_notification_multi_app @@ -336,7 +336,7 @@ function test_multi_app_kernel () wait $generator_pid 2> /dev/null rm -rf ${TESTAPP_STATE_FILE} 2> /dev/null - rmmod lttng-test + modprobe --remove lttng-test } function test_on_register_evaluation_ust () @@ -371,7 +371,7 @@ function test_on_register_evaluation_kernel() wait $generator_pid 2> /dev/null rm -rf ${TESTAPP_STATE_FILE} 2> /dev/null - rmmod lttng-test + modprobe --remove lttng-test } function test_on_register_evaluation () diff --git a/tests/regression/tools/regen-metadata/test_kernel b/tests/regression/tools/regen-metadata/test_kernel index 84249a8fc..b0143819a 100755 --- a/tests/regression/tools/regen-metadata/test_kernel +++ b/tests/regression/tools/regen-metadata/test_kernel @@ -112,7 +112,7 @@ skip $isroot "Root access is needed. Skipping all kernel streaming tests." $NUM_ ${fct_test} done - rmmod lttng-test + modprobe --remove lttng-test stop_lttng_sessiond stop_lttng_relayd } diff --git a/tests/regression/tools/regen-statedump/test_kernel b/tests/regression/tools/regen-statedump/test_kernel index be65472ba..e9932122c 100755 --- a/tests/regression/tools/regen-statedump/test_kernel +++ b/tests/regression/tools/regen-statedump/test_kernel @@ -69,6 +69,6 @@ skip $isroot "Root access is needed. Skipping all kernel streaming tests." $NUM_ ${fct_test} done - rmmod lttng-test + modprobe --remove lttng-test stop_lttng_sessiond } diff --git a/tests/regression/tools/rotation/test_kernel b/tests/regression/tools/rotation/test_kernel index ba084bc44..8ed95534f 100755 --- a/tests/regression/tools/rotation/test_kernel +++ b/tests/regression/tools/rotation/test_kernel @@ -115,7 +115,7 @@ skip $isroot "Root access is needed. Skipping all kernel streaming tests." $NUM_ clean_path $TRACE_PATH done - rmmod lttng-test + modprobe --remove lttng-test stop_lttng_sessiond stop_lttng_relayd } diff --git a/tests/regression/tools/wildcard/test_event_wildcard b/tests/regression/tools/wildcard/test_event_wildcard index 61ea67a72..14c6568ee 100755 --- a/tests/regression/tools/wildcard/test_event_wildcard +++ b/tests/regression/tools/wildcard/test_event_wildcard @@ -177,7 +177,7 @@ skip $isroot "Root access is needed. Skipping all kernel wildcard tests." $NUM_K test_event_wildcard kernel 1 'lttng*test*filter*event' test_event_wildcard kernel 1 '*lttng*test*filter*event*' - rmmod lttng-test + modprobe --remove lttng-test } stop_lttng_sessiond diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh index d03825e66..b29fa5f2d 100644 --- a/tests/utils/utils.sh +++ b/tests/utils/utils.sh @@ -664,7 +664,7 @@ function stop_lttng_sessiond_opt() if [ -n "$modules" ]; then diag "Unloading all LTTng modules" - modprobe -r "$modules" + modprobe --remove "$modules" fi fi fi -- 2.34.1