From d267104b87f12ea8f0bf73bff1af272f98dc069e 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/clear/test_kernel | 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/tracker/test_event_tracker | 2 +- tests/regression/tools/wildcard/test_event_wildcard | 2 +- tests/utils/utils.sh | 2 +- 15 files changed, 21 insertions(+), 23 deletions(-) diff --git a/tests/destructive/metadata-regeneration b/tests/destructive/metadata-regeneration index dcb102ff4..fed3426c9 100755 --- a/tests/destructive/metadata-regeneration +++ b/tests/destructive/metadata-regeneration @@ -205,7 +205,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 7ed6d33b9..550c0e9a3 100644 --- a/tests/perf/test_perf_raw.in +++ b/tests/perf/test_perf_raw.in @@ -123,7 +123,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 741e782a3..4b185db89 100755 --- a/tests/regression/kernel/test_clock_override +++ b/tests/regression/kernel/test_clock_override @@ -40,7 +40,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 } @@ -82,7 +82,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")" @@ -130,7 +130,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. @@ -148,10 +148,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 538d09b01..722468f57 100755 --- a/tests/regression/kernel/test_rotation_destroy_flush +++ b/tests/regression/kernel/test_rotation_destroy_flush @@ -29,7 +29,7 @@ source $TESTDIR/utils/utils.sh function signal_cleanup() { diag "*** Exiting ***" - modprobe -r lttng-test + modprobe --remove lttng-test full_cleanup } @@ -96,7 +96,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/clear/test_kernel b/tests/regression/tools/clear/test_kernel index 300a2bbc8..2fbddb84c 100755 --- a/tests/regression/tools/clear/test_kernel +++ b/tests/regression/tools/clear/test_kernel @@ -20,7 +20,7 @@ source $TESTDIR/utils/utils.sh function signal_cleanup () { stop_lttng_sessiond - modprobe -r lttng-test + modprobe --remove lttng-test full_cleanup } @@ -620,7 +620,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/filtering/test_valid_filter b/tests/regression/tools/filtering/test_valid_filter index 6d1009a48..8f3e5f4fe 100755 --- a/tests/regression/tools/filtering/test_valid_filter +++ b/tests/regression/tools/filtering/test_valid_filter @@ -1465,7 +1465,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 503bd13de..82faf43b9 100755 --- a/tests/regression/tools/metadata/test_kernel +++ b/tests/regression/tools/metadata/test_kernel @@ -119,7 +119,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 1eb0d1067..4caa976b2 100755 --- a/tests/regression/tools/notification/test_notification_kernel +++ b/tests/regression/tools/notification/test_notification_kernel @@ -90,7 +90,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 dae482ce3..9468d0a0d 100755 --- a/tests/regression/tools/notification/test_notification_multi_app +++ b/tests/regression/tools/notification/test_notification_multi_app @@ -325,7 +325,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 () @@ -360,7 +360,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 0f879e636..8c31d488e 100755 --- a/tests/regression/tools/regen-metadata/test_kernel +++ b/tests/regression/tools/regen-metadata/test_kernel @@ -102,7 +102,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 92f584bbd..00c7d76ce 100755 --- a/tests/regression/tools/regen-statedump/test_kernel +++ b/tests/regression/tools/regen-statedump/test_kernel @@ -59,6 +59,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 f4019b461..7cd51bc03 100755 --- a/tests/regression/tools/rotation/test_kernel +++ b/tests/regression/tools/rotation/test_kernel @@ -105,7 +105,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/tracker/test_event_tracker b/tests/regression/tools/tracker/test_event_tracker index fe4114ab4..711690af6 100755 --- a/tests/regression/tools/tracker/test_event_tracker +++ b/tests/regression/tools/tracker/test_event_tracker @@ -537,7 +537,7 @@ skip $isroot "Root access is needed. Skipping all kernel tracker tests." $NUM_KE test_event_tracker kernel 1 "${EVENT_NAME}" "--pid --all" test_event_pid_tracker kernel 1 "${EVENT_NAME}" - rmmod lttng-test + modprobe --remove lttng-test ok $? "Unloading lttng-test module" } diff --git a/tests/regression/tools/wildcard/test_event_wildcard b/tests/regression/tools/wildcard/test_event_wildcard index 0bf2d2c2c..36d56cd3e 100755 --- a/tests/regression/tools/wildcard/test_event_wildcard +++ b/tests/regression/tools/wildcard/test_event_wildcard @@ -167,7 +167,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 8e6563d06..b0a64ff94 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