[PATCH i-g-t v2 1/4] tests/intel/xe_configfs: Do not execute with open fd

[Lucas De Marchi]

On Tue, Aug 26, 2025 at 09:54:49AM +0200, Kamil Konieczny wrote:
>Hi Lucas,
>On 2025-08-25 at 08:59:43 -0700, Lucas De Marchi wrote:
>> The survivability_mode test does an unbind/set-configfs/bind sequence.
>> If and fd is open, that will still refer to the old instance of the
>> driver and is a dangling fd. Close it right when it's not needed anymore
>> as in future other tests will also unbind/bind.
>>
>> While moving this code around, also make sure to use drm_close_driver()
>> so the fd is dropped from the cache.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>
>Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>
>See one nit below not related to your change.
>
>> ---
>> v2:
>> - Use drm_close_driver()  (Kamil)
>> ---
>>  tests/intel/xe_configfs.c | 20 ++++++++++++++------
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/intel/xe_configfs.c b/tests/intel/xe_configfs.c
>> index 5c65cd01d..57f770b0b 100644
>> --- a/tests/intel/xe_configfs.c
>> +++ b/tests/intel/xe_configfs.c
>> @@ -101,15 +101,19 @@ static void test_engines_allowed(int configfs_device_fd)
>>  	}
>>  }
>>
>> -static int create_device_configfs_group(int configfs_fd, int fd)
>> +static void set_bus_addr(int fd)
>>  {
>> -	int configfs_device_fd;
>>  	struct pci_device *pci_dev;
>> -	mode_t mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
>>
>>  	pci_dev = igt_device_get_pci_device(fd);
>>  	snprintf(bus_addr, sizeof(bus_addr), "%04x:%02x:%02x.%01x",
>>  		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
>> +}
>> +
>> +static int create_device_configfs_group(int configfs_fd)
>> +{
>> +	mode_t mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
>> +	int configfs_device_fd;
>>
>>  	configfs_device_fd = igt_fs_create_dir(configfs_fd, bus_addr, mode);
>>  	igt_assert(configfs_device_fd);
>
>imho this assert should be igt_require? note that it is in

it's expected to fail if it can't create that dir

>an igt_fixup or function should just return it and leave
>assert/require to a test.

but this is just making a common part of the tests into a helper
function. I'd agree with this if this function was in
lib/igt_configfs.[ch].

>Also if create_dir fails, will this value be -1 or -errno
>so assert will not catch it.

yes, this looks wrong.

thanks
Lucas De Marchi