Re: [PATCH i-g-t v2 2/3] lib/igt_kmod: Make sure errno is not already set before unbinding

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

On 12/19/2024 3:29 PM, Lucas De Marchi wrote:
> On Thu, Dec 19, 2024 at 03:05:16PM -0800, Daniele Ceraolo Spurio wrote:
>> We assert that errno is zero after unbinding, so we need to make sure it
>> wasn't already non-zero beforehand or we might incorrectly fail.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>> The non-zero starting errno hitting the assert is reproed when using
>> unbind in the 1915 reload-with-fault-injection test:
>> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12349/shard-snb5/igt@i915_module_load@reload-with-fault-injection.html 
>>
>
> yeah, but I think I made a mistake checking for errno. Checking it
> outside of a call to libc is odd.
>
>
> I'd rather remove both the assert and the setting to 0:
>
> if we have an empty dir we won't ever reach the `errno = 0`, making it
> the same as the `if (!dir)` in which we are already returning 0.

sounds good, I'll wait for CI to make sure no other issues pop out now 
that the test is running properly and then respin with this change.

>
>
> This would be a
>
> Fixes: cb008246b0c7 ("lib/igt_kmod: Rewrite xe unload logic")

thanks, I'll add this in.

Daniele

>
> thanks
> Lucas De Marchi
>
>>
>> lib/igt_kmod.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>> index d73cb4101..a008b3c29 100644
>> --- a/lib/igt_kmod.c
>> +++ b/lib/igt_kmod.c
>> @@ -662,6 +662,8 @@ int igt_kmod_unbind(const char *mod_name)
>>     int dirlen;
>>     DIR *dir;
>>
>> +    errno = 0;
>> +
>>     dirlen = snprintf(path, sizeof(path), 
>> "/sys/module/%s/drivers/pci:%s/",
>>               mod_name, mod_name);
>>     igt_assert(dirlen < sizeof(path));
>> -- 
>> 2.43.0
>>