Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_kmod: Wait for a kmod to finish its probe before unloding it.

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 30 Mar 2022 11:32:58 -0700, Daniele Ceraolo Spurio wrote:
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index cf7a3b22..d2ac8a56 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -143,6 +143,12 @@ out:
>	return ret;
>  }
>
> +static bool
> +igt_kmod_is_loading(struct kmod_module *kmod)
> +{
> +	return kmod_module_get_initstate(kmod) == KMOD_MODULE_COMING;

One idea would be to check for KMOD_MODULE_LIVE here which will basically
invert the logic in the loop but will be a more exact check? But anyway
it's equivalent so no need to change I guess.

>  static int modprobe(struct kmod_module *kmod, const char *options)
>  {
>	unsigned int flags;
> @@ -289,6 +295,16 @@ igt_kmod_unload(const char *mod_name, unsigned int flags)
>		goto out;
>	}
>
> +	if (igt_kmod_is_loading(kmod)) {
> +		igt_debug("%s still initializing\n", mod_name);
> +		err = igt_wait(!igt_kmod_is_loading(kmod), 10000, 100);
> +		if (err < 0) {
> +			igt_debug("%s failed to complete init within the timeout\n",
> +				  mod_name);
> +			goto out;
> +		}
> +	}
> +
>	err = igt_kmod_unload_r(kmod, flags);

I think it's better to add this code to igt_kmod_unload_r() (instead of
igt_kmod_unload()) since that calls itself recursively to remove dependent
modules?

Also it seems strange that we should explicitly have to wait for module
state to become LIVE and kmod_module_remove_module() (which in turn calls
the delete_module syscall) doesn't handle this itself? But 'man
delete_module' specifically mentions this:

  ERRORS
       EBUSY  The module is not "live" (i.e., it is still being initialized or
              is already marked for removal) ...

So a patch like this seems to be needed somewhere (in IGT or in
libkmod). So adding it to IGT for now is fine.