On Wed, Mar 29, 2023 at 03:51:35PM +0200 Nicolas Schier wrote:
> Remove holders recursively when removal of module holders is requested.
>
> Extend commit 42b32d30c38e ("modprobe: Fix holders removal") by removing
> also indirect holders if --remove-holders is set. For a simple module
> dependency chain like
>
> module3 depends on module2
> module2 depends on module1
>
> 'modprobe -r --remove-holders module1' will remove module3, module2 and
> module1 in correct order:
>
> $ modprobe -r --remove-holders module1 --verbose
> rmmod module3
> rmmod module2
> rmmod module1
>
> (Actually, it will do the same when specifying module2 or module3 for
> removal instead of module1.)
>
> As a side-effect, 'modprobe -r module3' (w/o --remove-holders) will also
> remove all three modules from the example above, as after removal of
> module3, module2 does not have any holders and the same holds for
> module1 after removal of module2:
>
> $ modprobe -r module3 --verbose
> rmmod module3
> rmmod module2
> rmmod module1
>
> Without recursive evaluation of holders, modprobe leaves module1 loaded.
>
> Unfortunately, '--dry-run --remove-holders' breaks with indirect
> dependencies.
Ups. This is not true anymore, I forgot to remove this sentence from the
commit message.
Kind regards,
Nicolas
>
> Signed-off-by: Nicolas Schier <n.schier@xxxxxx>
> ---
> I am a bit unhappy about the introduction of the 'recursive' parameter
> to rmmod_do_modlist() as it always holds the same value as
> stop_on_errors; is re-using (and renaming) possibly a better option?
>
> modprobe --remove --remove-holders --dry-run now ignores current
> refcounts of loaded modules when simulating removal of holders.
>
> Changes in v2:
> * Handle modules that have just been removed, gently
> * Fix --dry-run by ignoring module refcounts (_only_ for --dry-run)
> * Add missing kmod_module_unref_list() calls
> ---
> tools/modprobe.c | 44 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/tools/modprobe.c b/tools/modprobe.c
> index 3b7897c..a705f88 100644
> --- a/tools/modprobe.c
> +++ b/tools/modprobe.c
> @@ -390,13 +390,27 @@ static int rmmod_do_remove_module(struct kmod_module *mod)
> static int rmmod_do_module(struct kmod_module *mod, int flags);
>
> /* Remove modules in reverse order */
> -static int rmmod_do_modlist(struct kmod_list *list, bool stop_on_errors)
> +static int rmmod_do_modlist(struct kmod_list *list, bool stop_on_errors,
> + bool recursive)
> {
> struct kmod_list *l;
>
> kmod_list_foreach_reverse(l, list) {
> struct kmod_module *m = kmod_module_get_module(l);
> - int r = rmmod_do_module(m, RMMOD_FLAG_IGNORE_BUILTIN);
> + int r = 0;
> +
> + if (recursive && kmod_module_get_initstate(m) >= 0) {
> + struct kmod_list *holders = kmod_module_get_holders(m);
> +
> + r = rmmod_do_modlist(holders, stop_on_errors,
> + recursive);
> +
> + kmod_module_unref_list(holders);
> + }
> +
> + if (!r)
> + r = rmmod_do_module(m, RMMOD_FLAG_IGNORE_BUILTIN);
> +
> kmod_module_unref(m);
>
> if (r < 0 && stop_on_errors)
> @@ -448,15 +462,17 @@ static int rmmod_do_module(struct kmod_module *mod, int flags)
> }
>
> /* 1. @mod's post-softdeps in reverse order */
> - rmmod_do_modlist(post, false);
> + rmmod_do_modlist(post, false, false);
>
> /* 2. Other modules holding @mod */
> if (flags & RMMOD_FLAG_REMOVE_HOLDERS) {
> struct kmod_list *holders = kmod_module_get_holders(mod);
>
> - err = rmmod_do_modlist(holders, true);
> + err = rmmod_do_modlist(holders, true, true);
> if (err < 0)
> goto error;
> +
> + kmod_module_unref_list(holders);
> }
>
> /* 3. @mod itself, but check for refcnt first */
> @@ -472,9 +488,16 @@ static int rmmod_do_module(struct kmod_module *mod, int flags)
> }
> }
>
> - if (!cmd)
> - err = rmmod_do_remove_module(mod);
> - else
> + if (!cmd) {
> + int state = kmod_module_get_initstate(mod);
> +
> + if (state < 0) {
> + /* Module was removed during recursive holder removal */
> + err = 0;
> + } else {
> + err = rmmod_do_remove_module(mod);
> + }
> + } else
> err = command_do(mod, "remove", cmd, NULL);
>
> if (err < 0)
> @@ -488,14 +511,14 @@ static int rmmod_do_module(struct kmod_module *mod, int flags)
> kmod_list_foreach(itr, deps) {
> struct kmod_module *dep = kmod_module_get_module(itr);
> if (kmod_module_get_refcnt(dep) == 0)
> - rmmod_do_remove_module(dep);
> + rmmod_do_module(dep, flags);
> kmod_module_unref(dep);
> }
> kmod_module_unref_list(deps);
> }
>
> /* 5. @mod's pre-softdeps in reverse order: errors are non-fatal */
> - rmmod_do_modlist(pre, false);
> + rmmod_do_modlist(pre, false, false);
>
> error:
> kmod_module_unref_list(pre);
> @@ -975,6 +998,9 @@ static int do_modprobe(int argc, char **orig_argv)
> fstat(fileno(stderr), &stat_buf)))
> use_syslog = 1;
>
> + if (remove_holders && dry_run)
> + ignore_loaded = 1;
> +
> log_open(use_syslog);
>
> if (!do_show_config) {
>
> --
> 2.40.0
