[LTP] [PATCH 1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r"

[LTP] [PATCH 1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r"

[Petr Vorel]

"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


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r"

[Edward Liaw via ltp]

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>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r"

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r"

[Edward Liaw via ltp]

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
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] lib/tst_module.c: Replace "rmmod" with "modprobe -r"

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp