[PATCH i-g-t v2 4/6] lib/kunit: Support writable filter* parameters of kunit module

[Lucas De Marchi]

On Thu, Feb 01, 2024 at 04:07:41PM +0100, Kamil Konieczny wrote:
>Hi Janusz,
>On 2024-01-31 at 19:03:51 +0100, Janusz Krzysztofik wrote:
>> Instead of wasting resources on reloading the base Kunit module each time
>> a different set of filter parameters is needed, try to write the required
>> values to sysfs representation of those parameters.  If that fails (e.g.
>> on older LTS kernels with read-only filter parameters), fall back to
>> legacy "no list of test cases available" mode as we do on kernels with
>> those module parameters not supported at all.
>>
>> This modification will also serve as a workaround for the issue of
>> impossibility to unload the base Kunit module on Xe platforms, available
>> to IGT as soon as the module supports writable filter parameters.
>>
>> v2: Work around ineffective writes of empty strings to sysfs module
>>     parameter files (Lucas) by using human readable non-empty strings that
>>     give the same results as default NULLs,
>>   - drop fallback to reload of base Kunit module method if assigning new
>>     values to module parameters via sysfs fails (Lucas), instead use the
>>     existing fallback to blind execution like if getting a list of test
>>     cases was not supported at all,
>>   - split move of open_parameters() helper up in the source file as well
>>     as cleanup of base KUnit module unloading to separate patches (Kamil),
>>   - address the issue of the second paragraph of commit description
>>     suggesting two separate changes combined in one patch (Kamil) by
>>     rewording the description.
>>
>> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>> ---
>>  lib/igt_kmod.c | 114 ++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 84 insertions(+), 30 deletions(-)
>>
>> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>> index d2c28d0a64..1d57ea708d 100644
>> --- a/lib/igt_kmod.c
>> +++ b/lib/igt_kmod.c
>> @@ -811,6 +811,56 @@ static int open_parameters(const char *module_name)
>>  	return open(path, O_RDONLY);
>>  }
>>
>> +static const char *kunit_params[3] = {
>> +	"filter_glob", "filter", "filter_action",
>> +};
>> +
>> +static bool kunit_set_params(const char **values)
>> +{
>> +	/*
>> +	 * Reapplying default NULLs to module parameters via sysfs seems not
>> +	 * possible, and applying empty strings doesn't work as expected,
>> +	 * leaving current values untouched.  As a workaround, use
>> +	 * human-readable non-empty strings that exhibit the same behaviour
>> +	 * as if default NULLs were in place.  In case of filter_action
>> +	 * parameter there is no such obvious non-empty string, however,
>> +	 * that's not a problem for us since even if it was previously set
>> +	 * to "skip" and we leave it as is, our "module!=none" default filter
>> +	 * guarantees that no test cases are filtered out to be skipped.
>> +	 */
>> +	const char *defaults[ARRAY_SIZE(kunit_params)] = {
>> +	     /* filter_glob, filter,	     filter_action */
>> +		"*",	     "module!=none", "",
>
>If writing empty string poses a problem, what about writing
>one space? Could it work?

no. we can't write a different value and expect the kernel to treat it
the same. There's no "strip()" happending on the kernel side and
depending on the sysfs file a space might make sense as a real value.

Lucas De Marchi