[igt-dev] [PATCH 1/4] lib/igt_kmod: simplify check for loaded module

[Lucas De Marchi]

On Fri, Apr 05, 2019 at 12:19:35AM +0100, Chris Wilson wrote:
>Quoting Lucas De Marchi (2019-04-04 23:39:22)
>> No need to go through the list of loaded modules and check one by one.
>> We can just check if it's in "live" state or if it's builtin.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  lib/igt_kmod.c | 19 ++++++-------------
>>  1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>> index 91662eb3..8d5ac788 100644
>> --- a/lib/igt_kmod.c
>> +++ b/lib/igt_kmod.c
>> @@ -119,25 +119,18 @@ bool
>>  igt_kmod_is_loaded(const char *mod_name)
>>  {
>>         struct kmod_ctx *ctx = kmod_ctx();
>> -       struct kmod_list *mod, *list;
>> +       enum kmod_module_initstate state;
>> +       struct kmod_module *kmod;
>>         bool ret = false;
>>
>> -       if (kmod_module_new_from_loaded(ctx, &list) < 0)
>> +       if (kmod_module_new_from_name(ctx, mod_name, &kmod) < 0)
>>                 goto out;
>>
>> -       kmod_list_foreach(mod, list) {
>> -               struct kmod_module *kmod = kmod_module_get_module(mod);
>> -               const char *kmod_name = kmod_module_get_name(kmod);
>> +       state = kmod_module_get_initstate(kmod);
>> +       ret = state == KMOD_MODULE_LIVE || state == KMOD_MODULE_BUILTIN;
>
>So if the module is being loaded, it is also present. If the module is
>being unloaded it is currently present and we need to wait.

It actually depends on what you want to do with this call. Technically
if it's in any state other than those above, it's not usable so we
return false.

>From what I've seen in its use, those other states shouldn't matter.
And we could actually improve them. E.g.: igt_i915_driver_unload() could
first kill the potential users and then try to remove all the modules.

It doesn't matter if the module is loaded, libkmod will already do the
necessary checks: we just need to handle the return code properly.

A potential 3rd step would be to loop through the modules and warn if
any of them are still loaded.

Lucas De Marchi

>
>Looks like this should be ret = state >= 0 ? If the module is any of
>LIVE/BUILTIN/COMING/GOING it is presently loaded.
>-Chris