| Message ID | 20230309-remove-holders-recursively-v2-1-8a8120269cc1@avm.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kmod: modprobe: Extend holders removal to multi-level dependencies | expand |
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@avm.de> > --- > 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
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. > >Signed-off-by: Nicolas Schier <n.schier@avm.de> >--- >I am a bit unhappy about the introduction of the 'recursive' parameter yeah... it makes it slightly harder to read. >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); this is a separate fix. We also need to unref it on error, so probably best to do: err = rmmod_do_modlist(holders, true, true); kmod_module_unref_list(holders); if (err < 0) goto error; I think the alternative to the recursive approach would be to make only the kmod_module_get_holders() be recursive: struct kmod_list *holders = recursive_holders(mod); And let recursive holders do recurse on modules passing the list as argument to be augmented. Then the rest remains the same. > } > > /* 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; wouldn't this fall in this case inside rmmod_do_remove_module()? err = kmod_module_remove_module(mod, flags); if (err == -EEXIST) { if (!first_time) 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); not sure also recursing the holders of the modules left is what we want. If there are holders, then the module's refcnt should not be 0 anyway. Lucas De Marchi > 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 >
On Wed, Apr 12, 2023 at 12:21:51PM -0700, Lucas De Marchi wrote: > 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. > > > > Signed-off-by: Nicolas Schier <n.schier@avm.de> > > --- > > I am a bit unhappy about the introduction of the 'recursive' parameter > > yeah... it makes it slightly harder to read. > > > 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); > > this is a separate fix. We also need to unref it on error, so probably > best to do: > > err = rmmod_do_modlist(holders, true, true); > kmod_module_unref_list(holders); > if (err < 0) > goto error; Thanks! Yes, sure. I sent it as a separate patch at https://lore.kernel.org/linux-modules/20230418-add-missing-kmod_module_unref_list-v1-1-ab5b554f15ee@avm.de/ > > I think the alternative to the recursive approach would be to make only > the kmod_module_get_holders() be recursive: > > struct kmod_list *holders = recursive_holders(mod); > > And let recursive holders do recurse on modules passing the list as > argument to be augmented. Then the rest remains the same. Yes, that sounds much nicer than the stuff I did. I will try and send a v3. > > } > > > > /* 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; > > wouldn't this fall in this case inside rmmod_do_remove_module()? > > err = kmod_module_remove_module(mod, flags); > if (err == -EEXIST) { > if (!first_time) > err = 0; No, as the module might already be removed and kmod_module_remove_module() thus returns -ENOENT. I think, your suggestion to regarding introduction of recursive_holders() should remove the need for checking the kmod state here. FTR: I tested the current patch w/o the kmod state check; as expected, it leads to errors: testsuite failure, when using --remove-holders: rmmod mod_dep_chain_c TESTSUITE: Added module to test delete_module: TESTSUITE: name=mod_dep_chain_c ret=0 errcode=0 TESTSUITE: Added module to test delete_module: TESTSUITE: name=mod_dep_chain_b ret=0 errcode=0 TESTSUITE: Added module to test delete_module: TESTSUITE: name=mod_dep_chain_a ret=0 errcode=0 rmmod mod_dep_chain_b libkmod: kmod_module_get_holders: could not open '/sys/module/mod_dep_chain_c/holders': No such file or directory rmmod mod_dep_chain_a libkmod: kmod_module_get_holders: could not open '/sys/module/mod_dep_chain_c/holders': No such file or directory libkmod: kmod_module_get_holders: could not open '/sys/module/mod_dep_chain_b/holders': No such file or directory rmmod mod_dep_chain_a libkmod: kmod_module_get_holders: could not open '/sys/module/mod_dep_chain_c/holders': No such file or directory libkmod: kmod_module_get_holders: could not open '/sys/module/mod_dep_chain_b/holders': No such file or directory libkmod: kmod_module_get_holders: could not open '/sys/module/mod_dep_chain_a/holders': No such file or directory modprobe: ERROR: could not remove 'mod_dep_chain_a': No such file or directory TESTSUITE: ERR: TRAP delete_module(): /home/nicolas/src/kmod/testsuite/rootfs/test-modprobe/remove-holders/sys/module/mod_dep_chain_a: opendir: No such file or directory TESTSUITE: ERR: TRAP delete_module(): unable to adjust sysfs tree TESTSUITE: running modprobe_remove_with_holders, in forked context TESTSUITE: ERR: 'modprobe_remove_with_holders' [2320200] exited with return code 1 TESTSUITE: ERR: FAILED: modprobe_remove_with_holders TESTSUITE: ------ FAIL testsuite/test-modprobe (exit status: 1) and on my dev maschine I also see attempts of module removal that fail: The relevant snippets from lsmod output: btrfs 1777664 0 zstd_compress 294912 1 btrfs ... raid456 180224 0 async_raid6_recov 24576 1 raid456 async_memcpy 20480 2 raid456,async_raid6_recov async_pq 20480 2 raid456,async_raid6_recov async_xor 20480 3 async_pq,raid456,async_raid6_recov async_tx 20480 5 async_pq,async_memcpy,async_xor,raid456,async_raid6_recov xor 24576 2 async_xor,btrfs raid6_pq 122880 4 async_pq,btrfs,raid456,async_raid6_recov and the modprobe call, without the additional kmod state check: sudo tools/modprobe -r --remove-holders xor -vv modprobe: INFO: custom logging function 0x5564236f5700 registered rmmod btrfs rmmod zstd_compress rmmod raid456 rmmod async_raid6_recov rmmod async_pq rmmod raid6_pq rmmod async_xor rmmod xor rmmod async_memcpy rmmod async_tx rmmod xor modprobe: ERROR: could not remove 'xor': No such file or directory modprobe: INFO: context 0x556423e68440 released [exit code 1] Both, testsuite and the modprobe -r, succeed as w/o complaints with the kmod state check included. (Again, as written above: I hope the kmod state check insertion becomes obsolete when switching to something like recursive_holders().) > > + } 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); > > not sure also recursing the holders of the modules left is what we want. > If there are holders, then the module's refcnt should not be 0 anyway. > > Lucas De Marchi Yeah, I have no strong opinion to that. Currently, 'modprobe -r --remove-holders MOD' removes only direct dependencies of MOD if they have a refcnt of 0 after MOD has been removed. I think, removing MOD's dependencies recursively, would make it more of an inverse operation: modprobe mod-dep-chain-c (loads mod-dep-chain-a, -b and -c) modprobe -r --remove-holders mod-dep-chain-c (unloads only mod-dep-chain-b ?) (If option '--remove-dependencies' wasn't burned, it could have been a switch to enable this?) But I cannot say, if someone really needs that functionality; thus, I will drop it in v3. Thanks for review and suggestions! Kind regards, Nicolas > > 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 > >
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) {
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. Signed-off-by: Nicolas Schier <n.schier@avm.de> --- 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(-)