| Message ID | 20231103160350.1096410-1-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r" | expand |
Hi, On Fri, Nov 3, 2023 at 9:03 AM Petr Vorel <pvorel@suse.cz> wrote: > > "modprobe -r" will remove also the dependencies loaded for kernel > modules. > > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > lib/tst_module.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/tst_module.c b/lib/tst_module.c > index 9bd443623..e52bb6e00 100644 > --- a/lib/tst_module.c > +++ b/lib/tst_module.c > @@ -105,7 +105,7 @@ void tst_module_unload_(void (cleanup_fn)(void), const char *mod_name) > { > int i, rc; > > - const char *const argv[] = { "rmmod", mod_name, NULL }; > + const char *const argv[] = { "modprobe", "-r", mod_name, NULL }; > > rc = 1; > for (i = 0; i < 50; i++) { > -- > 2.42.0 > This is fine with all the supported versions of Android GKI. Reviewed-by: Edward Liaw <edliaw@google.com>
Hi all, > "modprobe -r" will remove also the dependencies loaded for kernel > modules. > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > lib/tst_module.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/lib/tst_module.c b/lib/tst_module.c > index 9bd443623..e52bb6e00 100644 > --- a/lib/tst_module.c > +++ b/lib/tst_module.c > @@ -105,7 +105,7 @@ void tst_module_unload_(void (cleanup_fn)(void), const char *mod_name) > { > int i, rc; > - const char *const argv[] = { "rmmod", mod_name, NULL }; > + const char *const argv[] = { "modprobe", "-r", mod_name, NULL }; I'm sorry, obviously some modules needs rmmod: # ./delete_module03 ... delete_module03.c:32: TPASS: delete_module() failed as expected: EAGAIN/EWOULDBLOCK (11) tst_module.c:121: TWARN: could not unload dummy_del_mod_dep.ko module tst_module.c:121: TWARN: could not unload dummy_del_mod.ko module ... # ./delete_module03 insmod: ERROR: could not insert module dummy_del_mod.ko: File exists tst_cmd.c:121: TBROK: 'insmod' exited with a non-zero code 1 at tst_cmd.c:121 It's because these modules cannot be found by modprobe # modprobe -r dummy_del_mod_dep modprobe: FATAL: Module dummy_del_mod_dep not found. # modprobe -r dummy_del_mod modprobe: FATAL: Module dummy_del_mod not found. # rmmod dummy_del_mod_dep; echo $?; rmmod dummy_del_mod; echo $? 0 0 I guess for .modprobe_module we should have function which uses 'modprobe -r'. And we should keep tst_module_unload() for these special cases. I would rename it to tst_module_rmmod() to be obvious. Setting this as changes requested, next week there is Hackweek in SUSE, I'll send v3 following week. Kind regards, Petr
Hi Edward, > Hi, > On Fri, Nov 3, 2023 at 9:03 AM Petr Vorel <pvorel@suse.cz> wrote: > > "modprobe -r" will remove also the dependencies loaded for kernel > > modules. > > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > lib/tst_module.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/tst_module.c b/lib/tst_module.c > > index 9bd443623..e52bb6e00 100644 > > --- a/lib/tst_module.c > > +++ b/lib/tst_module.c > > @@ -105,7 +105,7 @@ void tst_module_unload_(void (cleanup_fn)(void), const char *mod_name) > > { > > int i, rc; > > - const char *const argv[] = { "rmmod", mod_name, NULL }; > > + const char *const argv[] = { "modprobe", "-r", mod_name, NULL }; > > rc = 1; > > for (i = 0; i < 50; i++) { > > -- > > 2.42.0 > This is fine with all the supported versions of Android GKI. > Reviewed-by: Edward Liaw <edliaw@google.com> Thanks for confirmation. Although this patch will not be merged, we know we can use 'modprobe -r'. BTW is it ok for AOSP which uses latest LTP to rely on modules.builtin and modules.dep? (there is #ifdef __ANDROID__ in tst_search_driver()). Kind regards, Petr
On Fri, Nov 3, 2023 at 3:16 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Edward, > > > Hi, > > > > > On Fri, Nov 3, 2023 at 9:03 AM Petr Vorel <pvorel@suse.cz> wrote: > > > > "modprobe -r" will remove also the dependencies loaded for kernel > > > modules. > > > > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > > --- > > > lib/tst_module.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/tst_module.c b/lib/tst_module.c > > > index 9bd443623..e52bb6e00 100644 > > > --- a/lib/tst_module.c > > > +++ b/lib/tst_module.c > > > @@ -105,7 +105,7 @@ void tst_module_unload_(void (cleanup_fn)(void), > const char *mod_name) > > > { > > > int i, rc; > > > > - const char *const argv[] = { "rmmod", mod_name, NULL }; > > > + const char *const argv[] = { "modprobe", "-r", mod_name, NULL > }; > > > > rc = 1; > > > for (i = 0; i < 50; i++) { > > > -- > > > 2.42.0 > > > > This is fine with all the supported versions of Android GKI. > > Reviewed-by: Edward Liaw <edliaw@google.com> > > Thanks for confirmation. Although this patch will not be merged, we know > we can > use 'modprobe -r'. > > BTW is it ok for AOSP which uses latest LTP to rely on modules.builtin and > modules.dep? (there is #ifdef __ANDROID__ in tst_search_driver()). > Ah, it seems like no. modules.builtin and modules.dep are not on the android devices I'm testing with. I can ask around to see why that is. > Kind regards, > Petr >
diff --git a/lib/tst_module.c b/lib/tst_module.c index 9bd443623..e52bb6e00 100644 --- a/lib/tst_module.c +++ b/lib/tst_module.c @@ -105,7 +105,7 @@ void tst_module_unload_(void (cleanup_fn)(void), const char *mod_name) { int i, rc; - const char *const argv[] = { "rmmod", mod_name, NULL }; + const char *const argv[] = { "modprobe", "-r", mod_name, NULL }; rc = 1; for (i = 0; i < 50; i++) {
"modprobe -r" will remove also the dependencies loaded for kernel modules. Suggested-by: Cyril Hrubis <chrubis@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- lib/tst_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)