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

["Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>]

On 3/31/2022 3:52 PM, Dixit, Ashutosh wrote:
> On Wed, 30 Mar 2022 11:32:58 -0700, Daniele Ceraolo Spurio wrote:
>> Modprobing i915 can result in other dependent modules being loaded
>> automatically as a follow up. This means that by the time the i915 load
>> function returns these modules may still be going through their init, so
>> if we try to immediately unload i915 again we will fail with an
>> -EBUSY.
> Can you explain a bit where we see this happening and if any tests fail
> because of this? My expectation would be if module A depends on module B,
> loading A will cause B to load, but loading B will not cause A to load. So
> what is causing these dependent modules to load when we load i915? Is there
> e.g. something in /etc/modprobe.d or udev which is causing this to happen?

This is a new scenario, introduced with the GSC series where i915 
exposes the GSC as an aux device and the mei-gsc module binds to it 
(link to the kernel series in the cover letter). The kernel handles the 
addition of an aux device similarly to a device hot-plug, with the 
relevant driver (mei-gsc in this case) being automatically probed, 
without requiring an explicit modprobe. This aux device probe happens 
separately from the parent device probe, which means that the i915 probe 
can complete and return before the mei-gsc probe is done. The fact that 
mei-gsc binds to an i915 aux device creates a dependency, even if 
mei-gsc doesn't actually explicitly use any function exported by i915, 
so we need to unload it to be able to unload i915.

> Currently in IGT we maintain a list of modules depenedent on i915 which we
> unload prior to unloading i915. An alternative approach to this patch may
> be to maintain a similar list in igt_i915_driver_load() and load dependent
> modules after loading i915 as long as they are present on the system. So we
> would not need to wait for these dependent modules to complete loading when
> we unload i915. Do you think this is feasible?

No, as mentioned above the module is loaded automatically.

Daniele

>
> Thanks.
>
>> To avoid this, if the module load is still in progress, wait for it to
>> complete before unloading.
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   lib/igt_kmod.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> 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;
>> +}
>> +
>>   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);
>> 	if (err < 0) {
>> 		igt_debug("Could not remove module %s (%s)\n", mod_name,
>> --
>> 2.25.1
>>