On Mon 2026-05-25 00:06:21, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) > to consider: > - [Medium] The cleanup loop iterates over the TEST_MODS array in forward > order, violating dependency requirements and causing rmmod to fail. > - [Medium] Functions invoked during cleanup call die() on failure, > prematurely aborting the EXIT trap and skipping pop_config(). > -- > > commit aafb48b2a54fe2209742378f31fb96583598557a > Author: Marcos Paulo de Souza <[email protected]> > > selftests: livepatch: Remove leftover modules when a testcase fails > > This commit introduces a cleanup mechanism for livepatch selftests that > tracks loaded modules and attempts to remove them if a test fails or exits > prematurely. It uses a TEST_MODS array to keep track of dependencies and > unloads the remaining modules during an EXIT trap. > > > diff --git a/tools/testing/selftests/livepatch/functions.sh > > b/tools/testing/selftests/livepatch/functions.sh > > index 3ec0b7962fc5c..25f1370038654 100644 > > --- a/tools/testing/selftests/livepatch/functions.sh > > +++ b/tools/testing/selftests/livepatch/functions.sh > [ ... ] > > @@ -125,6 +127,14 @@ function set_ftrace_enabled() { > > } > > > > function cleanup() { > > + # Remove leftover modules in reverse order to handle dependencies > > + for mod_item in "${TEST_MODS[@]}"; do > > [Severity: Medium] > Does this loop actually iterate in reverse order? > > The comment states the modules will be removed in reverse order to handle > dependencies, but ${TEST_MODS[@]} expands the array in ascending index > order. > > Because dependent modules (like livepatches) are loaded after their base > modules, iterating in forward order will attempt to unload the base modules > first. This would likely cause rmmod to fail with a module in use error.
The ordering should not be important. It does not matter whether the livepatch or livepatched module is removed first. And only one livepatch is enabled when atomic replace is used. I believe that the ordering does not matter even when more livepatches are enabled and the atomic replace is not used. Anyway, there is an idea to make the no-replace livepatches trully independent, see https://lore.kernel.org/r/[email protected] I would personally ignore the ordering here. Best Regards, Petr
