[igt-dev] [PATCH i-g-t] tests/i915_module_load: Use new name of fault injection module parameter

[Janusz Krzysztofik]

Hi Chris,

On Tuesday, October 29, 2019 1:49:26 PM CET Chris Wilson wrote:
> From: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> 
> Name of the i915 module parameter providing fault injection function is
> changing for consistency with a new convention of naming i915 driver
> internal functions called from the driver PCI .probe entry point.  Adjust
> the test to use the new name.
> 
> [ickle: keep the old parameter name around for stable testing]
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Cc: Michał Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Piotr Piórkowski <piotr.piorkowski at intel.com>
> Cc: Tomasz Lis <tomasz.lis at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  lib/igt_kmod.c                | 40 +++++++++++++++++++++++++++++++++++
>  lib/igt_kmod.h                |  2 ++
>  tests/i915/i915_module_load.c |  8 ++++++-
>  3 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index c3da46678..f43da9f42 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -153,6 +153,46 @@ static int modprobe(struct kmod_module *kmod, const 
char *options)
>  					       NULL, NULL, 
NULL);
>  }
>  
> +/**
> + * igt_kmod_has_param:
> + * @mod_name: The name of the module
> + * @param: The name of the parameter
> + *
> + * Returns: true if the module has the parameter, false otherwise.
> + */
> +bool igt_kmod_has_param(const char *module_name, const char *param)
> +{
> +	struct kmod_module *kmod;
> +	struct kmod_list *d, *pre;
> +	bool result = false;
> +
> +	if (kmod_module_new_from_name(kmod_ctx(), module_name, &kmod))
> +		return false;
> +
> +	pre = NULL;
> +	if (!kmod_module_get_info(kmod, &pre))
> +		goto out;
> +
> +	kmod_list_foreach(d, pre) {
> +		const char *key, *val;
> +
> +		key = kmod_module_info_get_key(d);
> +		if (strcmp(key, "parmtype"))
> +			continue;
> +
> +		val = kmod_module_info_get_value(d);
> +		if (val && strcmp(val, param) == 0) {
> +			result = true;
> +			break;
> +		}
> +	}
> +	kmod_module_info_free_list(pre);
> +
> +out:
> +	kmod_module_unref(kmod);
> +	return result;
> +}
> +
>  /**
>   * igt_kmod_load:
>   * @mod_name: The name of the module
> diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
> index 87d36d400..e47ff2df6 100644
> --- a/lib/igt_kmod.h
> +++ b/lib/igt_kmod.h
> @@ -31,6 +31,8 @@
>  bool igt_kmod_is_loaded(const char *mod_name);
>  void igt_kmod_list_loaded(void);
>  
> +bool igt_kmod_has_param(const char *mod_name, const char *param);
> +
>  int igt_kmod_load(const char *mod_name, const char *opts);
>  int igt_kmod_unload(const char *mod_name, unsigned int flags);
>  
> diff --git a/tests/i915/i915_module_load.c b/tests/i915/i915_module_load.c
> index f42083f53..7d9a5cfd2 100644
> --- a/tests/i915/i915_module_load.c
> +++ b/tests/i915/i915_module_load.c
> @@ -350,11 +350,17 @@ igt_main
>  	}
>  
>  	igt_subtest("reload-with-fault-injection") {
> +		const char *param;
>  		int i = 0;
>  
>  		igt_i915_driver_unload();
>  
> -		while (inject_fault("i915", "inject_load_failure", +
+i) == 0)
> +		param = "inject_probe_failure";
> +		if (!igt_kmod_has_param("i915", param))
> +			param = "inject_load_failure";
> +		igt_require(igt_kmod_has_param("i915", param));
> +
> +		while (inject_fault("i915", param, ++i) == 0)
>  			;

My first thought was to just retry the loop with the old parameter name if the 
new one fails but I'm OK with your approach (maybe there will be other users 
of the new library helper).  In case R-b is needed, please tell me.

Thanks,
Janusz



>  
>  		/* We expect to hit at least one fault! */
>