Re: [LTP] [PATCH 2/4] lib: Add .modprobe

Petr Vorel
2023-11-03T15:54:27Z

> Hi!
> > > if (tst_test->mount_device)
> > > tst_test->format_device = 1;

> > > @@ -1362,6 +1402,19 @@ static void do_cleanup(void)

> > > tst_sys_conf_restore(0);

> > > + if (tst_test->modprobe) {
> > > + const char *name;
> > > + int i;
> > > +
> > > + for (i = 0; (name = tst_test->modprobe[i]); ++i) {
> > > + if (!modules_loaded[i])
> > > + continue;
> > > +
> > > + const char *const cmd_rmmod[] = {"rmmod", name, NULL};

> > modprobe -r please, rmmod has been deprecated for ages.

> And one more minor point, we should attempt to remove the module only if
> it has shown up in the /proc/modules.

+1

> Assuming that we want to skip the tst_module_is_buildin() check on some
> systems as Ritchie suggested we would attempt to remove build in modules
> here if we blindly trusted the return value from modpprobe.

I guess for most of distros tst_check_builtin_driver() (which reads
modules.builtin) makes sense. And with it we will have valid info if we should
remove module or not.

Then there is:

1) AOSP (Android), we should ask Edward what makes sense in Android.
IMHO old AOSP versions used insmod and rmmod, but newer could support it [2].
@Edward, am I correct? Also do AOSP even care about tests which use
tst_module_unload()?

2) NixOS
This should be IMHO fixed by checking also the correct directory (ideally
wrapped by some #ifdef, but can be even without it, if there is none).

BTW, there is also /proc/sys/kernel/modules_disabled [1], I'm not sure if we
want to just ignore it.

Kind regards,
Petr

[1] https://www.kernel.org/doc/Documentation/sysctl/kernel.txt
[2] https://source.android.com/docs/core/architecture/kernel/loadable-kernel-modules